xiangfu0 commented on PR #19264: URL: https://github.com/apache/pinot/pull/19264#issuecomment-5758112399
Rebased onto latest master (`3b6be9c`) and pushed three profiling-driven optimization commits to the combine-time derive. Summary of what changed and current benchmark results. ## Optimizations (guided by JMH stack/GC profiling of the combine) Stack-profiling the high-cardinality derive showed most of its CPU in `ConcurrentHashMap` machinery (per-upsert `size()` counter sums, bin-lock blocking, `compute` overhead) rather than in the grouping-set projection itself: 1. **`2ce7dbd` — skip the per-upsert `ConcurrentHashMap.size()` check when the result size is unbounded.** `CHM.size()` sums a striped counter-cell array; calling it on every upsert of the (unbounded) derive table was ~4% of query CPU plus contention side effects. Benefits every multi-threaded no-ORDER-BY group-by, not just grouping sets. 2. **`1aaf34a` — partition the derive by grouping set into task-local plain HashMaps** instead of range-partitioning base groups into one shared concurrent table. Task key spaces are disjoint (every derived key carries its set ordinal), so the heavy merge work is contention-free; the disjoint maps are unioned with pure stores. Clone discipline for OBJECT intermediates is preserved (accumulator cloned on first store, re-cloned per merge), so AVG/DISTINCTCOUNT/percentiles stay exact. 3. **`8a36def` — cut allocation and re-hash overhead in the derive**: presize task maps (kills observed `HashMap.resize`), probe with a flyweight key (no key-array + `Key` allocation on the merge path), and adopt the largest task map as the result table's backing map (new `SimpleIndexedTable` constructor) so its keys are never re-hashed. Interleaved A/B per commit (high cardinality, 50 segments x 15k rows): CUBE_3 ~443 → ~408 → ~384 → ~359 ms; ROLLUP_2 ~270 → ~237 → ~223 ms. All 76 `GroupingSetsQueriesTest` cases, the `DeriveGroupingSetsTest` thread-count matrix, and `IndexedTableTest` pass on every commit. ## Current benchmark (head `8a36def`) `BenchmarkGroupingSetsQueriesSSE`, 50 segments x 15k rows, `LIMIT 10000`, base aggregation (default) vs `groupingSetsBaseAggregation=false` (per-row expansion). JMH AverageTime, 1 fork x 4 iterations; shared-VM environment so error bars are wide — treat ratios, not absolute times, as the signal. **Low / moderate cardinality (`EXP(0.5)`)** | Query | Expansion (ms) | Base aggregation (ms) | Speedup | |---|---|---|---| | `ROLLUP(D1, D2)` | 12.9 ± 4.7 | 4.9 ± 3.3 | **2.6x** | | `ROLLUP(D1, D2, D3)` | 24.2 ± 2.9 | 11.1 ± 4.0 | **2.2x** | | `CUBE(D1, D2, D3)` | 36.5 ± 4.9 | 11.9 ± 3.9 | **3.1x** | | `GROUPING SETS ((D1),(D2),(D3),(D1,D2),())` | 18.7 ± 9.8 | 9.5 ± 4.8 | **2.0x** | **High cardinality (`EXP(0.001)`, base groups ~ rows)** | Query | Expansion (ms) | Base aggregation (ms) | Ratio | |---|---|---|---| | `ROLLUP(D1, D2)` | 215.0 ± 35.3 | 220.8 ± 28.0 | 0.97x (parity) | | `ROLLUP(D1, D2, D3)` | 286.5 ± 16.5 | 222.5 ± 38.4 | **1.29x** | | `CUBE(D1, D2, D3)` | 451.2 ± 94.7 | 368.3 ± 72.6 | **1.22x** | | `GROUPING SETS ((D1),(D2),(D3),(D1,D2),())` | 238.8 ± 29.9 | 162.9 ± 24.1 | **1.47x** | **Takeaway:** low/moderate cardinality keeps the ~2-3x win, and high cardinality — previously the parity/regression regime for this run shape — is now a win for base aggregation on 3 of 4 shapes (parity on the fourth). Closing profile shows the derive is down to ~10% of query CPU with no concurrency overhead; the remaining time is engine-generic (Object[] key hashing across all group-by tables, `DataTableBuilder` serialization, broker reduce) and scales with derived-row count, for which the opt-in `groupingSetsServerTrimSize` remains the lever. -- 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]
