arunkumarucet opened a new pull request, #19056: URL: https://github.com/apache/pinot/pull/19056
## Motivation While benchmarking `DISTINCTCOUNT` on ClickBench data (10M rows, `UserID` LONG with ~1.53M distinct values) I profiled the server with async-profiler and found ~80% of CPU in `LongOpenHashSet` add/rehash/addAll, even though the query runs on the no-scan path where values come straight out of the dictionary — which is already sorted and duplicate-free for immutable segments. `NonScanBasedAggregationOperator.getDistinctValueSet()` hashes every dictionary value into a per-segment `LongOpenHashSet`, and the combine phase then hash-merges the per-segment sets. For high-cardinality LONG columns basically the whole query is spent re-deduplicating values the dictionary already deduplicated. ## Change Added `SortedLongDistinctSet` (a `LongSet` backed by sorted runs) and used it in the LONG branch of `getDistinctValueSet()`: - Per segment, dictionary values are copied into a sorted run with no hashing. Sortedness is verified during the copy (near-zero cost), so mutable/realtime dictionaries, which are insertion-ordered, fall back to sort+dedupe instead of being trusted blindly. - `merge()` during the combine phase just appends the other set's runs. - The actual union happens once, lazily, when the result is first read (count extraction or serialization): a multi-pass merge between two pre-allocated buffers on the query thread, with a query-termination check between passes. No per-merge garbage, no ForkJoin common pool. - Pending runs are eagerly compacted (with geometric backoff) once they cross an internal threshold, so peak memory stays proportional to the distinct count rather than to the number of segments when segments carry overlapping values. Only LONG is changed for now — it's the common high-cardinality case (IDs, timestamps). INT/FLOAT/DOUBLE keep the hash-set path; the same approach applies if profiling justifies it. The intermediate result still serializes through the existing `ObjectType.LongSet` layout (`size` + values), so nothing changes on the wire and mixed-version brokers/servers are unaffected. `DISTINCTSUM`/`DISTINCTAVG` and the SMARTHLL/SMARTULL under-threshold paths consume the same set through the `Set`/`LongSet` interfaces and are covered by tests. ## Numbers ClickBench `hits`, 10M rows / 10 segments, single server (M-series laptop), warm best-of-4: | query | before | after | |---|---|---| | `DISTINCTCOUNT(UserID)` (1.53M distinct) | 37ms | 19ms | | `COUNT(DISTINCT UserID)` | 37ms | 19ms | | `DISTINCTCOUNT(RegionID)` (3.6K distinct) | 1ms | 1ms | | `DISTINCTCOUNT(UserID)` group by RegionID | 66ms | 66ms (unchanged, scan path) | | `DISTINCTCOUNTHLL` / `SMARTHLL` / `HLLPLUS` | — | unchanged | Results are identical to the hash-set implementation (verified value-for-value, not just counts). An isolated microbenchmark of the union itself (10 runs matching the real per-segment cardinalities, ~1.59M values): balanced pairwise merge 2.8ms, k-way heap merge 7.3ms, concat+sort 2.3ms — pairwise multi-pass merging with reused buffers was chosen; the allocation reduction mattered more than the merge strategy (the old per-level allocations produced ~50MB of garbage per query and GC showed up at ~12% of profile samples, ~2% after). ## Testing - `SortedLongDistinctSetTest` (15 cases): single/multi runs, overlapping and disjoint merges, mixed merges with `LongOpenHashSet` in both directions, unsorted input fallback, merge-after-materialization, dedupe-slack handling, eager compaction (3x3M runs with an arithmetically known union of 7M), serde round-trip, and the unsupported-removal contract. - `NonScanBasedAggregationOperatorTest` (4 cases): the sortedness-detection loop against mocked sorted, unsorted, and duplicate-bearing dictionaries (the realtime/mutable case). - Existing `DistinctCountQueriesTest`, `DistinctQueriesTest`, `DistinctSum/AvgAggregationFunctionTest`, `SegmentPartitionedDistinctCountQueriesTest` all pass. - Additionally ran a randomized differential fuzz (200 trials, random run counts/sizes/overlaps/ranges) comparing size/iteration/contains/sum against `LongOpenHashSet` ground truth. -- 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]
