Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execution: rework binary operators, add set operatos #318

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

MichaHoffmann
Copy link
Contributor

@MichaHoffmann MichaHoffmann commented Sep 27, 2023

  • rewritten binary operators ( really no good reason; it just kinda happened )
  • added support for "and, or, unless" ( prepared support for native histograms but that will be followup )
  • fixed some bugs in our duplicate label detection logic
  • fixed minor bug in error reporting for hash collision on left side in a one-to-one matching

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-rework-binary-operator branch 4 times, most recently from 0e8edf3 to adecda9 Compare September 30, 2023 13:33
@MichaHoffmann
Copy link
Contributor Author

MichaHoffmann commented Sep 30, 2023

$ benchstat main.bench pr.bench
goos: linux
goarch: amd64
pkg: github.com/thanos-io/promql-engine/engine
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
                                                                │  main.bench  │              pr.bench               │
                                                                │    sec/op    │    sec/op     vs base               │
RangeQuery/binary_operation_with_one_to_one/new_engine-8          55.25m ± ∞ ¹   46.59m ± ∞ ¹  -15.67% (p=0.008 n=5)
RangeQuery/binary_operation_with_many_to_one/new_engine-8         129.8m ± ∞ ¹   111.2m ± ∞ ¹  -14.31% (p=0.008 n=5)
RangeQuery/binary_operation_with_vector_and_scalar/new_engine-8   103.8m ± ∞ ¹   103.5m ± ∞ ¹        ~ (p=0.841 n=5)
geomean                                                           90.62m         81.26m        -10.33%
¹ need >= 6 samples for confidence interval at level 0.95

                                                                │  main.bench   │               pr.bench               │
                                                                │     B/op      │     B/op       vs base               │
RangeQuery/binary_operation_with_one_to_one/new_engine-8          16.71Mi ± ∞ ¹   12.61Mi ± ∞ ¹  -24.58% (p=0.008 n=5)
RangeQuery/binary_operation_with_many_to_one/new_engine-8         42.33Mi ± ∞ ¹   31.45Mi ± ∞ ¹  -25.71% (p=0.008 n=5)
RangeQuery/binary_operation_with_vector_and_scalar/new_engine-8   28.22Mi ± ∞ ¹   28.23Mi ± ∞ ¹        ~ (p=0.690 n=5)
geomean                                                           27.13Mi         22.37Mi        -17.55%
¹ need >= 6 samples for confidence interval at level 0.95

                                                                │  main.bench  │              pr.bench               │
                                                                │  allocs/op   │  allocs/op    vs base               │
RangeQuery/binary_operation_with_one_to_one/new_engine-8          49.56k ± ∞ ¹   37.32k ± ∞ ¹  -24.70% (p=0.008 n=5)
RangeQuery/binary_operation_with_many_to_one/new_engine-8         91.59k ± ∞ ¹   69.45k ± ∞ ¹  -24.17% (p=0.008 n=5)
RangeQuery/binary_operation_with_vector_and_scalar/new_engine-8   55.46k ± ∞ ¹   55.46k ± ∞ ¹        ~ (p=0.841 n=5)
geomean                                                           63.14k         52.39k        -17.04%
¹ need >= 6 samples for confidence interval at level 0.95

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-rework-binary-operator branch 2 times, most recently from 239d11e to 337b02b Compare September 30, 2023 14:19
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-rework-binary-operator branch 3 times, most recently from 8374744 to 0ce81cb Compare September 30, 2023 14:35
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-rework-binary-operator branch 2 times, most recently from 2a4e078 to 005a074 Compare October 2, 2023 14:20
Copy link
Collaborator

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

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

Love the improvements, great work 👍

execution/binary/vector.go Outdated Show resolved Hide resolved
execution/binary/vector.go Outdated Show resolved Hide resolved
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Great improvement! LGTM

@yeya24 yeya24 merged commit d9ebdf8 into thanos-io:main Oct 2, 2023
6 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.

None yet

3 participants