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

Regex tweak #2484

Merged
merged 3 commits into from
May 17, 2023
Merged

Regex tweak #2484

merged 3 commits into from
May 17, 2023

Conversation

mdisibio
Copy link
Contributor

@mdisibio mdisibio commented May 17, 2023

What this PR does:
Makes regex predicate faster. Particularly the case where there are found matches because of all the map access.

Tweak originally discussed #2410 (comment)

name                                                 old time/op    new time/op    delta
BackendBlockTraceQL/spanAttIntrinsicRegexNoMatch-12    2.51ms ± 2%    2.45ms ± 0%   -2.52%  (p=0.000 n=9+8)
BackendBlockTraceQL/spanAttIntrinsicRegexMatch-12      72.1ms ± 2%    49.5ms ± 2%  -31.36%  (p=0.000 n=9+10)

name                                                 old speed      new speed      delta
BackendBlockTraceQL/spanAttIntrinsicRegexNoMatch-12   385MB/s ± 2%   395MB/s ± 0%   +2.58%  (p=0.000 n=9+8)
BackendBlockTraceQL/spanAttIntrinsicRegexMatch-12     186MB/s ± 2%   271MB/s ± 2%  +45.69%  (p=0.000 n=9+10)

name                                                 old MB_io/op   new MB_io/op   delta
BackendBlockTraceQL/spanAttIntrinsicRegexNoMatch-12      0.97 ± 0%      0.97 ± 0%     ~     (all equal)
BackendBlockTraceQL/spanAttIntrinsicRegexMatch-12        13.4 ± 0%      13.4 ± 0%     ~     (all equal)

name                                                 old alloc/op   new alloc/op   delta
BackendBlockTraceQL/spanAttIntrinsicRegexNoMatch-12    1.10MB ± 4%    1.07MB ± 4%   -2.71%  (p=0.017 n=10+9)
BackendBlockTraceQL/spanAttIntrinsicRegexMatch-12      33.4MB ± 1%    12.5MB ± 0%  -62.73%  (p=0.000 n=10+7)

name                                                 old allocs/op  new allocs/op  delta
BackendBlockTraceQL/spanAttIntrinsicRegexNoMatch-12     12.9k ± 0%     12.8k ± 0%   -0.18%  (p=0.000 n=10+7)
BackendBlockTraceQL/spanAttIntrinsicRegexMatch-12        786k ± 0%       14k ± 0%  -98.22%  (p=0.000 n=10+10)

Which issue(s) this PR fixes:
Fixes n/a

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

nice. i didn't know about that zero alloc optimization on maps

@@ -474,10 +476,10 @@ func BenchmarkBackendBlockTraceQL(b *testing.B) {

ctx := context.TODO()
tenantID := "1"
blockID := uuid.MustParse("149e41d2-cc4d-4f71-b355-3377eabc94c8")
blockID := uuid.MustParse("2968a567-5873-4e4c-b3cb-21c106c6714b")
Copy link
Member

Choose a reason for hiding this comment

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

booooooooooooooo

Copy link
Member

Choose a reason for hiding this comment

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

149e41d2-cc4d-4f71-b355-3377eabc94c8 is joe's fav uuid 😆

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

nice. i didn't know about that zero alloc optimization on maps

@mdisibio
Copy link
Contributor Author

nice. i didn't know about that zero alloc optimization on maps

Neither did I 💡

@mdisibio mdisibio merged commit eeb9986 into grafana:main May 17, 2023
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.

3 participants