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

Improve filter profile data #4313

Merged
merged 8 commits into from
Feb 26, 2024
Merged

Improve filter profile data #4313

merged 8 commits into from
Feb 26, 2024

Conversation

thorfour
Copy link
Contributor

Mark fields as null instead of rebuilding record during filtering.

thor@thors-MacBook-Pro ~/.../parca/pkg/query % benchstat before.txt after.txt
name           old time/op    new time/op    delta
FilterData-10    5.00ms ± 2%    0.38ms ± 1%  -92.37%  (p=0.000 n=10+10)

name           old alloc/op   new alloc/op   delta
FilterData-10    6.28MB ± 0%    0.00MB ± 1%  -99.94%  (p=0.000 n=10+10)

name           old allocs/op  new allocs/op  delta
FilterData-10     3.91k ± 1%     0.07k ± 2%  -98.19%  (p=0.000 n=10+10)

Copy link

alwaysmeticulous bot commented Feb 20, 2024

✅ Meticulous spotted zero visual differences across 379 screens tested: view results.

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

@thorfour
Copy link
Contributor Author

Updating it to slice the records instead of marking full rows as null

name           old time/op    new time/op    delta
FilterData-10    5.00ms ± 2%    0.39ms ± 2%  -92.13%  (p=0.000 n=10+10)

name           old alloc/op   new alloc/op   delta
FilterData-10    6.28MB ± 0%    0.09MB ± 0%  -98.55%  (p=0.000 n=10+9)

name           old allocs/op  new allocs/op  delta
FilterData-10     3.91k ± 1%     0.18k ± 1%  -95.45%  (p=0.000 n=10+9)

It's slightly worse, but removes some of the footguns from just marking whole rows as null

Previously we were rebuilding the record when we performed a filtering
operation. This caused a lot of copying of data. Instead we now just
mark the filtered our values as null in the array bitmap.
name           old time/op    new time/op    delta
FilterData-10    5.00ms ± 2%    0.38ms ± 1%  -92.37%  (p=0.000 n=10+10)

name           old alloc/op   new alloc/op   delta
FilterData-10    6.28MB ± 0%    0.00MB ± 1%  -99.94%  (p=0.000 n=10+10)

name           old allocs/op  new allocs/op  delta
FilterData-10     3.91k ± 1%     0.07k ± 2%  -98.19%  (p=0.000 n=10+10)
This prevents us from having to skip over records later and other
footguns where we Sum which accesses the data directly and ignores nulls
Copy link
Member

@metalmatze metalmatze left a comment

Choose a reason for hiding this comment

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

Very nice!
Improving ergonomics seems the right way forward. Overall this performance so much better, so I'm fine paying that small price. 👍

@thorfour thorfour enabled auto-merge (squash) February 26, 2024 14:32
@thorfour thorfour merged commit a5cd852 into main Feb 26, 2024
33 of 35 checks passed
@thorfour thorfour deleted the improve-filterProfileData branch February 26, 2024 14:33
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

2 participants