Currently the profiler mostly uses an array of strings to represent which
features are available and in use. This patch changes the profiler core to use
a uint32_t bitfield, which is a much simpler and faster representation.
(nsProfiler and the profiler add-on still use the array of strings, alas.) The
new ProfilerFeature type defines the values in the bitfield.
One side-effect of this change is that profiler_feature_active() now can be
used to query all features. Previously it was just a subset.
Another side-effect is that profiler_get_available_features() no longer incorrectly
indicates support for Java and stack-walking when they aren't supported. (The
handling of task tracer support is unchanged, because the old code handled it
correctly.)
Because ProfilerMarkerPayload is the main type defined in these files, and
because the next patch is going to introduce ProfilerMarker.{h,cpp}, which
would be confusingly similar to the old names.
--HG--
rename : tools/profiler/core/ProfilerMarkers.cpp => tools/profiler/core/ProfilerMarkerPayload.cpp
rename : tools/profiler/public/ProfilerMarkers.h => tools/profiler/public/ProfilerMarkerPayload.h
extra : rebase_source : df22a2ab3867650348ae78fe959ff0366aff230b
This fixes a JS exception that gets thrown when one tries to capture a profile
in this case.
--HG--
extra : rebase_source : 46f6eeed3c17086b0b6c35b26f3c9e4841dd6cff
Now that ThreadResponsiveness is only used on the main thread, we can refactor
ThreadInfo a bit. This patch does the following.
- Removes ThreadInfo::mThread, which is unused.
- Changes ThreadInfo::mRespInfo to a Maybe<>, and moves the is-main-thread
checking outside of ThreadInfo and ThreadResponsiveness.
- Renames {ThreadInfo,TickSample}::mRespInfo as mResponsiveness, to better
match the class name.
The state management is better done within nsProfiler::GetProfileDataAsync()
and nsProfiler::DumpProfileToFileAsync(). (The latter function is new in this
patch.)
This fixes a deadlock.
Other notes:
- The patch moves ProfileGatherer from ProfilerState to nsProfiler. This is
nice because the former is shared between threads but the latter is main
thread only. (This is how the deadlock is avoided.)
- ProfilerStateMutex and PSLockRef are no longer required in platform.h. Those
types and variables are now only used in platform.cpp and platform-*.cpp.
- ProfilerGatherer now calls profiler_get_profile() instead of ToJSON(). Which
means that ToJSON() now has a single caller, so the patch inlines it at the
callsite and removes it.
- profiler_save_profile_to_file_async() dispatched a Runnable to the main
thread. But this wasn't necessary, because it always ran on the main thread
itself. So the new function nsProfiler::DumpProfileToFileAsync() doesn't do
that.
- profiler_will_gather_OOP_profile(), profiler_gathered_OOP_profile(), and
profiler_OOP_exit_profile() are all moved into nsProfiler as well. This
removes the need for the horrible fake lock in
profiler_will_gather_OOP_profile(), hooray!
The conversion to a JSObject is better done within
nsProfiler::GetProfileData().
--HG--
extra : rebase_source : 4a0ba97d99681fca96f2d26b609bafe188095787
Replace it with profiler_is_active() in one place, and simply remove it in the
other places. These other places are:
- Around the call to profiler_OOP_exit_profile: profiler_OOP_exit_profile
itself already checks whether the profiler is running and does nothing if
it's not.
- When handling the 'profiler-subprocess-gather' notification. This
notification is sent by the profiler because it's interested in the
profile, so there's little reason to reject it.
- In RecvProfile: If the child process sent us a profile, it did so in
response to a GatherProfile request, so chances are that we're still
interested in that response.
These changes may get us a little closer to a state where you can call
getProfileDataAsync, stop the profiler before the content process profiles
have all come in, and then still receive a response with all the profiles.
At the moment, stopping the profiler will abort the profile gathering process,
but that seems more like an accident and less like the behavior you'd want.
MozReview-Commit-ID: 2tRXC70BztJ
--HG--
extra : rebase_source : 3b2f6f51d75d5f0d439e1a815d84164a5a763603
API before this change:
- nsIProfiler::getSharedLibraryInformation() returns a string containing a
JSON array of libraries.
- The profile format is at version 3.
- Every profile has a "libs" field that contains the same JSON string as the
return value of nsIProfiler::getSharedLibraryInformation.
- The array of libraries is not sorted.
- Each library has a "name" field that contains:
- The module's debug name on Windows
- The full path to the binary on Mac + Linux
API after this change:
- nsIProfiler::getSharedLibraryInformation() is removed.
- nsIProfiler has a readonly property called sharedLibraries.
- The profile format is at version 4.
- Every profile has a "libs" field that contains the same array as
nsIProfiler.sharedLibraries, no longer as a JSON string but as a regular
array.
- The array of libraries is sorted by start address.
- Each library has a "name" field that contains the binary file's basename,
on all platforms.
- Each library has a "path" field that contains the full path to the binary,
on all platforms.
- Each library has a "debugName" field that contains the library's debug
name, on all platforms. On Windows, the debug name is the filename
(basename) of the pdb file for that binary. On other platforms, debugName
is the same as |name|.
- Each library has a "debugPath" field that contains the absolute path
library's pdb file on Windows; on non-Windows, debugPath and path are the
same.
- Each library has an "arch" field that is either an empty string (Linux +
Windows) or the library's architecture; it'll differentiate between the
architectures "x86_64" and "x86_64h". (x86_64h is used for binaries that
contain instructions that are specific to the Intel Haswell
microarchitecture.)
MozReview-Commit-ID: 8Nrs4dyHhDS
--HG--
extra : rebase_source : 4039926ae4d776bf53ea71df5fe3f8200d3e2784
extra : source : 4e282aa03422de5b8d51e1aaeb3e53ee547293dd
API before this change:
- nsIProfiler::getSharedLibraryInformation() returns a string containing a
JSON array of libraries.
- The profile format is at version 3.
- Every profile has a "libs" field that contains the same JSON string as the
return value of nsIProfiler::getSharedLibraryInformation.
- The array of libraries is not sorted.
- Each library has a "name" field that contains:
- The module's debug name on Windows
- The full path to the binary on Mac + Linux
API after this change:
- nsIProfiler::getSharedLibraryInformation() is removed.
- nsIProfiler has a readonly property called sharedLibraries.
- The profile format is at version 4.
- Every profile has a "libs" field that contains the same array as
nsIProfiler.sharedLibraries, no longer as a JSON string but as a regular
array.
- The array of libraries is sorted by start address.
- Each library has a "name" field that contains the binary file's basename,
on all platforms.
- Each library has a "path" field that contains the full path to the binary,
on all platforms.
- Each library has a "debugName" field that contains the library's debug
name, on all platforms. On Windows, the debug name is the filename
(basename) of the pdb file for that binary. On other platforms, debugName
is the same as |name|.
- Each library has a "debugPath" field that contains the absolute path
library's pdb file on Windows; on non-Windows, debugPath and path are the
same.
- Each library has an "arch" field that is either an empty string (Linux +
Windows) or the library's architecture; it'll differentiate between the
architectures "x86_64" and "x86_64h". (x86_64h is used for binaries that
contain instructions that are specific to the Intel Haswell
microarchitecture.)
MozReview-Commit-ID: 8Nrs4dyHhDS
--HG--
extra : rebase_source : 4039926ae4d776bf53ea71df5fe3f8200d3e2784
extra : source : 4e282aa03422de5b8d51e1aaeb3e53ee547293dd
This is more of a workaround than a real fix, but the old code wasn't working
for non-main threads either, and we'd like to change the way this information
is computed anyway (bug 1340714) and then we won't need CheckResponsivenessTask
any more.
MozReview-Commit-ID: FGiomjwpk3z
--HG--
extra : rebase_source : 95fc57c0622fdbaa928700a971a4ce839ad92797
This patch does the following.
- Introduces NotifyObservers() for the simple notification cases in
platform.cpp.
- Removes profiler_lock() and profiler_unlock() because they do notifications
that the profiler add-on no longer listens for.
--HG--
extra : rebase_source : 77a1868ba494dea314702bbdf9478a1da36c9efb
This patch properly synchronizes all the global state in platform*.cpp, which
gets us a long way towards implementing bug 1330184.
- Most of the global state goes in a new class, ProfilerState, with a single
instance, gPS. All accesses to gPS are protected by gPSMutex. All functions
that access ProfilerState require a token proving that gPS is locked; this
makes things much clearer.
gRegisteredThreadsMutex is removed because it is subsumed by gPSMutex.
- gVerbosity, however, does not go in ProfilerState. It stays separate, and
gains its own mutex, gVerbosityMutex.
Also, the tracking of the current profiler state is streamlined. Previously it
was tracked via:
- stack_key_initialized, gInitCount, gSampler, gIsProfiling, gIsActive, and
gIsPaused.
Now it is tracked via:
- gPS, gPS->sActivity, and gPS->mIsPaused.
This means that the Sampler class is no longer necessary, and the patch removes
it.
Other changes of note made by the patch are as follows.
- It removes ThreadInfo::{mMutex,GetMutex}. This mutex was only used in two
places, and both these are now protected by gPSMutex.
- It tweaks the LOG calls. All the main functions (init(), shutdown(), start(),
stop()) now do consistent BEGIN/END logging, and a couple of other low-value
incidental LOG calls have been removed.
- It adds a lot of release assertions requiring that gPS be initialized (e.g.
profiler_init() has been called but profiler_shutdown() has not).
- It uses alphabetical order for everything involving profiler feature names.
- It removes Platform{Start,Stop}() and SamplerThread::{Start,Stop}Sampler().
These are no longer necessary now that SamplerThread::sInstance has been
replaced with ProfilerState::mSamplerThread which allows more direct access
to the current SamplerThread instance.
- It removes PseudoStack::mPrivacyMode. This was derived from the "privacy"
feature, and we now use gPS->mFeaturePrivacy directly, which is simpler.
It also replaces profiler_in_privacy_mode() with
profiler_is_active_and_not_in_privacy_mode(), which avoids an unnecessary
lock/unlock of gPSMutex on a moderately hot path.
Finally, the new code does more locking than the old one. A number of operation
The following operations now lock a mutex when they previously didn't; the
following are ones that are significant, according to some ad hoc profiling.
- profiler_tracing()
- profiler_is_active()
- profiler_is_active_and_not_in_privacy_mode()
- profiler_add_marker()
- profiler_feature_active()
- SamplerThread::Run() [when the profiler is paused]
All up this roughly doubles the amount of mutex locking done by the profiler.
It's probably possible to avoid this increase by allowing careful unlocked
access to three of the fields in ProfilerState (mActivityGeneration,
mFeaturePrivacy, mStartTime), but this should only be done as a follow-up if
the extra locking is found to be a problem.
--HG--
extra : rebase_source : c2e41231f131b3e9ccd23ddf43626b54ccc77b7b
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
It's only being used in a boolean fashion, so this patch replaces it with a
boolean.
--HG--
extra : rebase_source : 91152dff81107070fa49b3984e1b6759e0cd6d20
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
There's no point having them as separate classes. This removes the need for
some virtual functions, too.
--HG--
extra : rebase_source : b2607ba2431ae043b6e015f4f435b0d660b02d71
For some reason, we were decrementing the ProfileGatherer's mPendingProfiles
when receiving an "exit profile". An exit profile is handed over by a subprocess
parent actor, but in order to have that exit profile, the content process
_must_ have sent it up to the parent already, and that means that the counter
had alreay been decremented on its receipt.
This means that if the subprocess parent actor exited, it'd decrement
the counter twice, which means that we open ourselves up for missing out
on profiles that haven't yet reached the parent.
I can't think of a good reason why we'd want to decrement the counter
when storing an exit profile, so I've just removed that bit.
MozReview-Commit-ID: 8jSqtpYbXh0
--HG--
extra : rebase_source : 64eadae51b0192231846327c0e677154287bd1a6
Kan-Ru Chen
Markus Stange
Wes Kocher
Carsten "Tomcat" Book
Nicholas Nethercote
Dan Banner
Sebastian Hengst
Julian Seward
Nathan Froyd
Mike Conley
Andrew McCreight