[
https://issues.apache.org/jira/browse/LUCENE-6427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14496287#comment-14496287
]
Adrien Grand commented on LUCENE-6427:
--------------------------------------
Thanks Luc. I have some questions/comments:
- Lots of javadocs mention "Depends on the ghost bits being clear!" (eg.
cardinality()) but since your PR checks that there are no ghost bits when
creating or growing the bit set, this is not something that consumers of these
APIs should worry about?
- Do we actually need the new "checkIfEmpty" method? I suspect checking the
return values of nextSetBit(0) value would be almost as fast?
- In unit tests, you should use the random() object instead of creating new
Random objects so that tests remain reproducible (when there is a test failure,
we know the seed that triggered this failure and running the tests with the
same seed reproduces the same sequence of random numbers)
At first I did not understand your changes on FixedBitSet.ensureCapacity but
looking at the impl it has weird semantics indeed. Thanks for fixing the docs
for now but in the longer term I think we should fix it to work more like
ArrayUtil.grow.
Also thank you for adding javadocs to undocumented methods!
> BitSet fixes - assert on presence of 'ghost bits' and others
> ------------------------------------------------------------
>
> Key: LUCENE-6427
> URL: https://issues.apache.org/jira/browse/LUCENE-6427
> Project: Lucene - Core
> Issue Type: Bug
> Components: core/other
> Reporter: Luc Vanlerberghe
>
> Fixes after reviewing org.apache.lucene.util.FixedBitSet, LongBitSet and
> corresponding tests:
> * Some methods rely on the fact that no bits are set after numBits (what I
> call 'ghost' bits here).
> ** cardinality, nextSetBit, intersects and others may yield wrong results
> ** If ghost bits are present, they may become visible after ensureCapacity is
> called.
> ** The tests deliberately create bitsets with ghost bits, but then do not
> detect these failures
> * FixedBitSet.cardinality scans the complete backing array, even if only
> numWords are in use
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
