Mikael: I think you can go ahead and open a JIRA. Happy New Year.
On Thu, Dec 29, 2011 at 4:15 PM, Mikael Sitruk <[email protected]>wrote: > 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 >
