Mikael: Can you upload a patch ? Thanks
On Dec 30, 2011, at 11:06 AM, Mikael Sitruk <[email protected]> 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 <[email protected]> wrote: > >> oh no no... please file a jira and help us make HBase better. >> >> Mikael Sitruk <[email protected]> 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 <[email protected]> >> 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 <[email protected]> >>>> To: [email protected] >>>> 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
