Reduce allocations during aggregations #16849
Merged
+493
−338
Conversation
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
|
Thanks! |
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
May 15, 2025
Reduce allocations during aggregations (duckdb/duckdb#16849)
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
May 15, 2025
Reduce allocations during aggregations (duckdb/duckdb#16849)
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
May 16, 2025
Reduce allocations during aggregations (duckdb/duckdb#16849)
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
May 16, 2025
Reduce allocations during aggregations (duckdb/duckdb#16849)
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
May 17, 2025
Reduce allocations during aggregations (duckdb/duckdb#16849)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR started off as a minor performance improvement to the string hashing function (to use a fixed
Loadinstead of a variable-sizememcpy) but ended up with a lot of effort to reduce allocations during aggregation, which I profiled usingheaptrack.In the current
main, when doingSELECT count(DISTINCT UserID) FROM hits;(ClickBench), DuckDB would do >700K allocations (this includes startup, which does a bunch of allocations, e.g.,SingleFileBlockManager::LoadFreeList()andInMemoryLogStorage::InMemoryLogStorage. Note that this is onc6a.metal(192 vCPUs).With the changes in this PR, the total allocation count is down to just >300K. Considering that this includes the startup, the total number of allocations done during aggregation has gone down significantly.
Note that this does not reduce RSS (it's about the same), but just the number of allocations, which should reduce contention as memory is a shared resource.
EDIT: I checked how many allocations there are if we don't do a grouped aggregation, i.e.
select sum(userid) from hits;, and it's ~141k. So initially, the query did ~708k allocations, of which 708 - 141 = ~567k were done by the grouped aggregation. Now, we're doing 316k allocations, of which 316 - 141 = ~175k are done by the grouped aggregation, which is a >3x reduction.