`GetRunningTimesWithTightTimestamp` was running the native get-running-times functions in a loop until it could run it within a "fast enough" time (based on an initial quick benchmark); This was to associate a timestamp with CPU measurements as close as possible, to get a more reliable CPU utilization graph at the end.
But this loop condition meant that if the system was under high stress, the loop could potentially re-run many times, wasting way more resources than the resulting "tightness" warrants.
So instead of repeating until "fast enough", we repeat at most once, and take the best of both measurements.
In the worst case, CPU measurements and timestamps may be a bit far apart, but it just means that a spike of activity may be reported in the next time slice instead, but the cumulative amount of work will be correct overall.
Differential Revision: https://phabricator.services.mozilla.com/D129773
Changed wget output log granularity from 📣 to :giga: (only used in case the python library didn't cover all usecases)
Added python methods to async cache/download all required files for update verify
Modified Dockerfile requirements to include aiohttp python lib
Differential Revision: https://phabricator.services.mozilla.com/D129429
The user shouldn't set MOZ_PROFILER_SHUTDOWN to an empty string, it wouldn't work anyway.
So now there is an extra check for that, to avoid even attempting to write a profile when there is no actual filename.
Thanks to this, the parent process can now just re-set MOZ_PROFILER_SHUTDOWN to "" in its children, so that they won't try to output their own profile into the same file. (This used to mostly work, because the parent process was the last to write its profile; but anecdotal data shows this may not alwaybe be true.)
As a bonus optimization, this means that child processes don't waste time needlessly saving their profile to disk.
Differential Revision: https://phabricator.services.mozilla.com/D129237
Finally it all comes together!
Factored-out functions from previous patches are now used by the new functions, which work with streaming contexts, in order to prepare stacks are other things first, then read the profile buffer just once to extract all samples and markers, and finally output this data in the expected JSON format.
One trick, is that when working on "legacy" entries (mostly related to samples), we need to handle "modern" marker entries as we encounter them; this happens in EntryGetter, and also when reading compact stacks and same-samples.
Differential Revision: https://phabricator.services.mozilla.com/D128444
This refactoring is a bit more complex (to prepare for the next patch).
The body of StreamSamplesToJSON was moved to a function that takes a lambda, which will provide streaming parameters relevant to the thread id of each read sample.
For now, it's only used by StreamSamplesToJSON, so in this case the lambda only checks if the entry thread id is the expected one, and it provides the JSON writer and other things needed to write these samples.
One extra complexity is that the `ProfileSample sample` that was outside the loop is now removed -- because in later patches this code will handle samples from different threads, so they cannot all share this one sample object. Instead the information that must be carried over is in the streaming parameters (mPreviousStackState and mPreviousStack), so that they correspond to one thread each. But the overall logic is the same, apart from where this out-of-the-loop information lives.
And another difference is that old samples (before `aSinceTime`) are fully processed, and then discarded just before being output, because we need their information to be in the thread's UniqueStacks, in case it's used by a later same-as-before sample.
Differential Revision: https://phabricator.services.mozilla.com/D128443
ThreadStreamingContext contains all the information needed to output samples and markers relevant to one thread.
ProcessStreamingContext contains a list of ThreadStreamingContext's for all profiled threads. This way, when reading the profile buffer, it will be possible to send each sample&marker to the relevant JSON writer.
Differential Revision: https://phabricator.services.mozilla.com/D128442
StreamTraceLoggerJSON doesn't rely on ProfiledThreadData, so it can just be a file-static function.
The code hasn't changed at all, the function needed to be moved up the file, before its first use.
Differential Revision: https://phabricator.services.mozilla.com/D128440
The body of StreamSamplesAndMarkers has been factored out into a templated function, with some lambdas to cover the buffer-reading and streaming of samples and markers.
In a later patch, DoStreamSamplesAndMarkers will be used to also stream already-processed samples and markers.
Differential Revision: https://phabricator.services.mozilla.com/D128439
While processing the profile buffer, the EntryGetter's iterator is used to read some "modern" entries, but then the EntryGetter is left to skip these entries again until the next "legacy" entry (with `e.Next()`).
Now, after reading modern entries, the EntryGetter's iterator is updated to directly skip the already-read modern entries.
And in a later patch this will be in fact necessary, otherwise we would process this modern data twice!
Differential Revision: https://phabricator.services.mozilla.com/D128438
The start of StreamJSON is about setting up the UniqueStacks, it can be factored out.
In a later patch, this will be used to prepare the UniqueStacks for each thread in one loop, before processing the profile buffer.
Differential Revision: https://phabricator.services.mozilla.com/D128437
When it's constructed, UniqueStacks can now take a ProfilerCodeAddressService pointer, instead of having to add it separately.
It will be even more useful in a later patch, when constructing it as a struct member in one shot.
Differential Revision: https://phabricator.services.mozilla.com/D128434
Previously, DeserializeAfterKindAndStream would take a JSON writer and a thread id, using the thread id (if specified) to only output markers from that thread to the given writer.
Now, DeserializeAfterKindAndStream takes a lambda, which will be called with the thread id found in the marker, that lambda can either return null (nothing to output) or a pointer to a writer, in which case the marker is read and output to the given writer.
This makes DeserializeAfterKindAndStream more flexible, and will allow handling markers from different threads, each possibly being output to different writers.
Also, for simplicity the entry is now always fully read, so there is no need for the caller to do anything. The return bool is therefore unnecessary, and has been removed.
Differential Revision: https://phabricator.services.mozilla.com/D128433
The makes the graphs much more correct:
If there were multiple skipped samples, the front-end would graph a long diagonal line between the samples before and after the skipped ones, making it look like the value gradually changed during that time.
Instead, by forcing the output of values before a change, this shows a better graph, with a horizontal line while the value didn't change, and a more abrupt diagonal line between the last two sample times when the change actually happened.
Also always output the very last sample, to clarify the final value (in case it was the same as previous ones.)
Differential Revision: https://phabricator.services.mozilla.com/D128841
This patch adds generic checks in all JSON profiles, that "counters" look valid.
And in GeckoProfiler.Counters, there are new checks for manually-added counters.
WaitForSamplingState had to be moved up. It's more efficient and reliable than an arbitraty `PR_Sleep` to wait for samples to be taken.
Differential Revision: https://phabricator.services.mozilla.com/D128840
We have a check in the 'CodespellProcess' class to ignore errors that are
fixing single letter camelCase errors (since these tend to be used frequently.
Unfortunately, when using '--fix', these errors are fixed regardless as the
fixing happens with the codespell process. Since we increment the 'fix'
variable after the check happens, we don't even report that anything was
'fixed'.
Ideally we would find a way to prevent these types of errors from being fixed,
but for now this at will at least ensure that the user is notified that
something was modified.
Differential Revision: https://phabricator.services.mozilla.com/D128885
`profiler_add_marker()` now checks if the marker's target thread is actively being profiled. This is to prevent adding markers that would be discarded anyway, from taking CPU time to process, and from using space in the profile buffer.
This means that `profiler_thread_is_being_profiled(ProfilerThreadId)` must be used when a marker is intended for another thread, i.e., when it uses the MarkerThreadId option.
(Note: since baseprofiler::profiler_thread_is_being_profiled(ProfilerThreadId) is not available, baseprofiler::AddMarker cannot prevent markers targetted at non-profiled thread; There are none yet anyway.)
Differential Revision: https://phabricator.services.mozilla.com/D128576
If the profiler is paused, then really, threads are not *being* profiled.
profiler_is_active_and_unpaused() was added, to help with non-MOZ_GECKO_PROFILER builds.
(Note: baseprofiler::profiler_thread_is_being_profiled(ProfilerThreadId) is not possible to implement, but it's not needed anyway.)
Differential Revision: https://phabricator.services.mozilla.com/D128707
ProfilerThreadRegistry::WithOffThreadRefOr is to ProfilerThreadRegistry::WithOffThreadRef as
ProfilerThreadRegistration::WithOnThreadRefOr is to ProfilerThreadRegistration::WithOnThreadRef.
So it is similar to ProfilerThreadRegistry::WithOffThreadRef, but it returns whatever the callback returns if the thread id was found, otherwise it returns a fallback value.
Differential Revision: https://phabricator.services.mozilla.com/D128575
This moves the more expensive check for testing the file type until after we've found an instance of setTimeout.
This is quicker as the initial checks are for simple comparisons.
Differential Revision: https://phabricator.services.mozilla.com/D128594
This makes sure colorama stays in pylint_requirements.txt next
time someone regenerates it on a non-Windows machine. Since
colorama is only required by pylint on Windows.
Differential Revision: https://phabricator.services.mozilla.com/D128426