Some performance improvements#5878
Conversation
mstange
force-pushed
the
push-trrquwtstqkw
branch
4 times, most recently
from
f48f421 to
f4e1be1
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5878 +/- ##
=======================================
Coverage 85.41% 85.42%
=======================================
Files 321 321
Lines 32036 32073 +37
Branches 8815 8821 +6
=======================================
+ Hits 27364 27398 +34
- Misses 4241 4244 +3
Partials 431 431 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Looks good to me, thanks! Fewer branches FTW!
src/profile-logic/profile-data.ts
Outdated
| * A helper function for creating a new StackTable. | ||
| * | ||
| * The caller passes in the prefix column of the new StackTable, | ||
| * and a bitset about which stacks to "discard". Then this function |
There was a problem hiding this comment.
"discard"
nit: The comment mentions that the bitset is about which stacks to discard, but the function takes keepStack bitset which is the inverted version.
There was a problem hiding this comment.
Oh, thanks for catching that.
| stackIndex === null | ||
| ? greyCategoryIndex | ||
| : stackTable.category[stackIndex]; | ||
| const category = samples.category[i]; |
There was a problem hiding this comment.
Yes, fewer branches, but more importantly less memory traffic. Now it reads one byte per sample instead of 16 bytes (8 bytes for the stack + 8 bytes for the category), and it reduces the number of dependent loads.
| // Derived from RawStackTable + FrameTable | ||
| category: IndexIntoCategoryList[]; | ||
| subcategory: IndexIntoSubcategoryListForCategory[]; | ||
| category: Uint8Array<ArrayBuffer>; // represents a Map<IndexIntoStackTable, IndexIntoCategoryList> |
There was a problem hiding this comment.
I wonder if it's worth documenting this new 256 limitation now. But I'm not so sure where. Ideally this should be a safe number, but let's see if there will be an importer that has a lot of categories that we don't know of 😄 But at least it's good that we are showing a console error.
There was a problem hiding this comment.
Yeah, I'm hoping that our hardcoded set of category colours makes it so that people aren't tempted to create a high number of categories; lots of them would end up with the same colour.
Create the newPrefixCol first, and then, once we know the new stackTable length and the remapping, compute the other columns from the remapping.
…le, and use the new column during activity graph drawing.
We are iterating over the filtered samples, which were already constrained to the zoomed range. Checking the time again might be useful to handle error cases where samples not sorted by time and the range filtering didn't work properly, but I don't think that's useful either; _accumulateInBuffer will make sure to never write outside of its bounds.
mstange
force-pushed
the
push-trrquwtstqkw
branch
from
f4e1be1 to
23fea36
Compare
2 participants
Production | Deploy preview
This PR contains some changes which make it faster to change the selected call node in the call tree. Whenever the selected call node changes, we redraw the activity graph and we compute which information to display in the sidebar.