Add "drop samples with category" transform

[canova]

This is useful for cases where I want to drop all the "idle" category samples, so I can focus on non-idle samples. Or it can be useful for dropping the whole profiler overhead by removing the "profiler" category.

I would like to use this transform in the pq cli tool, so we can remove the idle category before finding the "top functions". Otherwise the idle category just pollutes the output of the cli a lot.

Example profile: deploy preview
Use the drop samples with category transform on the idle category above.

[codecov]

Codecov Report

❌ Patch coverage is 88.63636% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.34%. Comparing base (4308a73) to head (f202cbb).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/profile-logic/transforms.ts	90.62%	3 Missing ⚠️
src/components/shared/CallNodeContextMenu.tsx	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5943      +/-   ##
==========================================
- Coverage   85.37%   85.34%   -0.03%     
==========================================
  Files         322      322              
  Lines       32069    32159      +90     
  Branches     8814     8846      +32     
==========================================
+ Hits        27378    27446      +68     
- Misses       4260     4281      +21     
- Partials      431      432       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mstange]

Hmm, in the deploy preview, if I drop samples with category JavaScript, it also drops samples with category DOM where the stack contains a JS function.

[canova]

Oh, I'm currently removing all the samples if they contain the category anywhere in their stack. It makes more sense to drop them if that category is the leaf only.

[canova]

I guess I was following how drop-function does, and it also drops the samples if any of the frame in the stack contains it. I updated the code to look at leaf only now. Is it better?

[mstange]

It's definitely more in line with my expectations. But now it's a bit confusing that you apply the filter by clicking a call node which will often still be there even with the filter applied. But maybe it's good enough.

[canova]

Yeah, I think it's a reasonable until we have something better.

[mstange]

Hmm, it looks at the stack category after other transforms have been applied. Is that what we want? It might be... but look at this one for example:
https://deploy-preview-5943--perf-html.netlify.app/public/p2fc32xj20c900rgyq8car76010v9gvc6995ab8/flame-graph/?globalTrackOrder=m0eafhlkijcb6g7w9d51w4&hiddenGlobalTracks=0wdfwm&hiddenLocalTracksByPid=10715-1w3~10735-0~10741-0~10726-0~10723-0~10739-01~16424-0~16294-0~16367-0~14881-0~10725-0~13801-0w2~10924-0w3~10722-2~16556-01~16395-0~10731-0~14104-0&range=3017m585~3335370u6526&thread=s&transforms=cfs-8337~cfs-8352~mf-8337~dg-8&v=16

Here I've merged away the DOM stack nodes in two DOM samples and then dropped samples with category DOM. In the activity graph you can still see that those samples have "sample category" DOM - they're blue but not dropped. How do you feel about this?

[canova]

@mstange Hm that's a good question.

I agree that it's confusing a bit. But I also feel like there is no perfect solution because I can also see another problem on the other option.

Let's say that we changed this logic to always apply the drop-category transform on the non-filtered stacks. Then if I collapse a DOM category function (that contains non-DOM category leaf), and then apply this drop category transform, then we will continue seeing DOM category leafs in the flame graph. So the timeline will be correct, but this time the flame graph will look wrong.

For example I prototyped the other solution and applied these changes:

So technically this had a "Other" category leaf at first. But now, I see that after I collapsed it and dropped "DOM", I still continue to see DOM leaf.

So to me it sounds like the question is "Where do we want to lie? Timeline or flame graph?" And I would probably chose timeline here as I would prefer to keep the flame graph the real source of truth as this is where the analysis happen. What do you think?

I guess my initial implementation of dropping categories if they are anywhere in the stack (and not only in the leaf) would be kinda better as both the timeline and flame graph will have the same content. But again, probably that's not what we want either.

[mstange]

That's true. Tricky.

I'm still leaning towards keeping it consistent with the time line - the disconnect is pre-existing, and the timeline is more closely-connected to "samples" rather than "stacks" than the flame graph is, and this is a "drop samples" transform. Part of the problem here is that the user initiates the transform from the flame graph / call tree and not from the activity graph or something outside the stack views.

I would like to say "what you have is good enough, we'll fix it later" but then we may have to deal with broken links or complicated upgraders in the future...

Another option would be to have an "Include idle samples" checkbox, and only show it if we have an Idle category. How would you feel about that?

We've wanted an "Include off-cpu samples" checkbox (off-by-default) for a while, and I'm not sure how this would interact. Actually, maybe once we have an on/off-cpu flag per sample, we can upgrade profiles by looking at the Idle category, and then just rename the checkbox! What do you think?

[canova]

I think I like the idea of having a checkbox for including idle samples.

Currently I wasn't super happy with the way I use this transform to drop idle frames in the cli. I think having something like this would make it easier there too. And it also makes sense for the future when we want to toggle off-cpu samples. I'll close this PR and will open up a new one with that.