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

Reply via email to