It's fairly straightforward, and measures the important parts of:
- Sampler, PseudoStack, ProfileBuffer, ThreadInfo.
- LUL, PriMap, SecMap
Coverage isn't perfect, but it gets the major things I found via DMD on Linux.
Example output in about:memory:
├──151.21 MB (49.73%) -- profiler
│ ├──141.49 MB (46.53%) ── lul
│ └────9.72 MB (03.20%) ── sampler
--HG--
extra : rebase_source : 67d2ada42aead43f68f5100a08204a1d1f1cfceb
We only have one global Sampler, gSampler, and so SamplerRegistry is just an
unnecessary and obfuscating wrapper around it.
--HG--
extra : rebase_source : b675c414d0081dab2cd7dac864ffdff5070afe7d
StaticMutexes don't need to be created or destroyed, which avoids the need for
checking if they still exist.
This patch also adds locking to a few functions that lacked it. Every access to
sRegisteredThreads is now protected appropriately.
--HG--
extra : rebase_source : 2ea1d7372652ddd532e6e98c167be0997df33f07
Sampler::sRegisteredThreadsMutex is public. Might as well make
Sampler::sRegisteredThreads public too. This allows the getter to be removed,
and makes the code a little easier to read.
--HG--
extra : rebase_source : 4ff3ba92c9d74ae1b10a2be227ca15f61d15bee2
ThreadInfo contains a ThreadResponsiveness, and then ThreadResponsiveness has a
pointer back to its containing ThreadInfo, which is gross.
The back pointer is only needed for Update(), and it's easy to pass in the
necessary info instead via a new method UpdateThreadResponsiveness().
This change also means ThreadInfo::GetThread() can be removed.
--HG--
extra : rebase_source : 46ebeb142e8c678be204b106713147738bcbc4a4
ThreadInfo and ThreadProfile are hopelessly intertwined.
- ThreadInfo has an owning pointer to ThreadProfile. ThreadProfile has a raw
back pointer to ThreadInfo. A reference to one is as good as a reference to
the other, and references to one frequently reach into the other.
- An exception is SyncProfile, a sub-class of ThreadProfile, which instead has
an owning pointer to its ThreadInfo. (This makes the SyncProfile's destructor
dubious, because it deletes the ThreadInfo, which could conceivably re-call
into SyncProfile's destructor.)
- ThreadProfile also has copies of five ThreadInfo fields (mThreadId,
mIsMainThread, mPlatformData, mStackTop, mPseudoStack) even though it also
has direct ThreadInfo access via the back pointer.
The only good reason for having the two classes separate is that some
ThreadInfo objects have a null ThreadProfile pointer. But this doesn't justify
the entanglement.
So this patch merges ThreadProfile into ThreadInfo. It visually separates the
methods and fields related to profiles to partially preserve the original
meaning of the split. The new ThreadInfo::hasProfile() method replaces
ThreadInfo::Profile() as the indicator of whether a ThreadInfo has associated
profile data.
Notable points of simplification:
- The five duplicated fields are no longer duplicated.
- NewSyncProfile(), RegisterThread() no longer create ThreadProfile objects.
- ~SyncProfile() becomes trivial.
- ThreadInfo::SetPendingDelete() is simpler.
- Overall it removes ~80 lines of code.
Much of the rest is just plumbing changes.
--HG--
extra : rebase_source : 2e8c4cc46aa15943ffdc1fa19d9c829587267ee9
|sample| is non-null at all callsites. The function derefs |sampler| before
doing any of the null checks, anyway.
--HG--
extra : rebase_source : 5c3c917009310ade1faaf985ac866a4a98fe6abc
The patch also moves some Sampler methods from platform.cpp to Sampler.cpp. Now
all Sampler methods are in Sampler.cpp except for a small number of
platform-specific ones, which are in platform-*.cpp.
--HG--
rename : tools/profiler/core/GeckoSampler.cpp => tools/profiler/core/Sampler.cpp
extra : rebase_source : a13862dccfcb1c78567cc9eb22e92b8410d2e544
There's no point having them as separate classes. This removes the need for
some virtual functions, too.
--HG--
extra : rebase_source : b2607ba2431ae043b6e015f4f435b0d660b02d71
They're defined separately for each platform, but the definitions are almost
identical and can be commoned up.
--HG--
extra : rebase_source : 67b7f88410db1951a20f03f6c156aa6b35a3ee20
It's a clumsy cross-platform abstraction. GetCurrentId() and tid_t are the only
genuinely cross-platform things in it, and the patch keeps those in place.
The remaining methods are not implemented on all platforms (none are
implemented on Linux) and the fields are all used on either Mac or Windows, but
not both. So the patch moves the relevant Thread method implementations and
fields into the existing SamplerThread classes. There is sufficiently little
overlap between the two SamplerThread classes that there is no point sharing a
base class between them. This avoids the need for virtual functions, and
simplifies thread naming on Mac.
The patch also changes some of the existing code to use Gecko style, e.g.
|mFoo| instead of |foo_| for class fields.
This variable is set in GeckoSampler's destructor and cleared in GeckoSampler's
destructor, which means it's just another handle to the GeckoSampler singleton,
which means it duplicates gSampler.
This patch replaces its uses with gSampler uses. This exposes various places
where GeckoSampler is being accessed off the main thread, which I have marked
with XXX comments.
--HG--
extra : rebase_source : 3c2d012b4d38d353022f384c31a528096a462711
There is a single GeckoSampler and it is currently only accessed on the main
thread, so it's silly to use TLS for it; a normal global variable is better.
This patch also adds main thread assertions to a number of the profiler_*()
functions. Even though bug 1330184 may get rid of some of them, right now they
are a useful as both a sanity check and documentation.
--HG--
extra : rebase_source : 7613eb3cc8089b31180365f6463c81f4556c7b66
Note that the comment on ::Mutex said that it should support recursive locking,
but GeckoMutex was implemented using mozilla::Mutex which does *not* support
recursive locking.
The patch also removes OS::CreateMutex(), because it's only used twice and
doesn't make the code more concise.
--HG--
extra : rebase_source : 81f324cb3856ea5329d8b6edb457c8ec44395c56
There are lots of profiler_*() functions that simply call onto equivalent or
nearly-equivalent mozilla_sampler_*() functions. This patch removes the
unnecessary indirection by removing the mozilla_sampler_*() functions.
The most important changes:
- In platform.cpp, all the mozilla_sampler_*() definitions are renamed as
profiler_*().
- In GeckoProfiler.h, the new PROFILER_FUNC{,_VOID} macros provide a neat way
to declare the functions that must be present whether the profiler is enabled
or not.
- In GeckoProfiler.h, all the mozilla_sampler_*() declarations are removed, as
are all the profiler_*() definitions that corresponded to a
mozilla_sampler_*() function.
Other things of note:
- profiler_log(const char* str) is now defined in platform.cpp, instead of in
GeckoProfiler.h, for consistency with all the other profiler_*() functions.
Likewise with profiler_js_operation_callback() and
profiler_in_privacy_mode().
- ProfilerBacktraceDestructor::operator() is treated slightly different to all
the profiler_*() functions.
- Both variants of profiler_tracing() got some early-return conditions moved
into them from GeckoProfiler.h.
- There were some cases where the profiler_*() and mozilla_sampler_*() name
didn't quite match. Specifically:
* mozilla_sampler_get_profile_data() and profiler_get_profiler_jsobject():
name mismatch. Kept the latter.
* mozilla_sampler_get_profile_data_async() and
profiler_get_profile_jsobject_async(): name mismatch. Kept the latter.
* mozilla_sampler_register_thread() and profiler_register_thread(): return
type mismatch. Changed to void.
* mozilla_sampler_frame_number() and profiler_set_frame_number(): name
mismatch. Kept the latter.
* mozilla_sampler_save_profile_to_file() and
profile_sampler_save_profile_to_file(): the former was 'extern "C"' so it
could be called from a debugger easily. The latter now is 'extern "C"'.
- profiler_get_buffer_info() didn't fit the patterns handled by
PROFILER_FUNC{,VOID}, so the patch makes it call onto the new function
profiler_get_buffer_info_helper(), which does fit the pattern.
--HG--
extra : rebase_source : fa1817854ade81e8a3027907d1476ff2563f1cc2
Nobody has ever used this, and measuring power consumption while running a
sampling profiler at 1000 Hz isn't a good idea.
--HG--
extra : rebase_source : b84255a08bfea07b90bedc1f24086695143d5c8e
With frame pointer omission disabled we should always have usable stacks on Windows. This allows us to remove the MOZ_STACKWALKING define as it will always be enabled.
MozReview-Commit-ID: 54xs3Hf1r4P
--HG--
extra : rebase_source : dfaf13fb4c2185985f4f074c338ccf1fef8f3c94
CLOSED TREE
Backed out changeset 01cfc71ce542 (bug 1322735)
Backed out changeset 84c729c41230 (bug 1322735)
Backed out changeset b419aaefae95 (bug 1322735)
With frame pointer omission disabled we should always have usable stacks on Windows. This allows us to remove the MOZ_STACKWALKING define as it will always be enabled.
MozReview-Commit-ID: 54xs3Hf1r4P
--HG--
extra : rebase_source : 5fe27cdeeb464d81fbedc8c02ac187658bd759e7
The only interesting setter of this field is
GeckoSampler::GetPrimaryThreadProfile, which is unused. Once that
function is removed, mPrimaryThreadProfile is set to null in
GeckoSampler's constructor and never changed or referenced.
Smart pointers are better than raw pointers, and this makes clients of
PlatformData slightly simpler because they don't have to manage
destruction themselves: the new UniquePtr-derived type handles all of
that for us.
Smart pointers are better than raw pointers. This change also has the
benefit of removing the manual memory management in ~GeckoSampler and
locating all the memory management in ThreadInfo, where it belongs.
We have an abstraction over the current thread ID already, so we can use
that to get the current thread id and eliminate this bit of
cut-and-paste programming.
Nicholas Nethercote
Julian Seward
Shih-Chiang Chien
Luke Wagner
Eric Rahm
Phil Ringnalda
Nathan Froyd
Steve Fink
Markus Stange
Ting-Yu Chou
Nathan Froyd