Simplify some code related to thread CPU deltas

[mstange]

There was a mismatch in whether computeMaxThreadCPUDeltaPerMs was called before or after thread CPU delta processing. It didn't make a difference in practice, but it's better to just always call it with the unprocessed values. This will avoid problems if we end up giving the processed values a different type.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.61%. Comparing base (6fa213a) to head (df6d128).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5265      +/-   ##
==========================================
- Coverage   88.62%   88.61%   -0.01%     
==========================================
  Files         308      308              
  Lines       28105    28095      -10     
  Branches     7621     7618       -3     
==========================================
- Hits        24908    24897      -11     
- Misses       2983     2984       +1     
  Partials      214      214              

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

[julienw]

This looks reasonable to me, but I'd very much like to have feedback from Nazım, because he knows a lot more about the CPU values than I do. Unfortunately he's now away until next year, I hope that's OK.

[julienw]

From my understanding:

• before we were treating "null" as "I don't know", and as a result we were, in a way, splitting the "I don't know" hole in half, with the first half using the previous value, and the second half using the next value.
• With your change, the "I don't know part" is transformed into a "0".

I don't know if that's OK. Especially I don't know how common it is to have null values and what that means. I'd like to have Nazim feedback on this, if that's OK?

[mstange]

After checking 1000 profiles, I have found the following:

• There can be more than one null value at the start of the array.
• In every checked profile, once you see a non-null value, the rest of the values are also non-null.

And the reason for multiple null values at the beginning appears to be that we never implemented threadCPUDelta in the base profiler! So during process startup, you get all nulls until the non-base profiler takes over. Example: https://share.firefox.dev/4iG7vsj

[mstange]

https://bugzilla.mozilla.org/show_bug.cgi?id=1756519

[mstange]

Given Florian's confusion in bug 1756519, I would say zeroing out the base profiler values is preferable over what we do today: It'll be more obviously-wrong and less plausible-but-wrong.

[julienw]

thanks for the investigation, this sounds good to me!

[mstange]

Actually, after talking to Florian more, I'm no longer convinced that setting the base profiler values to zero is the better outcome. Before, after. I think I'll just update this PR to avoid the behavior change.

[mstange]

I've removed the first commit. This has the effect that, since getMaxThreadCPUDeltaPerMs no longer looks at the processed CPU deltas, it might return a value that's lower than the maximum cpu delta per ms once the null substitution has happened:
Let's say you have this unprocessed data: [{ timeDelta: 1, cpuDelta: null }, { timeDelta: 10, cpuDelta: 10 }]
Then you replace the null with the 10.
Now the first value has { timeDelta: 1, cpuDelta: 10 }, i.e. 10 CPU per ms. getMaxThreadCPUDeltaPerMs will now just compute 1 CPU per ms.
I think that's ok because this situation is broken anyway.