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 <lhofha...@yahoo.com> wrote: > oh no no... please file a jira and help us make HBase better. > > Mikael Sitruk <mikael.sit...@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 <lhofha...@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 <ha...@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