fix union perf regression - #3003
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f308643375
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // remove the docset, it has been entirely consumed. | ||
| return true; | ||
| } | ||
| scorer.advance(); |
There was a problem hiding this comment.
Stop when advance reaches TERMINATED
When a scorer's last document is within HORIZON of TERMINATED, horizon becomes greater than the sentinel (TERMINATED is i32::MAX, not u32::MAX). After this call advances past that last hit, doc() returns TERMINATED, which is still < horizon, so the next loop iteration treats the sentinel as another document and repeats forever. The previous code broke out immediately when advance() returned TERMINATED; this needs the same check before relying on the doc >= horizon guard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
a segment doesn't have that many docs
Fixes a union performance regression since 0.26 and another ~4% improvement on top (better assembly in
BufferedUnionScorer::advance)The regression was "caused" by a completely independent change (#2983)