FrankChen021 opened a new pull request, #20450: URL: https://github.com/apache/druid/pull/20450
## Description On-heap incremental ingestion creates a new aggregator for every new rollup key. For expression-based aggregators, each factorization previously rebuilt the expression bindings and row-value selector even when the selector factory and memoized parsed `Expr` were unchanged. This repeated work is significant for ingestion workloads with many rollup keys and expression metrics. The investigation also found an existing correctness issue in `RowBasedExpressionColumnValueSelector`: transformed expressions were cached using only the hash code of the array-binding list. Distinct binding lists such as `["Aa"]` and `["BB"]` have the same Java hash code and could therefore reuse the wrong transformed expression. <details> <summary>Profile</summary> <img width="738" height="651" alt="Profile" src="https://github.com/user-attachments/assets/39c564a6-829f-465d-8fbc-9ddd3cf3c384"; /> </details> ## Changes - Add an `ExprEvalSelectorCache` contract and implement it in `OnheapIncrementalIndex.CachingColumnSelectorFactory`. - Cache completed `ColumnValueSelector<ExprEval>` instances by the identity of the original parsed expression. - Keep aggregator instances and their mutable aggregation state independent; only the row-value selector is shared. - Use explicit `get`, construct, and `put` operations because selector construction can re-enter the same factory while resolving input columns. - Change the transformed-expression cache from a hash-code key to the complete array-binding list, allowing normal list equality to distinguish hash collisions. - Add regression coverage for selector reuse, independent aggregator state, expression ingestion with rollup, changing scalar and array bindings, and colliding array-binding hashes. - Add `ExpressionIngestionBenchmark` as a reproducible JMH benchmark for this ingestion path. ## Scope - Selector caching is limited to the on-heap incremental-ingestion selector factory. - Query-time selector factories are unchanged. - `IncrementalIndex.add` remains single-threaded or externally synchronized; this change does not introduce concurrent selector sharing. - `ExpressionLambdaAggregatorFactory` is unchanged because it plans its expressions directly. - The transformed-expression cache fix is included so selector reuse exercises the correct combined path. - The benchmark covers incremental-index ingestion with many new rollup keys and expression metrics. It does not measure Kafka networking, JSON parsing, compression, persistence, concurrent task activity, or end-to-end production ingestion throughput. - No release-note entry is required because the change affects internal ingestion implementation. - Local validation passed 41 focused tests, Checkstyle, benchmark compilation and packaging, and `git diff --check`. ## Benchmark `ExpressionIngestionBenchmark` measures on-heap ingestion of 100,000 rows into 25,000 rollup keys using three expression metrics. Input-row construction is excluded from the measured invocation. Each variant was measured with JMH 1.37 on JDK 25 using three JVM forks, three single-shot warmup iterations, seven single-shot measurement iterations, a 1 GiB heap, and the JMH GC profiler. | Variant | Mean elapsed | Approx. throughput | Allocation | |---|---:|---:|---:| | PR merge-base `d2bff3c9ec` | 364.981 ± 16.502 ms | 273,987 rows/s | 985.00 MiB/op | | Expression-selector cache `abe0fc5847` | 129.557 ± 4.759 ms | 771,860 rows/s | 162.00 MiB/op | Selector caching reduced elapsed time by 64.5%, increased approximate throughput by 2.82x, and reduced allocation by 83.6% in this workload. -- 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]
