1587165 - Consider removing the window-based approach for profile buffers

Status: NEW

(Core :: Gecko Profiler, task, P3)

Description

[Greg Tatum [:gregtatum]]

The profile buffers are circular, and old data is rolled off as new data comes in. At one time a windowing feature was adding where the buffer would grow as new data came in, but old data would be removed after a certain time period. This ended up causing performance problems as new buffers were re-allocated. The buffer growing mechanism was removed, but the windowing option was left in there.

At this point, I'm not sure how useful this feature is, since the data already rolls off when the buffer gets full, and the buffer sizes are fixed. In the new profiler popup, I never added UI for the windowing feature. I'm wondering if we should just not expose it at all, and remove the windowing feature.

This bug is for tracking both this decision, and to possibly to do the work to remove it.

Comment 1

[Markus Stange [:mstange]]

I remember coming across tons of profiles where all the interesting information was squeezed into the top right corner of the time line. In those profiles, there was an idle content process which contained samples that were multiple minutes old. This content process determined the time scale at which the profile was displayed. However, the threads that contained any interesting activity only had samples from the last few seconds. So their activity was taking up a very small percentage of the total timeline width.
I haven't seen many profiles of that shape recently. I think that's mostly because of the 20 second default window size.

Comment 2

[Greg Tatum [:gregtatum]]

Ah, that makes sense. I realize now why I've been seeing more of those profiles, since I have not been using the windowing through the profiler popup interface. It's probably worth adding it back then!

Comment 3

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

Now we only use this window to restrict the output if needed, right before starting the serialization:
https://searchfox.org/mozilla-central/rev/7cc0f0e89cb40e43bf5c96906f13d44705401042/tools/profiler/core/platform.cpp#2191-2195

It doesn't impact the buffer size, so the only reason to keep it would be if someone is sure, before they start the profiler, that they will only want the last X seconds. I'd say it would probably be rare enough that we'd want to get rid of it?
Markus' scenario in comment 1 just means the user may have to select a small range in the output profile after the facts, it doesn't seem that much work -- or am I missing or misunderstanding something?
Also, profiles can now be re-shared with just a selected sub-range, so it's easy to permanently remove boring sections.

I think the main use of this window, would be if we were able to grow the buffer accordingly. But I doubt we'll want that, as it would add overhead during these need-to-grow moments. (And there was an attempt, but it was backed-out, I can't remember why exactly.)

In the future, the work needed for Fission (meta bug 1559842) may in fact make this window more useful again: I'm thinking that if we had one giant buffer shared between all processes, it may be efficient enough to distribute different portions of this buffer to certain processes, to match the time window where possible, and maybe it would also be possible to grow that buffer as needed.
But I think we could still remove this option in the meantime, and re-introduce it if/when needed later on...