1586618 - Profile serialization should not include entries added after the serialization starts

Status: NEW

(Core :: Gecko Profiler, task, P2)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thanks to bug 1576555, markers may now be added at any time, independent of the SamplerThread::Run() loop.
Markus noticed it IRL: "I just grabbed a profile that has a marker on the compositor thread which goes from 60ms after the last sample to 200ms after the last sample."

Previously, because markers were only collected as part of the sampling loop, and because the sampler was blocked during the whole serialization, no extra markers could be recorded after the last sample.

Now the serialization still blocks the sampler, but it only blocks the (separately-mutexed) ProfileBuffer storage while accessing each part of the buffer, which means markers could sneak in. E.g.:

1. Serialization blocks the buffer, reads thread #0 samples, unblocks;
2. Markers are added;
3. (repeat 1 and 2 for each recorded thread)
4. Serialization blocks the buffer, reads thread #0 markers (including late ones), unblocks;
5. Markers are added (but those from thread #0 will now be ignored);
   etc.

I can think of a few options:
A. Pause the profiler, so that new markers may not be added. We would actually lose them, but in practice the profiler is stopped afterwards anyway so it shouldn't matter.
B. Block the buffer during the whole serialization. So markers would just be delayed, but this would also block these threads for a fairly long time, which is not very user-friendly; serialization is a rare and special operation that few people will use, so maybe it's not a big issue.
C. Record the timestamp of the start of serialization, and ignore any record added after that. It doesn't block threads, and it doesn't discard later entries.
D. Let the frontend deal with this extra data!

Option 'C' seems better, but it makes me realize that in the worst case (high volume of markers during serialization) markers could trample over the start of the buffer, meaning that each thread would lose early samples&markers compared to previous threads.
So options 'A' and 'B' may actually be better overall; the choice will depend on which trade-off we prefer: Lose late data, or block the browser.

Note that bug 1577658 "Consider reading ProfileBuffer once to generate the JSON profile" would effectively feel like option B, blocking the buffer during the whole serialization, but hopefully making it faster overall.