lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yonik Seeley <yo...@lucidimagination.com>
Subject Re: Greetings and questions about patches
Date Thu, 23 Apr 2009 03:36:32 GMT
On Wed, Apr 22, 2009 at 9:33 PM, Erick Erickson <erickerickson@gmail.com> wrote:
> So, according to the coverage report, there are two methods that
> are never executed by the unit tests (actually 4, 2 that operate on
> ints and 2 that operate on longs), isPowerOfTwo and
> nextHighestPowerOfTwo. nextHighestPowerOfTwo is especially
> clever, had to get out a paper and pencil to really understand it.
>
> Issues:
> 1> none of these methods is ever called.

OpenBitset, BitUtil, etc, were developed in Solr and later moved to
Lucene.  Solr does use nextHighestPowerOfTwo().
isPowerOfTwo() I had used at some point as a short circuit to check if
something was already a power of two already... probably only worth it
for programmer constants that are supposed to be a power of two (i.e.
the short circuit would be worth it).

>   1a> What's the consensus about unused code? Take it out (my
>          preference)

That's fine with me... I don't mind moving stuff that's only used by
solr back to solr.

> 2> I don't agree with the behavior of nextHighestPowerOfTwo. Should
>      I make changes if we decide to keep it?
>   2a> Why should it return the parameter passed in when it happens to be
>         a perfect power of two? e.g. this passes:
>        assertEquals(BitUtil.nextHighestPowerOfTwo(128L), 128);
>        I'd expect this to actually return 256, given the name.

It's really to ensure that you have a power of two (for fast
hashtables).  Perhaps the name should be changed rather than the
functionality?

> 2b> Why should it ever return 0? There's no power of two that is
>        zero. e.g. this passes:
>        assertEquals(BitUtil.nextHighestPowerOfTwo(-1), 0);
>        as does this: assertEquals(BitUtil.nextHighestPowerOfTwo(0), 0).
>        *Assuming* that someone wants to use this sometime to, say, size
>         an array they'd have to test against a return of 0.

An array of size zero would actually work to store 0 elements though.
And it's a utility function that should be kept as fast as possible -
a zero check would add a branch.

-Yonik
http://www.lucidimagination.com

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message