goankur commented on PR #16806:
URL: https://github.com/apache/lucene/pull/16806#issuecomment-6587603704
Thanks @msokolov for the review and for the `IntHashSet` nudge.
I treated AI like a fast pair programmer and kept the design calls. The
hotspot came from profiling a **large merge** where clearing the visited bit
set took about 60% of the graph workers' time, which surprised me too.
After your suggestion, I measured both versions on a real merge before
choosing. IntHashSet came out ahead (5.1x vs 4.3x over main), and it is
simpler. From there I mostly pushed for less:
- I reused the existing `expectedVisitedNodes < graphSize >>> 7` rule, so
behavior only changes where SparseFixedBitSet was used before.
- I kept BitSet's set / getAndSet / clear, so the search loops did not
change.
- I kept two implementations, not three. Keeping SparseFixedBitSet as well
would put three types behind a hot call that queries and merges share.
- I specifically asked for tests that are short and check which set each
caller gets.
- Lastly, I asked for benchmarks that look like real merges: ids spread over
the whole segment, one worker per core, plus luceneutil for queries.
**On an existing abstraction:**
I didn't find an exiting IntHashSet + FixedBitSet. The closest I could find
were BitSet.of (SparseFixedBitSet vs FixedBitSet, same >>> 7 threshold) and
DocIdSetBuilder (an int buffer that upgrades to a FixedBitSet). Both build a
set once, while a visited set is cleared before every search.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
