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
The data-mutex lock assertion in the ThreadRegistration destructor was done too early, because that ThreadRegistration could still be publicly accessible through the ThreadRegistry at that time.
This assertion is now moved after `ThreadRegistry::UnregisterThread`, after which other threads shouldn't have access to this ThreadRegistration anymore.
This patch also adds a stress gtest that registers and unregisters a thread many times, and a separate thread attempts to access and lock the test thread data. It did trigger the previous assertion before it was moved.
Differential Revision: https://phabricator.services.mozilla.com/D128162
CLOSED TREE
This patch reduces the number of browser-cycles that a live-site test (cnn in this case) does from 25 down to 5. Furthermore, we also disable media autoplay for all live site tests. This change required an update to browsertime to prevent default prefs from being set and overriding our prefs.
Differential Revision: https://phabricator.services.mozilla.com/D127831
This patch reduces the number of browser-cycles that a live-site test (cnn in this case) does from 25 down to 5. Furthermore, we also disable media autoplay for all live site tests. This change required an update to browsertime to prevent default prefs from being set and overriding our prefs.
Differential Revision: https://phabricator.services.mozilla.com/D127831
This change adds a new lint `android-format` which enforces formatting of Java
code using google-java-format.
To run the lint simply run:
./mach lint -l android-format
This command also support automatically fixing all errors running by adding
--fix:
./mach lint -l android-format --fix
This change also removes all the formatting-related checkstyle checks which are
now implicitly enforced by the formatter.
Differential Revision: https://phabricator.services.mozilla.com/D127734
This was causing asan and dt failures because bindgen was computing the size of
MarkerSchema as 122 bytes instead of 144 bytes. This was causing a heap buffer
overflow. After making the std::vector an opaque type, it computes the size
properly as 144 bytes.
Depends on D127114
Differential Revision: https://phabricator.services.mozilla.com/D127956
Clone will make the ProfilerTime easier to use. And {add,subtract}_microseconds
implementations are helpful when you need to subtract/add some duration before
adding a marker. There is a similar code in the Webrender marker code, and this
will allow them to use the new API instead of some custom code.
Differential Revision: https://phabricator.services.mozilla.com/D127111
Clone will make the ProfilerTime easier to use. And {add,subtract}_microseconds
implementations are helpful when you need to subtract/add some duration before
adding a marker. There is a similar code in the Webrender marker code, and this
will allow them to use the new API instead of some custom code.
Differential Revision: https://phabricator.services.mozilla.com/D127111
For a long time two copies of the 'taskgraph' module have existed in parallel.
We've attempted to keep them in sync, but over time they have diverged and the
maintenance burden has increased.
In order to reduce this burden, we'd like to re-join the two code bases. The
canonical repo will be the one that lives outside of mozilla-central, and this
module will depend on it. Since they both have the same module name (taskgraph)
we need to rename the version in mozilla-central to avoid collisions.
Other consumers of 'taskgraph' (like mobile repos) have standardized on
'<project>_taskgraph' as their module names. So replicating that here as well.
Differential Revision: https://phabricator.services.mozilla.com/D127118
Clone will make the ProfilerTime easier to use. And {add,subtract}_microseconds
implementations are helpful when you need to subtract/add some duration before
adding a marker. There is a similar code in the Webrender marker code, and this
will allow them to use the new API instead of some custom code.
Differential Revision: https://phabricator.services.mozilla.com/D127111
This patch adds an installation step for vismet requirements in the mach browsertime setup. Without this change, the check would falsely report that the instlalation failed even though it never actually ran. The current code has the pillow/pyssim setup being done after the check, with this change we'll be able to use --install-vismet-reqs to have them installed during the setup.
Differential Revision: https://phabricator.services.mozilla.com/D126703
platform-linux-android.cpp:199:9: error: variable 'r' set but not used [-Werror,-Wunused-but-set-variable]
int r = sem_init(&mMessage2, /* pshared */ 0, 0);
^
platform-linux-android.cpp:206:9: error: variable 'r' set but not used [-Werror,-Wunused-but-set-variable]
not used [-Werror,-Wunused-but-set-variable]
int r = sem_destroy(&mMessage2);
^
Differential Revision: https://phabricator.services.mozilla.com/D126459
This removes the `@CommandProvider` decorator and the need to implement
mach commands inside subclasses of `MachCommandBase`, and moves all
existing commands out from classes to module level functions.
Differential Revision: https://phabricator.services.mozilla.com/D121512
This removes the `@CommandProvider` decorator and the need to implement
mach commands inside subclasses of `MachCommandBase`, and moves all
existing commands out from classes to module level functions.
Differential Revision: https://phabricator.services.mozilla.com/D121512
This only adds the API and then adds the profiler payload to the buffer. The
deserialization and streaming will happen in the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D124026
You can see the `mozilla::MarkerSchema` for the C++ counterpart. This Rust
struct simply wraps the C++ object and keeps the reference of it as RAII. This
heap allocates the inner C++ object but it's fine to do it here, because it's
we only create a MarkerSchema object at the end of a profiling session and it
happens once per marker type. It should be very rare.
Differential Revision: https://phabricator.services.mozilla.com/D124025
JSON writer will be used for the third and last marker API. This is needed
because we need to describe a marker payload struct on how to serialize it.
Differential Revision: https://phabricator.services.mozilla.com/D124024
This is the second API for the markers. This allows one payload as a text for
more information. See the PROFILER_MARKER_TEXT macro for the C++ counterpart.
Differential Revision: https://phabricator.services.mozilla.com/D124023
This is the first and simplest API for the markers. There will be two more
APIs in the following patches (add_text_marker and add_marker). You can see the
PROFILER_MARKER_UNTYPED macro for the C++ counterpart.
Differential Revision: https://phabricator.services.mozilla.com/D124022
MarkerStack object is a bit more complex in the C++ side. But we don't need a
complex object like that in here. A simple enum is enough to determine what
type of marker stack we are capturing. C++ side will be handling the capturing
of the stack.
Differential Revision: https://phabricator.services.mozilla.com/D124021
These structs are needed for the marker APIs. We also have the same structs as
the C++ classes. See `mozilla::MarkerTiming` and `mozilla::MarkerOptions`.
Differential Revision: https://phabricator.services.mozilla.com/D124020
This is a syntactical sugar to write the categories more easily. I prefer to
have this macro because this is the same syntax as the categories in the label
frames API.
Differential Revision: https://phabricator.services.mozilla.com/D124019
For some reason, I thought that `parse_callbacks(Box::new(CargoCallbacks))`
would also automatically adds the initial header files that are manually added.
But apparently `parse_callbacks` is only executed when the inner included
header files are found.
Differential Revision: https://phabricator.services.mozilla.com/D124017
Some profiler operations have moved from using the main profiler mutex, to their own finer-grained mutexes. So we need to expose whether these are also locked on the current thread, in particular so that nativeallocations will not be intercepted during that time, which could re-enter the profiler code, potentially causing deadlocks.
Differential Revision: https://phabricator.services.mozilla.com/D126069
In the following patch, profiler_is_locked_on_current_thread could allocate memory on some platforms (e.g.: While accessing the TLS on Linux, because the very first access on a thread allocates some memory for it.)
So we need to prevent the interception of memory allocations before calling profiler_is_locked_on_current_thread.
This is done by making the ThreadIntercept class RAII, so that as soon as it's created it already blocks nested interceptions, before calling profiler_is_locked_on_current_thread if necessary to potentially block even more interceptions.
This has the advantage of making ThreadIntercept safer overall, there is now only one way to use it: Create on the stack, and check `IsBlocked()`.
Other functions like `Block()` and `Unblock()` previously made it possible to do incorrect things.
We don't need the extra space of `Maybe` from the old `MaybeGet()` function anymore.
And ThreadIntercept itself is smaller than AutoBlockIntercepts and its reference to the old ThreadIntercept inside the Maybe.
Differential Revision: https://phabricator.services.mozilla.com/D126068
This removes the `@CommandProvider` decorator and the need to implement
mach commands inside subclasses of `MachCommandBase`, and moves all
existing commands out from classes to module level functions.
Differential Revision: https://phabricator.services.mozilla.com/D121512
Gerald Squelart
Frederik Braun
Andi-Bogdan Postelnicu
Andrew Halberstadt
Heitor Neiva
Nika Layzell
Mark Banner
Evgenia Kotovich
Daisuke Akatsuka
Alexandru Michis
Noemi Erli
Kimberly Sereduck
Mathew Hodson
Nihanth Subramanya
Renovate Bot
Mats Palmgren
Gregory Mierzwinski
Andrei Oprea
Agi Sferro
Nazım Can Altınova
Sylvestre Ledru
Kagami Sascha Rosylight
Sandor Molnar
Marian-Vasile Laza
Mirko Brodesser
Mitchell Hentges
Gabriele Svelto
Mike Hommey
criss
Alex Lopez
Steve Fink
Henrik Skupin
Butkovits Atila