Skip to content

Lazily allocate DocAndScoreAccBuffer in MaxScoreBulkScorer#16316

Merged
romseygeek merged 2 commits into
apache:mainfrom
romseygeek:maxscore/allocations
Jul 1, 2026
Merged

Lazily allocate DocAndScoreAccBuffer in MaxScoreBulkScorer#16316
romseygeek merged 2 commits into
apache:mainfrom
romseygeek:maxscore/allocations

Conversation

@romseygeek

Copy link
Copy Markdown
Contributor

This buffer is only used in certain code paths, so allocation in the constructor adds unnecessary costs.

This buffer is only used in certain code paths, so allocation in the
constructor adds unnecessary costs.
@romseygeek romseygeek requested a review from iverase July 1, 2026 10:35
@romseygeek romseygeek self-assigned this Jul 1, 2026

@iverase iverase left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iprithv

iprithv commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

thanks for the fix, @romseygeek.
Apologies.. this regression was introduced in #15971, where I replaced the cardinality() call with growNoCopy(INNER_WINDOW_SIZE) directly in the MaxScoreBulkScorer constructor to avoid a redundant bitset pass. while that helped the hot multi-essential-clause path, it caused unnecessary eager allocation of the large DocAndScoreAccBuffer arrays in many other cases (especially boolean queries with expensive two-phase subclauses, which affected some percolator queries).
I see that you have restored lazy allocation for the buffer while preserving the performance benefit. thanks again...Tagging #15971 for traceability.

@romseygeek

Copy link
Copy Markdown
Contributor Author

No worries @iprithv, this didn't get picked up in any of the nightly benchmarks and it only really effects some corner cases (like elasticsearch's percolator) that ought to be disabling bulk scoring in any case. I think this is probably still worth doing to make things slightly more efficient when we use the other code paths, but it isn't a serious regression. I'll put it into 10.6

@github-actions github-actions Bot added this to the 10.6.0 milestone Jul 1, 2026
@romseygeek romseygeek merged commit d87b4f3 into apache:main Jul 1, 2026
13 checks passed
@romseygeek romseygeek deleted the maxscore/allocations branch July 1, 2026 14:15
romseygeek added a commit that referenced this pull request Jul 1, 2026
This buffer is only used in certain code paths, so allocation in the
constructor adds unnecessary costs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants