As mentioned in bug 1747354, the location of the dist directory is
relied to be $topobjdir/dist, so just use that consistently rather
than getting it from a separate variable for rust build scripts.
Differential Revision: https://phabricator.services.mozilla.com/D136556
* Define urlbarMarginInline and urlbarSearchButtonWidth as CSS rather than preprocessor variables
* Replace the urlbarViewPadding, urlbarViewFaviconWidth and urlbarViewIconMarginEnd preprocessor variables with CSS vars.
* Remove %ifdefs around the license comment in these 2 files
* Update ambiguous/out-of-date comment in the skeleton UI code
Differential Revision: https://phabricator.services.mozilla.com/D135962
* Define urlbarMarginInline and urlbarSearchButtonWidth as CSS rather than preprocessor variables
* Replace the urlbarViewPadding, urlbarViewFaviconWidth and urlbarViewIconMarginEnd preprocessor variables with CSS vars.
* Remove %ifdefs around the license comment in these 2 files
* Update ambiguous/out-of-date comment in the skeleton UI code
Differential Revision: https://phabricator.services.mozilla.com/D135962
Note: The atomic variable is named `gSkipSampling`, not mentioning forks because it could later be used in other situations, so it's best to describe it by the effect it has.
Differential Revision: https://phabricator.services.mozilla.com/D136205
`PrintUsageThenExit(code)` was supposed to exit when `code` was not zero, but:
- The name didn't reflect that, so it was confusing that `PrintUsageThenExit(0)` would *not* exit.
- The implementation in the Base Profiler exited anyway! This caused issues with some legacy code that still used the now-removed "threads" feature.
This patch renames the function to just `PrintUsage()` and never exits, leaving the caller to invoke `exit(code)` as needed -- with the added benefit that it's possible to exit with a zero code, useful in cases where an exit is not actually an error.
Differential Revision: https://phabricator.services.mozilla.com/D135666
I'm gonna guess there was no existing use of the tuple
serialization/deserialization code, because `Bytes` doesn't exist in the
deserializer, and cannot possibly function properly on tuple members that
serialize to a non-constant size, since it's called on a default-constructed
tuple.
This patch took inspiration in the deserializer for Variant and seems to work
fine.
Differential Revision: https://phabricator.services.mozilla.com/D135028
I'm gonna guess there was no existing use of the tuple
serialization/deserialization code, because `Bytes` doesn't exist in the
deserializer, and cannot possibly function properly on tuple members that
serialize to a non-constant size, since it's called on a default-constructed
tuple.
This patch took inspiration in the deserializer for Variant and seems to work
fine.
Differential Revision: https://phabricator.services.mozilla.com/D135028
While mingw builds don't require user32 and advapi32 explicitly, it doesn't
hurt for them to be there (and they're required for clang-cl build).
Likewise, while clang-builds don't require uuid and userenv explicitly
because they're pulled in via #pragmas in the source code, mingw doesn't
support those #pragmas and needs them explicitly, which doesn't hurt the
clang-cl builds.
Differential Revision: https://phabricator.services.mozilla.com/D134737
While mingw builds don't require user32 and advapi32 explicitly, it doesn't
hurt for them to be there (and they're required for clang-cl build).
Likewise, while clang-builds don't require uuid and userenv explicitly
because they're pulled in via #pragmas in the source code, mingw doesn't
support those #pragmas and needs them explicitly, which doesn't hurt the
clang-cl builds.
Differential Revision: https://phabricator.services.mozilla.com/D134737
This is necessary, because the next patch will remove the "threads" feature, and some tests only add that one feature so now they will have an empty feature list, equivalent to a feature bitset of 0 (zero).
Differential Revision: https://phabricator.services.mozilla.com/D134136
This patch was r+ed before by Ted, but it never landed because I initially intended
to address Ted's review comment (about making it work on 10.11 and below), and
because it needed to be rebased around bug 1374888.
The rebase turned out to be really simple, and Ted's review comment no longer applies
because Firefox no longer runs on 10.11 and below.
Profile with fix: https://share.firefox.dev/3oYzvO6
Differential Revision: https://phabricator.services.mozilla.com/D134008
The main thread is the busiest, so it benefits the most from having its own chunked buffer. This removes one allocation when capturing a marker stack on the main thread of each process.
That buffer is allocated when the first profiler starts, and is destroyed when the last profiler stops.
Note: Further improvements are possible (e.g.: Pool of pre-allocated buffers, attempting to use a stack-based buffer, etc.), but they are more complex and will require more work in future bugs.
Differential Revision: https://phabricator.services.mozilla.com/D133725
The resulting code should be the same. The factored lambda will be called in the next patch with a different chunked buffer.
Differential Revision: https://phabricator.services.mozilla.com/D133724
The main thread is the busiest, so it benefits the most from having its own chunked buffer. This removes one allocation when capturing a marker stack on the main thread of each process.
That buffer is allocated when the first profiler starts, and is destroyed when the last profiler stops.
Note: Further improvements are possible (e.g.: Pool of pre-allocated buffers, attempting to use a stack-based buffer, etc.), but they are more complex and will require more work in future bugs.
Differential Revision: https://phabricator.services.mozilla.com/D133725
The resulting code should be the same. The factored lambda will be called in the next patch with a different chunked buffer.
Differential Revision: https://phabricator.services.mozilla.com/D133724
Historically, va_copy was not supported everywhere (notably, MSVC didn't
have it). That's not true anymore. We already have code such as
xpcom/base/Logging.cpp that uses it unconditionally.
Differential Revision: https://phabricator.services.mozilla.com/D133458
The main thread is the busiest, so it benefits the most from having its own chunked buffer. This removes all but one allocation when capturing a marker stack on the main thread of each process.
Note: Further improvements are possible (e.g.: Pool of pre-allocated buffers, attempting to use a stack-based buffer, etc.), but they are more complex and will require more work in future bugs.
Depends on D133724
Differential Revision: https://phabricator.services.mozilla.com/D133725
The resulting code should be the same. The factored lambda will be called in the next patch with a different chunked buffer.
Depends on D133723
Differential Revision: https://phabricator.services.mozilla.com/D133724
The profiler uses GetProcInfoSync to discover unregistered threads, and record their CPU utilization. This data is captured in markers on the main thread track.
On Windows, because this work takes much longer than the usual sampling interval, it is done on its own thread.
Differential Revision: https://phabricator.services.mozilla.com/D132213
When constructing the SamplerThread object, individual features were extracted from the feature set (a uint32_t), and passed as bools. This could be error-prone, wasteful, and a maintenance burden when more features are needed in some/all platform implementations (like the next patch adding a new feature with a Windows-specific impact).
So now the full feature set is given to the SamplerThread, which can then extract the features it requires on each platform.
Differential Revision: https://phabricator.services.mozilla.com/D132523
-Wshadow warnings are not enabled globally, so these -Wno-shadow suppressions have no effect. I had intended to enable -Wshadow globally along with these suppressions in some directories (in bug 1272513), but that was blocked by other issues.
There are too many -Wshadow warnings (now over 2000) to realistically fix them all. We should remove all these unnecessary -Wno-shadow flags cluttering many moz.build files.
Differential Revision: https://phabricator.services.mozilla.com/D132289
-bind_at_load was added in bug 1139036 because of a dead-lock on startup
in jemalloc3 when a) we used jemalloc3 by default on nightly b) macOS
10.10.3 was released.
Since then we upgraded to jemalloc 4, and eventually ... removed
jemalloc 4 in bug 1363992, which makes the -bind_at_load flag leftover
from this removal.
Differential Revision: https://phabricator.services.mozilla.com/D132453
New features cpuallthreads, stacksallthreads, and markersallthreads now allow the user to selectively profile non-selected threads for more information.
The gtest GeckoProfiler.FeatureCombinations is modified to test combinations of up to 3 of a set of features, to lower its cost, which allows adding the new features.
Differential Revision: https://phabricator.services.mozilla.com/D130011
New features cpuallthreads, stacksallthreads, and markersallthreads now allow the user to selectively profile non-selected threads for more information.
The gtest GeckoProfiler.FeatureCombinations is modified to test combinations of up to 3 of a set of features, to lower its cost, which allows adding the new features.
Differential Revision: https://phabricator.services.mozilla.com/D130011
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
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
`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
Fixes race between nsSocketTransport::Close and
nsSocketTransport::RecoverFromError called from OnSocketDetached.
Depends on D128183
Differential Revision: https://phabricator.services.mozilla.com/D128364
For the same reasons discussed in the previous commit, it's impractical
to join these threads on shutdown, and so we should suppress thread leak
reports for them.
Differential Revision: https://phabricator.services.mozilla.com/D128651
Looks like there are a couple of impacted sites - these temporary suppressions should clear up the tests until they can be addressed.
Differential Revision: https://phabricator.services.mozilla.com/D128222
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 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
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
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
Because string contents could be split in two separate chunks, the default ProfilerStringView deserializer needed to concatenate it together in an off-chunk buffer.
But now thanks to ProfileBufferEntryReader::ReadSpans, it is possible to know if the contents are in a single memory area inside one chunk (which should be the vast majority of cases), in which case the ProfilerStringView can just reference it using its internal std::string_view, which saves managing a separate buffer and copying data into it.
However this can only be done safely when the span is correctly aligned for the character type, which may not be the case for char16_t strings that must be even-aligned.
Differential Revision: https://phabricator.services.mozilla.com/D124430
Instead of always having to use `ReadBytes` to copy bytes out of the profile buffer into an external buffer, these functions provide one or two spans (pointer+size) pointing at the area, which can be used to look at the data without copying it, especially in the majority of cases where areas are fully inside one chunk and only need one span to address.
Differential Revision: https://phabricator.services.mozilla.com/D124429
`ProfilerStringView::Data()` would return a pointer to the start of the string, but there may not be a null terminator at the end!
To reduce the likelihood of misuses, that function has now been removed.
Instead, callers must now access the data through `AsSpan` or the `Span` conversion operator (which makes it easy to use with `NS_ConvertUTF16toUTF8` for example).
It was not an issue until now, because deserialized string would always be terminated when copied out of the profile buffer, but a following patch will add optimized code where the non-terminated string inside the buffer will be directly pointed at.
Differential Revision: https://phabricator.services.mozilla.com/D125027