Skip to content

aggregation: pre-size the term map from the dictionary hint - #3252

Closed
wsxiaoys wants to merge 3 commits into
quickwit-oss:mainfrom
wsxiaoys:apol/q33-term-map-presize
Closed

wsxiaoys wants to merge 3 commits into
quickwit-oss:mainfrom
wsxiaoys:apol/q33-term-map-presize

Conversation

@wsxiaoys

@wsxiaoys wsxiaoys commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

HashMapTermBuckets ignored the map_init_value capacity hint, so high-cardinality terms aggregations grew the map through repeated rehashes. Add HashMapTermBuckets::with_capacity_hint (a capped FxHashMap::with_capacity_and_hasher pre-size) and use it for the top-level hash-map branches of build_segment_term_collector.

Context

Found while profiling ClickBench Q33 on Quickwit: hashbrown fallible_with_capacity 9.37% + reserve_rehash 9.25% + _rjem_je_large_ralloc 30.21%.

Two details shape the patch:

  • The hint is max_column_val + 1, which for the hash-map fallback is a raw column value (it can be u64::MAX), not the number of distinct terms. aggregation_bucket_limit bounds the entries actually kept, so the hint is capped at 65,536.
  • Only the top-level map is pre-sized. TermAggregationMap::new is also called once per parent bucket by prepare_max_bucket, so pre-sizing there multiplies the reservation by the parent cardinality — that OOM'd a two-level terms > terms aggregation in testing. new therefore stays lazy.

The aggregation memory guard is unchanged.

Test plan

  • cargo test -p tantivy aggregation
  • ClickBench Q33 A/B on Quickwit (E003, in progress)

The per-parent-bucket maps created by prepare_max_bucket must stay lazy: there is
one per parent bucket (up to the aggregation bucket limit), so a 65k-entry
reservation each multiplied the memory by the parent cardinality and OOM'd on a
two-level terms aggregation.
@wsxiaoys

wsxiaoys commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing: benchmarked this and the pre-size does not move the query.

ClickBench, 100M docs / 16 splits / 1 node, Q33 = terms(ClientIP, size 10, segment_size 100000000), 3 runs:

before after
Q33 hot (median) 8.498 s 8.367 s (-1.5%, inside run-to-run noise)
Q33 cold 9.353 s 8.532 s (GCS-bound)
_rjem_je_large_ralloc 30.2% 33.8% cold / 36.2% hot
reserve_rehash 9.3% 7.6% cold / 8.8% hot

The hint is the problem: map_init_value is max_column_val + 1, which for the hash-map fallback is a raw column value (ClientIP reaches ~4e9), not the number of distinct terms. Capped at 65,536 it only skips the first ~11 of ~16 doublings on the way to a ~2.8M-entry map, so the expensive reallocs remain. A throwaway probe with a 4M cap was no better (Q33 [9.84, 9.05, 9.35] s, large_ralloc still 29.8/35.8%), which says the allocator cost is not the map's growth at all.

The prepare_max_bucket finding is worth keeping in mind for any future map pre-size: TermAggregationMap::new is called once per parent bucket, so pre-sizing there multiplies the reservation by the parent cardinality (it OOM'd a two-level terms > terms aggregation).

@wsxiaoys wsxiaoys closed this Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant