Skip to content

execution: use regular map in binary expressions#589

Merged
fpetkovski merged 1 commit into
mainfrom
mhoffmann/execution-use-regular-map
Jun 11, 2025
Merged

execution: use regular map in binary expressions#589
fpetkovski merged 1 commit into
mainfrom
mhoffmann/execution-use-regular-map

Conversation

@MichaHoffmann
Copy link
Copy Markdown
Contributor

@MichaHoffmann MichaHoffmann commented Jun 11, 2025

The author of the umap package was also coauthor of the new go1.24 maps. There is no major regression here and it gets rid of a dependency:

$ go tool -modfile go.tools.mod benchstat benchmarks/main.out  benchmarks/pr.out 
goos: linux
goarch: amd64
pkg: github.com/thanos-io/promql-engine/engine
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
                                                      │ benchmarks/main.out │         benchmarks/pr.out          │
                                                      │       sec/op        │    sec/op     vs base              │
RangeQuery/binary_operation_with_one_to_one-20                 47.69m ± ∞ ¹   51.27m ± ∞ ¹  +7.49% (p=0.008 n=5)
RangeQuery/binary_operation_with_many_to_one-20                114.3m ± ∞ ¹   121.9m ± ∞ ¹       ~ (p=0.095 n=5)
RangeQuery/binary_operation_with_vector_and_scalar-20          91.39m ± ∞ ¹   93.33m ± ∞ ¹       ~ (p=0.421 n=5)
geomean                                                        79.27m         83.56m        +5.41%
¹ need >= 6 samples for confidence interval at level 0.95

                                                      │ benchmarks/main.out │          benchmarks/pr.out          │
                                                      │        B/op         │     B/op       vs base              │
RangeQuery/binary_operation_with_one_to_one-20                13.53Mi ± ∞ ¹   13.58Mi ± ∞ ¹       ~ (p=0.310 n=5)
RangeQuery/binary_operation_with_many_to_one-20               33.58Mi ± ∞ ¹   33.60Mi ± ∞ ¹       ~ (p=0.841 n=5)
RangeQuery/binary_operation_with_vector_and_scalar-20         29.65Mi ± ∞ ¹   29.69Mi ± ∞ ¹       ~ (p=0.421 n=5)
geomean                                                       23.80Mi         23.84Mi        +0.17%
¹ need >= 6 samples for confidence interval at level 0.95

                                                      │ benchmarks/main.out │         benchmarks/pr.out          │
                                                      │      allocs/op      │  allocs/op    vs base              │
RangeQuery/binary_operation_with_one_to_one-20                 47.25k ± ∞ ¹   47.34k ± ∞ ¹       ~ (p=0.151 n=5)
RangeQuery/binary_operation_with_many_to_one-20                81.78k ± ∞ ¹   81.78k ± ∞ ¹       ~ (p=0.730 n=5)
RangeQuery/binary_operation_with_vector_and_scalar-20          62.59k ± ∞ ¹   62.60k ± ∞ ¹       ~ (p=0.794 n=5)
geomean                                                        62.31k         62.35k        +0.06%
¹ need >= 6 samples for confidence interval at level 0.95

I also made "benchstat" a tool in the go.tools.mod modfile

Copy link
Copy Markdown
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Signed-off-by: Michael Hoffmann <mhoffmann@cloudflare.com>
@MichaHoffmann MichaHoffmann force-pushed the mhoffmann/execution-use-regular-map branch from d9f8941 to 160e405 Compare June 11, 2025 08:04
@fpetkovski fpetkovski merged commit d4a546e into main Jun 11, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants