hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yuzhih...@gmail.com
Subject Re: question regarding code
Date Fri, 30 Dec 2011 19:10:02 GMT
Mikael:
Can you upload a patch ?

Thanks



On Dec 30, 2011, at 11:06 AM, Mikael Sitruk <mikael.sitruk@gmail.com> wrote:

> Thanks to all of you, jira -
> HBASE-5110<https://issues.apache.org/jira/browse/HBASE-5110> (code
> enhancement - remove unnecessary if-checks in every loop in HLog class)
> 
> Happy new year for all of you.
> 
> Mikael.S
> 
> On Fri, Dec 30, 2011 at 6:34 AM, Lars H <lhofhansl@yahoo.com> wrote:
> 
>> oh no no... please file a jira and help us make HBase better.
>> 
>> Mikael Sitruk <mikael.sitruk@gmail.com> schrieb:
>> 
>>> Lars hi
>>> 
>>> This method will be called anytime that old log exist and might be cleaned
>>> (HLog.cleanOldsLogs()) and there are too much logs.
>>> It is called from LogRoller thread, Hlog creation, and
>> metaUtils.shutdown()
>>> (code in 0.90.1 - also present at least in those classes in trunk),
>>> The creation of Arraylist is approx 24 bytes (8 for Arraylist object, 12
>>> for the array member and 4 for the int member) if I didn't forgot
>>> something, nevertheless the "if" will not be checked for each region
>> having
>>> an older log file (which will occurs a lot if you have heavy writes
>>> scenarios).
>>> This is true that this will create garbage (in case it will not be used)
>>> but this is a short lived object, it will be collected immediately with a
>>> minor garbage collection. It would have be more problematic if the object
>>> was long lived.
>>> 
>>> Anyway you are the one to decide, if you accept a jira, i will open one,
>>> just let me know.
>>> 
>>> Regards,
>>> Mikael.S
>>> 
>>> 
>>> On Thu, Dec 29, 2011 at 6:54 PM, lars hofhansl <lhofhansl@yahoo.com>
>> wrote:
>>> 
>>>> The only downside is that the ArrayList now is always created, even if
>> it
>>>> is not needed, thereby producing unnecessary garbage.
>>>> I have not bearing how frequently we'll get here and find no relevant
>>>> regions, though.
>>>> 
>>>> 
>>>> -- Lars
>>>> 
>>>> 
>>>> ----- Original Message -----
>>>> From: Harsh J <harsh@cloudera.com>
>>>> To: dev@hbase.apache.org
>>>> Cc:
>>>> Sent: Thursday, December 29, 2011 12:51 AM
>>>> Subject: Re: question regarding code
>>>> 
>>>> Yeah that'd work too. File a JIRA with the change?
>>>> 
>>>> On 29-Dec-2011, at 2:12 PM, Mikael Sitruk wrote:
>>>> 
>>>>> Hi all
>>>>> 
>>>>> I have question on some code (taken from HLog) see below
>>>>> 
>>>>> 
>>>>> static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long
>>>>> oldestWALseqid,
>>>>>     final Map<byte [], Long> regionsToSeqids) {
>>>>>   //  This method is static so it can be unit tested the easier.
>>>>>   List<byte []> regions = null;
>>>>>   for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet())
{
>>>>>     if (e.getValue().longValue() <= oldestWALseqid) {
>>>>>       if (regions == null) regions = new ArrayList<byte []>();
>>>>>       regions.add(e.getKey());
>>>>>     }
>>>>>   }
>>>>>   return regions == null?
>>>>>     null: regions.toArray(new byte [][]
>> {HConstants.EMPTY_BYTE_ARRAY});
>>>>> }
>>>>> 
>>>>> Shouldn't be better to remove the if in the loop doing as follow?
>>>>> 
>>>>> static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long
>>>>> oldestWALseqid,
>>>>>     final Map<byte [], Long> regionsToSeqids) {
>>>>>   //  This method is static so it can be unit tested the easier.
>>>>>   List<byte []> regions = new ArrayList<byte []>();
>>>>>   for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet())
{
>>>>>     if (e.getValue().longValue() <= oldestWALseqid) {
>>>>>       //if (regions == null) regions = new ArrayList<byte []>();
>>>>>       regions.add(e.getKey());
>>>>>     }
>>>>>   }
>>>>>   return regions.size() == 0?
>>>>>     null: regions.toArray(new byte [][]
>> {HConstants.EMPTY_BYTE_ARRAY});
>>>>> }
>>>>> 
>>>>> regards,
>>>>> 
>>>>> Mikael.S
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Mikael.S
>> 
> 
> 
> 
> -- 
> Mikael.S

Mime
View raw message