it should not be a problem i'll try Mikael.S On Fri, Dec 30, 2011 at 9:10 PM, <yuzhih...@gmail.com> wrote:
> Mikael: > Can you upload a patch ? > > Thanks > > > > On Dec 30, 2011, at 11:06 AM, Mikael Sitruk <mikael.sit...@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 <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 > -- Mikael.S
