[
https://issues.apache.org/jira/browse/LUCENE-3449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13112885#comment-13112885
]
Michael McCandless commented on LUCENE-3449:
--------------------------------------------
bq. Can you check if this actually slows down things, Mike?
Well... I doubt this is easily measured empirically?
But I don't think measuring it is necessary to make a decision here,
since it's an if statement that must find its way down to the CPU. It
can only add unecessary cost for those callers that will never pass an
out-of-bounds index.
I also don't like the API inconsistency that this would introduce to
FixedBitSet: currently all methods require index be in-bounds. This
is one of the improvements over OpenBitSet, which had some methods
requiring in-bounds and some not.
I don't think we should in any way feel compelled to replicate the
mistakes of Java's BitSet just because it's a common idiom / some
named pattern / taught in textbooks. Rather, it's Lucene that is
using this bit set impl, and so we should do what's best for Lucene.
> Fix FixedBitSet.nextSetBit/prevSetBit to support the common usage pattern in
> every programming book
> ---------------------------------------------------------------------------------------------------
>
> Key: LUCENE-3449
> URL: https://issues.apache.org/jira/browse/LUCENE-3449
> Project: Lucene - Java
> Issue Type: Improvement
> Components: core/other
> Affects Versions: 3.4, 4.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-3449.patch
>
>
> The usage pattern for nextSetBit/prevSetBit is the following:
> {code:java}
> for(int i=bs.nextSetBit(0); i>=0; i=bs.nextSetBit(i+1)) {
> // operate on index i here
> }
> {code}
> The problem is that the i+1 at the end can be bs.length(), but the code in
> nextSetBit does not allow this (same applies to prevSetBit(0)). The above
> usage pattern is in every programming book, so it should really be supported.
> The check has to be done in all cases (with the current impl in the calling
> code).
> If the check is done inside xxxSetBit() it can also be optimized to be only
> called seldom and not all the time, like in the ugly looking replacement,
> thats currently needed:
> {code:java}
> for(int i=bs.nextSetBit(0); i>=0; i=(i<bs.length()-1) ? bs.nextSetBit(i+1) :
> -1) {
> // operate on index i here
> }
> {code}
> We should change this and allow out-of bounds indexes for those two methods
> (they already do some checks in that direction). Enforcing this with an
> assert is unuseable on the client side.
> The test code for FixedBitSet also uses this, horrible. Please support the
> common usage pattern for BitSets.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
