xiangfu0 opened a new pull request, #19587: URL: https://github.com/apache/pinot/pull/19587
## Problem The serialized-bitmap (`BYTES`) aggregation paths of `DISTINCT_COUNT_BITMAP` — the paths used when unioning pre-aggregated bitmaps such as star-tree `DISTINCT_COUNT_BITMAP` columns — fold every matching document's bitmap into a per-group accumulator with `RoaringBitmap.or()`. Eager `or()` recomputes container cardinality and re-normalizes container types on every union. Profiling a production-like workload dominated by `DISTINCT_COUNT_BITMAP` over star-tree pre-aggregated bitmaps attributed the large majority (~85%) of server execution samples to these unions. ## Change Switch the fold to RoaringBitmap's lazy union — the same primitive `FastAggregation` uses internally: - Container cardinality maintenance is skipped during accumulation, and array containers promote to bitmap containers past 1024 combined cardinality, so subsequent unions of hot containers become plain word-wise ORs. - The accumulator is repaired once at extraction: `extractAggregationResult()` / `extractGroupByResult()` call `repairAfterLazy()` before the bitmap escapes the function, so results, intermediate result types, and the wire format are unchanged. - `lazyor()`/`repairAfterLazy()` are `protected` in RoaringBitmap, so a small `org.roaringbitmap.RoaringBitmapLazyUnion` shim (package-placed for access, free of Pinot types) exposes them. `merge()` intentionally stays eager: lazy intermediates escaping into serialization or `getCardinality()` would be incorrect, and merged intermediates flow to many SSE/MSE consumers. `lazyor` was chosen over the faster `naivelazyor` deliberately: `naivelazyor` promotes accumulator containers to 8 KiB bitmap containers on first overlap, which for hash-spread bitmaps (hashed LONG/STRING values) or high-group-count group-bys can retain hundreds of MB transiently until extraction. `lazyor` keeps retention bounded near eager levels. ## Benchmark New `BenchmarkDistinctCountBitmapAggregation` (JMH, included in this PR) drives the real aggregation function over 10K serialized bitmaps per op and verifies the result each invocation: | Value universe | Values per bitmap | master (ops/s) | this PR (ops/s) | Speedup | |---|---|---|---|---| | 2M (dense accumulator) | 200 | 16.1 | 67.0 | 4.2x | | 2M (dense) | 2000 | 25.2 | 37.8 | 1.5x | | 100M (sparse accumulator) | 200 | 0.95 | 1.37 | 1.4x | | 100M (sparse) | 2000 | 0.15 | 1.08 | 7.2x | Standalone fold benchmarks additionally show allocation is ~flat for dense accumulators; the sparse regime trades transient young-gen allocation (pre-promotion array copies) for its CPU win. ## Testing - New `DistinctCountBitmapLazyUnionTest` covers all three serialized-bitmap paths (`aggregate`, `aggregateGroupBySV`, `aggregateGroupByMV`) with cardinalities crossing the lazy container-promotion thresholds, multi-block accumulation, input reuse across group keys, and serialization round-trip of extracted results, verified against eagerly unioned references. - Existing `DistinctCountBitmapStarTreeV2Test`, `DistinctCountBitmapWithMVStarTreeV2Test`, `DistinctCountBitmapMVAggregationFunctionTest`, and `DistinctCountBitmapQueriesTest` all pass. No config changes, no API changes, no backward-incompatible changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
