The MacOS and Windows profiler cores have a threading structure where one
thread ("sampler thread") collects information from a thread to be profiled
("samplee thread") by suspending it, getting its register state, unwinding its
stack, and then resuming it. This requires kernel-level primitives to perform
the suspend, get-registers and resume steps.
The Linux/Android core is different, because none of those three primitives
exist. Until now, data collection has been done by sending a SIGPROF to the
samplee, and collecting all relevant data within the signal handler. This has
a number of disadvantages:
(1) Current work to rationalise/clean up the threading structure of the
profiler is complicated by the need to reason about/verify two different
schemes.
In particular, the Tick call in the Windows and MacOS implementations will
produce its output on the sampler thread. In the Linux implementation
that is produced on the sampled threads.
(2) Dynamic verification results (primarily, absence of data races and
deadlocks) established for the Linux implementation are less likely to
carry over to the other two implementations, because the threading
structures are different.
(3) It causes a lot of duplicated code in platform-*.cpp. For example
SamplerThread::Run() in the -win32.cpp and -macos.cpp files are very
similar. Ideally all three could be merged into a single file with the
identical logic commoned up.
(4) Running lots of code -- the entire contents of Tick -- in a signal handler
isn't considered good practice. POSIX severely restricts the set of
functions we can safely call from within a signal handler.
This commit changes the Linux implementation by using semaphores to implement
the suspend and resume primitives, and moves the bulk of the data collection
work to the sampler thread. By doing this, it causes the Linux implementation
to have the same threading structure as the other two.
--HG--
extra : rebase_source : 675b6ef76915d164ed263b831dddd6ce0c0e97f3
There's no need to lock when calling Tick() on a local TickSample that uses a
fresh SyncProfile with its own fresh ProfileBuffer -- none of that data can
be touched by another thread.
--HG--
extra : rebase_source : aaabef89e8481758b566e6dd01e4bb61a5855b1a
Both of these functions are now trivial and identical in both ThreadInfo and
SyncProfile. This patch inlines and removes them.
--HG--
extra : rebase_source : 15fb7c1d4df9fbc80d8e671761b4aa1508845cac
It appears to be a remnant of a time when SyncProfile lifetimes were more
complex. Nowadays they are simple.
- profiler_get_backtrace() constructs a SyncProfile called |profile|.
|profile|'s mOwnerState is REFERENCED.
- profiler_get_backtrace() then calls BeginUnwind() and EndUnwind() on
|profile|. After the EndUnwind(), |profile->mOwnerState| is always OWNED.
- |profile| then is put into the returned ProfilerBacktrace. That
ProfilerBacktrace will destroy |profile| in its destructor because
ShouldDestroy() always returns true because mOwnerState is always OWNED.
The OWNER_DESTROYING and ORPHANED states are never used, and the whole
OwnerState type isn't necessary. This patch removes it and ShouldDestroy().
--HG--
extra : rebase_source : f1828b4a5d6b8f73245e666f457b93a24ca7266e
This patch does the following.
- Uses "entries" consistently for the name of the value that is obtained from
MOZ_PROFILER_ENTRIES and is the first argument to profiler_start(). (I.e. not
"entry" or "entrySize".)
- Removes variables (e.g. PROFILER_HELP) holding env var names and uses the
names (e.g. "MOZ_PROFILER_HELP") directly. Some of the names are already used
directly and I think the slight repetition isn't harmful. It's unlikely that
we'd want to change these names the way we might need to change a numeric
value, and they're perfectly descriptive.
- Changes the "MOZ_PROFILING_FEATURES" string in the weird Android-only startup
code to be "MOZ_PROFILER_FEATURES", for consistency.
- Renames gUnwindInterval and gProfileEntries as gEnvVarInterval and
gEnvVarEntries to make it clearer that they come from environment variables,
but otherwise are parallel to gInterval and gEntries.
- Puts entries before intervals in most places, to match the profiler_start()
argument order.
- Changes profiler_usage() so that (a) it always prints, no matter the
verbosity, (b) it exits at its end, and (c) doesn't double-print "Profiler: "
at the start of each line.
--HG--
extra : rebase_source : e5a0b1c48e390ada894c746f050f08ff5c241066
The |nsIFile*| one is only called by the |const nsACString&| one, so this patch
combines them.
--HG--
extra : rebase_source : d8338e88cef4799d95e590c056ab343d5a1c546a
profiler_get_gatherer() exposes ProfileGatherer to the outside world in a way
that makes future changes difficult.
This patch:
- Removes ProfileGatherer.h from the list of headers exported from the
profiler.
- Removes nsIProfiler.profileGatherer and nsProfiler::GetProfileGatherer().
- Replaces profiler_get_gatherer() with three new functions that provide
minimal but sufficient access to ProfileGatherer:
profiler_will_gather_OOP_profile(), profiler_gathered_OOP_profile(), and
profiler_OOP_exit_profile().
These functions provide access to the ProfileGatherer in a similar fashion to
the pre-existing functions profiler_get_profile_jsobject_async() and
profiler_save_profile_to_file_async()
This significantly reduces the size of the profiler's API surface.
--HG--
rename : tools/profiler/public/ProfileGatherer.h => tools/profiler/gecko/ProfileGatherer.h
extra : rebase_source : d8e06a1133d4098c3a214858d3ff2c4bdcd9f1f2
This removes the one use of gStartTime outside of platform*.cpp, which lets us
restrict its visibility to just that compilation unit.
--HG--
extra : rebase_source : bf7207572cba5c1a31b544ea73e783ecd559978a
PlatformStart() and PlatformStop() are currently responsible for setting and
clearing gIsActive, but it's better if we do it in profiler_{start,stop}().
The patch also does the following.
- Adds some missing emacs/vim modelines.
- Makes Platform{Start,Stop}() crash if they have failures. I'm not at all
confident that ignoring the errors as is currently done will result in
sensible behaviour, so brittleness is better.
--HG--
extra : rebase_source : b9ab8437f5b92f6a8993ba7677ecb74a321ce219
This patch mostly does formatting fixes.
It also removes some declarations from platform.h that are no longer necessary
now that platform-linux-android.cpp is in the same compilation unit as
platform.cpp (due to it being #include-d directly); this required reordering
some things.
--HG--
extra : rebase_source : d07ef71455885fe8f1414d87c261ca054989a6a8
This avoids the need for platform-linux-android.cpp to read gInterval off the
main thread in an awkward spot. It also makes platform-linux-android.cpp
more like platform-{win32,macos}.cpp.
--HG--
extra : rebase_source : c1c76a382d6373f9fd2e3f89a1e1f8fef9072257
There is another PlatformStop() call earlier in the function, and gIsActive is
always false by the time we reach the removed call, so it's dead code.
--HG--
extra : rebase_source : 3b358b6bef47d394d6d6bc76d1153ea38968919e
It's only being used in a boolean fashion, so this patch replaces it with a
boolean.
--HG--
extra : rebase_source : 91152dff81107070fa49b3984e1b6759e0cd6d20
This change means that all the relevant code is now within
platform-linux-android.cpp, which is nice.
--HG--
extra : rebase_source : 886a31005fdb67fae65e6f4209796973f1391244
- Don't bother checking gSampler in ProfilerSignalHandler. It is equivalent
to checking gIsActive and we do that at the top of the loop in
SignalSender(). There is no point repeatedly checking the same condition in
the middle of that loop; that just opens up the possibility of partially
complete samples where some threads are missing.
- Clear gCurrentThreadInfo in SignalSender() instead of in
ProfilerSignalHandler(). The effect is much the same, but this change means
gCurrentThreadInfo is both set and cleared in SignalSender(), i.e. on a
single thread, removing any need for Atomic<>.
--HG--
extra : rebase_source : 645d321de4cad6fdb32383b6f1d0c7cbe54308fc
We don't need OS now that the platform-*.cpp files are in the same compilation
unit as platform.cpp.
The patch removes the sleep functions because they are unnecessary indirection.
OS::Startup() is necessary, but the patch renames it PlatformInit() to match
Platform{Start,Stop}() and profiler_init(), from which it is called.
Specifically:
- platform-linux.cc -> platform-linux-android.cpp
- platform-macos.cc -> platform-macos.cpp
- platform-win32.cc -> platform-win32.cpp
Adding "android" to the first one is the most important part, because it makes
things clearer. The .cc to .cpp change is less important but I might as well do
it while I'm in here.
--HG--
rename : tools/profiler/core/platform-linux.cc => tools/profiler/core/platform-linux-android.cpp
rename : tools/profiler/core/platform-macos.cc => tools/profiler/core/platform-macos.cpp
rename : tools/profiler/core/platform-win32.cc => tools/profiler/core/platform-win32.cpp
extra : rebase_source : 371f91c4cd95e88e1723e192e68f16ba66965c8f
Currently we use the SPS_* macros in some places, but also use other ones like
__arm__ and ANDROID and XP_{WIN,MAC,LINUX}. This patch makes the profiler
consistently use the SPS_* macros and removes the V8_HOST_ARCH_* macros.
The patch also does the following.
- Cleans up some header inclusions, e.g. including pthread.h directly in the
files that use it, and removing some unneeded android/log.h inclusions.
- Removes an unused branch in SetSampleContext() -- we don't support ARM on
anything other than Android, and glibc 2.3 is ancient.
- Doesn't use SPS_* in PseudoStack.h because that would require exporting
PlatformMacros.h, which doesn't seem worthwhile.
Some things that aid the understanding of this patch.
- XP_LINUX and LINUX are both defined for Linux *and* Android.
- x86/Android is the only supported platform that doesn't define
HAVE_NATIVE_UNWIND.
- Every platform that defines USE_LUL_STACKWALK also defines
HAVE_NATIVE_UNWIND.
--HG--
extra : rebase_source : 561b708f9434cabd9c0e00d4f4bfdd53f7008670
It's defined if any of XP_{WIN,MAC,LINUX} are defined and the latter includes
Android as well. So it's defined on all the OSes the profiler supports.
--HG--
extra : rebase_source : 1fa9c1fb573a99375b477a048c0b4575ac1eeca0
They must be set before the call to MaybeSetProfiler(), which checks
gProfileThreads, and before the gTaskTracer check.
This patch fixes the failure of some threads (e.g. Compositor) to be profiled,
as well as allowing TaskTracer to start up again!
