1812452 - Increase the minimum profiler buffer size

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P2)

Description

[Florian Quèze [:florian]]

The current minimum buffer size is 1MB. This might be fine for profiler unit tests, but is much smaller than we need to get anything useful out of a real profile. When for some reason the profiler is misconfigured (eg. bug 1753483), the 1MB profiles are useless.

I think we should fix the profiler configuration for these tests, but it would also make sense to bump significantly the minimum buffer size.

I thought this would be a trivial patch, but it turns out the code around this is quite complicated, and seems to calculate the chunk size based on the number of entries.

I got lost reading https://searchfox.org/mozilla-central/rev/e1b4aec181f4e630fd9b08ac29c1d00a56ae5eed/tools/profiler/core/platform.cpp#697-761
Still, it seems we could easily increase the value of scMinimumNumberOfChunks.

I think a minimum buffer size of 128MB would make sense, as I can't think of any real use case for a smaller buffer, but I could be convinced to have 64MB as the minimum.
I'm not sure if the minimum buffer size should be the same for the base profiler or if it should be lower there.

The current code has vestiges from the time when the buffer was allocated at once per process when starting the profiler.

• The help message still says the number of entries is "per process" (https://searchfox.org/mozilla-central/rev/e1b4aec181f4e630fd9b08ac29c1d00a56ae5eed/tools/profiler/core/platform.cpp#3541-3542).
• The comment where we currently set the minimum explains why we need to have 4 chunks, but that should be 4 chunks per process!

  // Ideally we want at least 2 unreleased chunks to work with (1 current and 1
  // next), and 2 released chunks (so that one can be recycled when old, leaving
  // one with some data).
  constexpr static uint32_t scMinimumNumberOfChunks = 4;

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1812452 - Clamp the ProfileBufferGlobalController entries as well r?florian

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1812452 - Increase the min profiler buffer limit to 128MiB r?florian

Depends on D181653

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68509d3cc6b1
Clamp the ProfileBufferGlobalController entries as well r=florian
https://hg.mozilla.org/integration/autoland/rev/1cca13c16a81
Increase the min profiler buffer limit to 128MiB r=florian,geckoview-reviewers,m_kato

Comment 4

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/68509d3cc6b1
https://hg.mozilla.org/mozilla-central/rev/1cca13c16a81