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

pkg/query: Add fast map of strings to json marshaling #3564

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Conversation

brancz
Copy link
Member

@brancz brancz commented Aug 5, 2023

This has a ~82% improvement compared to the previously used method.

$ benchstat stdlib.txt ours.txt
name        old time/op  new time/op  delta
MarshalMap   412ns ± 1%    73ns ± 0%  -82.34%  (p=0.008 n=5+5)

Which results in an overall improvement of ~44% for real life profiling data that has labels:

$ benchstat old.txt new.txt
name                old time/op  new time/op  delta
ArrowFlamegraph-10   289ms ± 1%   161ms ± 1%  -44.23%  (p=0.008 n=5+5)

Let's profile live stream where we explored this: https://www.youtube.com/watch?v=4hZtHsOWKKk (turns out we incorrectly measured and the improvement was actually significantly larger than initially thought)

@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Aug 5, 2023

🤖 Meticulous replayed 50 user sessions and took 326 screenshots. Meticulous has not yet run on 2d6b35a of the main branch and so there was nothing to compare against.

If you recently setup Meticulous, this is expected. Meticulous will start reporting comparisons for new pull requests after the next commit to the main branch.

Last updated for commit 356bc4d. This comment will update as new commits are pushed.

This has a ~82% improvement compared to the previously used method.

```
$ benchstat stdlib.txt ours.txt
name        old time/op  new time/op  delta
MarshalMap   412ns ± 1%    73ns ± 0%  -82.34%  (p=0.008 n=5+5)
```

Which results in an overall improvement of ~44% for real life profiling
data that has labels:

```
$ benchstat old.txt new.txt
name                old time/op  new time/op  delta
ArrowFlamegraph-10   289ms ± 1%   161ms ± 1%  -44.23%  (p=0.008 n=5+5)
```
@brancz brancz merged commit 20e4c70 into main Aug 7, 2023
32 checks passed
@brancz brancz deleted the fast-map-json branch August 7, 2023 06:23
@metalmatze
Copy link
Member

Very cool! I knew this was eventually going to happen, but didn't expect it so soon 😃
Very happy that we are not using reflection anymore now!

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.

2 participants