1869876 - Fix gecko profiler config parsing for extra profiler run

Status: RESOLVED FIXED

(Testing :: Raptor, defect, P2)

Description

[Kash Shampur [:kshampur] ⌚EST]

Presently it does not look like gecko_profiler_* configs are properly parsed by the manifest parsing script when --extra-profiler-run is used.

It may be as simple as fixing the logic here and double checking the ini files (at least for sp3 and motion mark)

Comment 1

[Nazım Can Altınova [:canova]]

Attachment: Bug 1869876 - Parse the gecko profiler arguments properly for exra profiler run r?kshampur!,#perftest

Previously we were only parsing the gecko_profile_* arguments for
--gecko-profile and not for --extra-profiler-run. This patch changes these
argument parsing code to make sure that we parse the profiler arguments for
both cases properly.

Note that args.extra_profiler_run might be true for apps that we don't
support. That's why we also check that the app is either one of the
GECKO_PROFILER_APPS or TRACE_APPS.

Comment 2

[Nazım Can Altınova [:canova]]

Attachment: Bug 1869876 - Remove most of the "gecko_profile_entries" settings from browsertime toml files r?sparky,kshampur,#perftest

gecko_profile_entries has an interesting history. Entries setting in the
profiler sets the profiler buffer limit for the maximum entries that it can
hold to limit the memory usage of Firefox. Each entry is 8 bytes, so for
example, if we set the entries limit to 8000000, it means that it's 64MB
(8000000 * 8 / 1000 / 1000, or ~61MiB if you devide by 1024).

Previously, on the gecko profiler entries limit was set per-process to
capture a profile. Meaning that if you set it to 8000000, every process would
have 64MB each to spend. But since Bug 1612356, we have an app-global entries
limit, meaning that this entries limit is shared between all the processes.
If you put 8000000 there, it'll be used by all the processes and once it's
full, it's going to start discarding the oldest entries to write the newer
entries. Also before, we were allocating the whole buffer at the start of the
profiling session. But with Bug 1612356, we don't allocate anymore, meaning
that if we put a 1GB entries here, it will not allocate all of them at once. It
will gradually allocate and stop until it reaches its 1GB limit (which may
never reach).

So considering all of these. I think it's important to go over all of the test
toml files and remove the outdated gecko_profile_entries options. 64MB is
clearly not enough and we have a more sensible default of 500MiB in raptor
here:
https://searchfox.org/mozilla-central/rev/a2709c2f0e31b2ad1a7b6447117626a34063fe57/testing/raptor/raptor/browsertime/base.py#644

Also good to note that, before this patch all the extra profiler runs were
using 500MiB limit since these weren't parsed correctly and everything was
fine. So I think it's safe to use it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b884354e9e1
Parse the gecko profiler arguments properly for exra profiler run r=kshampur,perftest-reviewers
https://hg.mozilla.org/integration/autoland/rev/f496924d8b71
Remove most of the "gecko_profile_entries" settings from browsertime toml files r=kshampur,perftest-reviewers

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5b884354e9e1
https://hg.mozilla.org/mozilla-central/rev/f496924d8b71