perf: reuse key/arg scratch buffers in hash aggregation (#674) - #676
Open
iheitlager wants to merge 1 commit into
Open
perf: reuse key/arg scratch buffers in hash aggregation (#674)#676iheitlager wants to merge 1 commit into
iheitlager wants to merge 1 commit into
Conversation
hash_agg_find allocated a fresh Vec<u8> key buffer and Vec<Value> for key values on every row, and hash_agg_step allocated a fresh Vec<Value> for arguments every row. Reuse HashAggState-held scratch buffers via take/give-back instead: cleared and refilled per row, with clones only on the group-creation path (once per distinct group) rather than per row. Reduces read_group_by_agg from ~2.5x to ~2.26x oracle. spend: matched estimate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hash_agg_find(src/vdbe/hash_agg.rs:268) no longer allocates a freshVec<u8>key buffer /Vec<Value>key-values buffer per row — it reusesHashAggState-held scratch buffers via take/give-back, cloning intoGroupSlot/the indexHashMaponly when a row starts a new group.hash_agg_step(src/vdbe/hash_agg.rs:340) reuses a scratchVec<Value>for per-row aggregate arguments instead of allocating fresh each row; arguments are only read by reference so no clone is needed at all.read_group_by_aggimproves from ~2.5x oracle to ~2.26x oracle onbench_1mb.db.Test plan
cargo test --lib hash_agg(13 tests, all pass, unmodified)cargo test(full suite, 979+ tests, all pass)cargo clippy --lib --all-targetscleancargo bench --bench crud -- read_group_by_aggshows improvement (4.87ms → oracle 2.16ms, was 2.5x, now ~2.26x)spend: matched estimate
Closes #674