Speed up flush of softdelete by intoBitset#14552
Conversation
|
I managed to get some numbers on this change: BTW, i added some docvalue fields in my benchmark. Then i'm seeing rather huge cost of virtual calls represented by
|
|
I made some effort to simplify the change, this is ready for review now. |
|
Hi @jpountz , may i have your opinion on this optimization? |
jpountz
left a comment
There was a problem hiding this comment.
I left some small comments but the change makes sense to me, nice optimization.
| // we need a scratch bitset because the param bitset doesn't allow bits to be cleared. | ||
| if (scratch == null || scratch.length() < upTo - offset) { | ||
| // intoBitSet is usually called with fixed window size so we do not do overSize here. | ||
| scratch = new FixedBitSet(upTo - offset); |
There was a problem hiding this comment.
Maybe use FixedBitSet#ensureCapacity?
| } | ||
|
|
||
| // we need a scratch bitset because the param bitset doesn't allow bits to be cleared. | ||
| if (scratch == null || scratch.length() < upTo - offset) { |
There was a problem hiding this comment.
Should we compare scratch.length() with bitSet.length() rather than upTo - offset? My concern is that there are multiple call sites that call intoBitSet with upTo=NO_MORE_DOCS, so this could create a giant bit set?
| @Override | ||
| public int docIDRunEnd() throws IOException { | ||
| return in.docIDRunEnd(); | ||
| } |
There was a problem hiding this comment.
Maybe extract this change to a separate PR and cover all wrappers?
| @@ -45,11 +45,12 @@ public final class FixedBitSet extends BitSet { | |||
|
|
|||
| /** | |||
| * If the given {@link FixedBitSet} is large enough to hold {@code numBits+1}, returns the given | |||
There was a problem hiding this comment.
This method is typically called with pattern like:
FixedBitSet bits = FixedBitSet.ensureCapacity(bits, doc);
bits.set(doc):
So the returned bitSet is required to hold 0(inclusive) to numBits(inclusive), which is numBits+1 bits.
Speed up the flush of softdelete by intoBitset.
relates: #14521