Conversation
|
Locally, the perf impact on the stats_merge benchmark is: |
8324b4d to
e4f4dba
Compare
Signed-off-by: Adam Gutglick <adamgsal@gmail.com>
e4f4dba to
4e420d6
Compare
|
run benchmark stats_merge |
|
run benchmark sql_planner |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing adamg/stats-in-place-min-max (2fb0a43) to bc2b36c (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing adamg/stats-in-place-min-max (2fb0a43) to bc2b36c (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Which issue does this PR close?
Follow up of #20768.
Rationale for this change
Precision::min/maxallocates a lot of newScalarValues, and it can be done in place.While running the
sql_plannerbenchmark, it seems like for clickbenchStatistics::try_merge_iteris a significant part of the runtime, and this PR improves that part by about 20-25% locally.What changes are included in this PR?
Introduces a couple of of new internal functions to calculate the min/max of a
Precisionin-place.Are these changes tested?
Existing general tests, and a few new unit tests.
Are there any user-facing changes?
None