Use `weight` values in CPU calculations in the timeline activity graph

[canova]

From Julien's message here: https://github.com/firefox-devtools/profiler/pull/4067#issuecomment-1145922828 Currently we calculate the activity graph height depending on the cpu ratio of each sample. But we do not include the weight values in these calculations.

I think we didn't think about the weight values before because they weren't used when we had the CPU delta values at the same time. But in the future we may have profiles with both of them included. So it's better to think about the weights and include them in the calculations as well.

[mstange]

One thing that makes thinking about this hard is that we use weight in many different ways.

• In memory profiles, the weight scales with the size of the allocated memory at a certain stack. In this case, we don't want it to affect thread CPU values: More memory doesn't mean more CPU usage.
• When diffing threads, one thread has weight -1 and the other +1. It is a bit unclear how to draw a proper "diffed" activity graph. I think a good solution for this would require a special code path, and can't just be solved by multiplying thread CPU deltas with weights.
• In perf profiles with long idle gaps, we can use the weight to avoid repeating the idle sample many times. But these weighted samples have a CPU delta of zero, so multiplying them with the weight would make no difference.

[canova]

@mstange Yeah, I agree. It's a bit tricky since we have multiple meanings. But at least we know what kind of meaning we use for weight array by looking at the weightType field: https://github.com/firefox-devtools/profiler/blob/94991fc27e5403a2fe197246fa702834b9508a26/src/types/profile.js#L128

• For memory profiles, we can directly ignore them by looking at this field and ignore the bytes.
• For diffing, it's indeed pretty tricky and we need to figure out the best approach.
• For perf profiles, it wouldn't make any difference for idle samples. But it could make a difference if we have other repeating samples maybe? I don't know if we would like to do this though.

[julienw]

Remember also that the memory data is in a different tables than the sample data. The allocation tables don't have CPU information anyway.

For diffing I agree: this is when we introduced this weight property (initially called duration), as a quick hack to generate these diffing tracks. That was then expanded to other meanings. I hope that with the work to copy the thread id information to merged tracks we could change the algorithm to use this new data.

[mstange]

I hope that with the work to copy the thread id information to merged tracks we could change the algorithm to use this new data.

Change the algorithm to use the new thread id data? Yes, I agree. But that seems orthogonal to the weight. Do you still think there's anything we need to do with the weight during activity graph drawing?

[mstange]

I'm going to close this issue because I don't think anything needs to be changed about the behavior. It's also currently marked as "bug" (which, according to the label's description, makes it "Very important to fix").