Closed MichaHoffmann closed 8 months ago
If its too many changes at once we can split the PR too!
Can we check bench results
Can we check bench results
Can do in a bit, but ultimately we need to check for duplicates at every step since thats what prometheus is doing too roughly.
LGTM overall, I just wonder if we need to wrap the user defined operator too.
I think generally yes since we have no preconditions to prove otherwise!
No meaningful regressiosn in benchmarks:
$ benchstat benchmarks/new.out benchmarks/pr.out
goos: linux
goarch: amd64
pkg: github.com/thanos-io/promql-engine/engine
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
│ benchmarks/new.out │ benchmarks/pr.out │
│ sec/op │ sec/op vs base │
RangeQuery/vector_selector-8 66.25m ± ∞ ¹ 66.10m ± ∞ ¹ ~ (p=1.000 n=5)
RangeQuery/sum-8 51.74m ± ∞ ¹ 52.32m ± ∞ ¹ +1.12% (p=0.008 n=5)
RangeQuery/sum_by_pod-8 60.00m ± ∞ ¹ 60.48m ± ∞ ¹ ~ (p=0.310 n=5)
RangeQuery/topk-8 50.64m ± ∞ ¹ 52.33m ± ∞ ¹ +3.34% (p=0.032 n=5)
RangeQuery/bottomk-8 50.23m ± ∞ ¹ 52.47m ± ∞ ¹ +4.46% (p=0.008 n=5)
RangeQuery/rate-8 112.8m ± ∞ ¹ 107.0m ± ∞ ¹ ~ (p=0.151 n=5)
RangeQuery/subquery-8 184.6m ± ∞ ¹ 184.9m ± ∞ ¹ ~ (p=1.000 n=5)
RangeQuery/sum_rate-8 89.82m ± ∞ ¹ 93.80m ± ∞ ¹ ~ (p=0.690 n=5)
RangeQuery/sum_by_rate-8 107.0m ± ∞ ¹ 106.8m ± ∞ ¹ ~ (p=0.690 n=5)
RangeQuery/quantile_with_variable_parameter-8 129.3m ± ∞ ¹ 132.6m ± ∞ ¹ +2.56% (p=0.008 n=5)
RangeQuery/binary_operation_with_one_to_one-8 47.83m ± ∞ ¹ 48.19m ± ∞ ¹ ~ (p=0.222 n=5)
RangeQuery/binary_operation_with_many_to_one-8 119.2m ± ∞ ¹ 119.5m ± ∞ ¹ ~ (p=0.841 n=5)
RangeQuery/binary_operation_with_vector_and_scalar-8 100.24m ± ∞ ¹ 99.11m ± ∞ ¹ ~ (p=0.310 n=5)
RangeQuery/unary_negation-8 68.91m ± ∞ ¹ 69.75m ± ∞ ¹ ~ (p=0.222 n=5)
RangeQuery/vector_and_scalar_comparison-8 99.44m ± ∞ ¹ 99.22m ± ∞ ¹ ~ (p=1.000 n=5)
RangeQuery/positive_offset_vector-8 64.43m ± ∞ ¹ 64.06m ± ∞ ¹ ~ (p=0.841 n=5)
RangeQuery/at_modifier_-8 58.44m ± ∞ ¹ 57.52m ± ∞ ¹ ~ (p=0.310 n=5)
RangeQuery/at_modifier_with_positive_offset_vector-8 56.09m ± ∞ ¹ 55.94m ± ∞ ¹ ~ (p=0.690 n=5)
RangeQuery/clamp-8 85.71m ± ∞ ¹ 87.20m ± ∞ ¹ ~ (p=0.095 n=5)
RangeQuery/clamp_min-8 80.25m ± ∞ ¹ 80.98m ± ∞ ¹ +0.92% (p=0.016 n=5)
RangeQuery/complex_func_query-8 116.4m ± ∞ ¹ 126.0m ± ∞ ¹ +8.26% (p=0.008 n=5)
RangeQuery/func_within_func_query-8 111.9m ± ∞ ¹ 110.3m ± ∞ ¹ ~ (p=0.310 n=5)
RangeQuery/aggr_within_func_query-8 117.5m ± ∞ ¹ 116.4m ± ∞ ¹ ~ (p=0.548 n=5)
RangeQuery/histogram_quantile-8 338.8m ± ∞ ¹ 340.0m ± ∞ ¹ ~ (p=1.000 n=5)
RangeQuery/sort-8 68.22m ± ∞ ¹ 67.38m ± ∞ ¹ ~ (p=0.095 n=5)
RangeQuery/sort_desc-8 67.71m ± ∞ ¹ 67.65m ± ∞ ¹ ~ (p=0.690 n=5)
RangeQuery/absent_and_exists-8 47.78m ± ∞ ¹ 48.55m ± ∞ ¹ ~ (p=0.056 n=5)
RangeQuery/absent_and_doesnt_exist-8 1.113m ± ∞ ¹ 1.129m ± ∞ ¹ +1.45% (p=0.032 n=5)
NativeHistograms/selector-8 427.6m ± ∞ ¹ 426.1m ± ∞ ¹ ~ (p=0.690 n=5)
NativeHistograms/sum-8 502.1m ± ∞ ¹ 510.3m ± ∞ ¹ +1.64% (p=0.032 n=5)
NativeHistograms/rate-8 923.8m ± ∞ ¹ 979.8m ± ∞ ¹ ~ (p=0.548 n=5)
NativeHistograms/sum_rate-8 1.022 ± ∞ ¹ 1.013 ± ∞ ¹ ~ (p=0.548 n=5)
NativeHistograms/histogram_sum-8 866.1m ± ∞ ¹ 877.0m ± ∞ ¹ ~ (p=0.222 n=5)
NativeHistograms/histogram_count-8 862.5m ± ∞ ¹ 870.8m ± ∞ ¹ ~ (p=0.548 n=5)
NativeHistograms/histogram_quantile-8 553.2m ± ∞ ¹ 543.3m ± ∞ ¹ ~ (p=0.690 n=5)
NativeHistograms/histogram_scalar_binop-8 906.5m ± ∞ ¹ 903.8m ± ∞ ¹ ~ (p=0.548 n=5)
geomean 120.2m 121.0m +0.72%
¹ need >= 6 samples for confidence interval at level 0.95