Some more activity graph drawing perf improvements#5886
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5886 +/- ##
=======================================
Coverage 85.44% 85.44%
=======================================
Files 321 321
Lines 32061 32050 -11
Branches 8753 8836 +83
=======================================
- Hits 27393 27385 -8
+ Misses 4236 4233 -3
Partials 432 432 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
961c48b to
21b7660
Compare
21b7660 to
f90c13d
Compare
canova
left a comment
There was a problem hiding this comment.
Looks good to me with some nits. Thanks!
| ) => void; | ||
| readonly categories: CategoryList; | ||
| readonly sampleSelectedStates: null | Uint8Array; | ||
| readonly sampleSelectedStates: Uint8Array; |
There was a problem hiding this comment.
Nice!
Nit: Grepping for sampleSelectedStates: still shows some types that are left as null | Uint8Array;. Can you also convert them to Uint8Array?
| readonly filteredOutByTabPercentageAtPixel: Float32Array<ArrayBuffer>; | ||
| }; | ||
| // These Float32Arrays are mutated in place during the computation step. | ||
| // buffers[selectedState] is the buffer for the given SelectedState. |
There was a problem hiding this comment.
"for the given SelectedState."
This part was a bit difficult to understand for me. Then I remembered SelectedState was an enum that we have now. I feel like this could make it easier to understand for me "for the given SelectedState enum value."
There was a problem hiding this comment.
Good suggestions, fixed.
| readonly timeRange: Milliseconds; | ||
| }; | ||
|
|
||
| const SELECTED_STATE_BUFFER_COUNT = 4; |
There was a problem hiding this comment.
This is a bit magical, and separate from the SelectedState enum. I think we could add a comment to both here and SelectedState to make sure to update if we add a new value. Or we can move it near the enum.
I tried to see if there is a better way to get the const enum max value, but it seems like there isn't one.
There was a problem hiding this comment.
I agree it's not pretty. I'm going to leave it as-is now because the upcoming commits will change this around some more.
| samplesSelectedStates !== null && | ||
| samplesSelectedStates[i] === SelectedState.Selected | ||
| sampleSelectedStates !== null && | ||
| sampleSelectedStates[i] === (SelectedState.Selected as number) |
There was a problem hiding this comment.
Nit: Do we need these as number casts here and in other places in this commit?
There was a problem hiding this comment.
Without it, there are lint failures from https://typescript-eslint.io/rules/no-unsafe-enum-comparison/
f90c13d to
8cad8d9
Compare
Changes: [Markus Stange] Start using const enum (#5879) [Markus Stange] Some performance improvements (#5878) [Nazım Can Altınova] Add startLine, startColumn, sourceMapURL and rename uuid to id in source table (#5882) [Markus Stange] Reduce repetition in profile compacting code (#5885) [Markus Stange] Some more activity graph drawing perf improvements (#5886) [fatadel] Make network markers in the network panel sticky on click (#5884) [Markus Stange] Improve SampleGraph and HeightGraph performance (#5897) [Nazım Can Altınova] 🔃 Sync: l10n -> main (March 20, 2026) (#5899) And special thanks to our localizers: be: Andrei Mukamolau el: Jim Spentzos en-GB: Paul es-CL: ravmn ia: Melo46 sv-SE: Andreas Pettersson tr: Nazım Can Altınova
See individual commits.