Skip to content

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)

@alwaysmeticulous
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 disabled auto-merge February 26, 2024 14:32
@thorfour thorfour merged commit a5cd852 into main Feb 26, 2024
@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.

3 participants