-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Not maintain docBufferUpTo when only docs needed #14164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I had meant to try out something like that too. I left minor comments but the change looks good to me.
@@ -388,6 +388,7 @@ private enum DeltaEncoding { | |||
final boolean needsOffsetsOrPayloads; | |||
final boolean needsImpacts; | |||
final boolean needsDocsAndFreqsOnly; | |||
final boolean needsDocsOnly; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we do not need to track a separate variable as it should always be the same as needsFreq == false
(needsFreq
should always be true whenever positions or impacts are needed).
@@ -345,7 +345,7 @@ private enum DeltaEncoding { | |||
private int prevDocID; // last doc ID of the previous block | |||
|
|||
private int docBufferSize; | |||
private int docBufferUpto; | |||
private int docBufferUpto; // only makes sense for packed encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is a bit misleading: it makes sense for the bitset encoding, but in this case it's only the index into the freq buffer, not into the doc buffer.
The
docBufferUpTo
variable is mainly maintained to obtain the corresponding value of freq/pos buffer. We can avoid the maintaining when only docs needed.Result on
wikimediumall
: