Two optimizations for the marker chart

[julienw]

Fixes part of #5120.

Commit 1: this uses a Uint32Array to hold the map of MarkerIndex to Marker Timing Row index. We use this information to decide which row we need to redraw when the user highlight a specific marker.

Initially I wasn't sure about it: when the user zooms in quite a bit, and there are just 3 markers left, then we still create a Uint32Array that can hold all markers. I was concerned about the amount of memory, and then decided this wasn't that much compared to all the other things, and kept it.

This computation is happening each time the user switches to the Marker Chart, because it's memoized in the Canvas component. We might want to move that to selectors? That would be a tradeoff because it uses a Weakmap currently (so this makes zooming in and popping a range fast, but switching between panels is slow -- although not so slow anymore).

Commit 2 and Commit 3: this makes the label computation lazy: now it's computed only when we actually need it. Because there's a minWidth to display text, this skips a lot of the computation.
This computation was done only once, the first time the marker chart is displayed, when computing the marker timing.

Other commits are about test updates and clean ups.

There are other issues when showing the marker chart for the first time. Especially related to GC. It would be good to measure with the native allocations feature, but this crashes on my computer.

deploy preview

Profile before
Profile after

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.43%. Comparing base (6b8d13c) to head (390fe35).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5121   +/-   ##
=======================================
  Coverage   88.43%   88.43%           
=======================================
  Files         304      304           
  Lines       27581    27581           
  Branches     7458     7458           
=======================================
  Hits        24390    24390           
  Misses       2963     2963           
  Partials      228      228           

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

[julienw]

I tried to use makeMarkerTimingSerializable on this list, but Flow was really getting in the way. Our type CombinedTimingRows isn't great because it doesn't have a string property to distinguish the 2 possible types in the union.

So after spending 1h on this I decided to go the easy way instead.

[mstange]

Thank you!

For the textGetter change, I fear that we might be cloning the function scope of getMarkerTiming for each marker, in order to store the markerIndex value on the lambda function object. We already have the markerIndex in the markerTiming index column, so I wonder if we could just store the getLabel function on the side. Then we would just carry around a single function for all markers, or maybe one per row, rather than one per marker. This would also make the test modification unnecessary.

But what you have is already a big improvement, so let's take it.

[julienw]

Thank you!

For the textGetter change, I fear that we might be cloning the function scope of getMarkerTiming for each marker, in order to store the markerIndex value on the lambda function object. We already have the markerIndex in the markerTiming index column, so I wonder if we could just store the getLabel function on the side. Then we would just carry around a single function for all markers, or maybe one per row, rather than one per marker. This would also make the test modification unnecessary.

Oh yeah, I thought about this possibility, but didn't think about the fact we may clope the full function scope for each marker. This doesn't sound great. I'll see if I can come up with this solution quickly otherwise I'll land this as is.

But what you have is already a big improvement, so let's take it.

[julienw]

This wasn't difficult so I did it, and then added some cleanup.

new profile => https://share.firefox.dev/3XqEiYU

[mstange]

Looks good! I didn't realize that the getter was always the same - this is simpler than I expected. I think I was confused by the fact that you can have user timing rows in the stack chart, but that's completely unrelated to the marker chart.