This patch puts the arrays inside NativeStack, gives NativeStack a constructor,
and renames its fields using "mFoo" form.
The patch also moves MAX_NATIVE_FRAMES from the LUL-only code and applies it
globally. This increases the max native frame count from 256 to 1024 on the
LUL platforms, which makes things consistent with other platforms.
--HG--
extra : rebase_source : 0854ab37d101e090ea05cba7cb6d296450f8dfdc
This increases naming consistency. The remaining aStartTime parameters within
the profiler refer to a different start time than the process start time.
--HG--
extra : rebase_source : 0a07c54288f31af5a15518180b00fe59b587f784
ProfilerStackFrameRAII and ProfilerStackFrameDynamicRAII are very similar; the
latter lets a dynamic string be specified as well (and lacks the
MOZ_GUARD_OBJECT stuff, for no good reason).
This patch does the following.
- Removes ProfilerStackFrameDynamicRAII, and adds a dynamic string to
ProfilerStackFrameRAII. It also reorders the constructor's arguments to match
the field ordering of ProfileEntry. There aren't many usage sites so these
changes don't affect many places.
- With that done, there is only a single callsite for each of
profiler_call_enter() and profiler_call_exit(), so the patch also inlines and
removes them.
These annotations aren't doing anything useful. The important thing with
the PseudoStack is that, during pushes, the stack pointer incrementing happens
after the new entry is written, and this is ensured by the stack pointer being
Atomic.
The patch also improves the comments on PseudoStack.
--HG--
extra : rebase_source : 100f8a5e4b750c15fac66175550c4c284a141f16
There are three flags in ProfileEntry::Flags, which suggests there are 2**3 = 8
combinations. But there are only actually 4 valid combinations.
This patch converts the three flags to a single "kind" enum, which makes things
clearer. Note also that the patch moves the condition at the start of
AddPseudoEntry() to its callsite, for consistency with the earlier JS_OSR entry
kind check.
--HG--
extra : rebase_source : 0950769ee1530291860ef3be47d240df5939e871
The category handling code at the end of AddPseudoEntry has two problems.
- The assertion checks |category| for the IS_CPP_ENTRY flag. This represents a
confusion between the entry's |category| and its |flags|. They're both stored
in a single uint32_t, but are conceptually different types. So the assertion
is vacuously satisfied.
Furthermore, there's no clear way to fix the assertion -- it doesn't make
sense to check the entry's flags for IS_CPP_ENTRY, because this code can
clearly take C++ or JS entries. So the patch just removes the assertion.
- The category is compared to zero. This also doesn't make sense, because zero
isn't a valid category. The patch removes this comparison.
--HG--
extra : rebase_source : 16a7248ffb90ccd90e6102d597fb5bdff706312e
Two improvements.
- More testing of markers by introducing and using a ProfilerMarkerPayload
subclass that keeps track of how many instances have been created, streamed,
and destroyed. This change would have caught the defect that was in the first
landing of part 3 of this bug.
- More testing of JSON output, to check for various substrings that are
expected to be present.
I think all of these assertions are now unnecessary.
MozReview-Commit-ID: 9EI195QsizN
--HG--
extra : rebase_source : 4f03ef02ba6680ee6cad22d5d3f347db7d70aa9c
extra : intermediate-source : 2b8d50fcb20fc0bd808707aae00d6cbcb4536bac
extra : source : 327c145ded03d39970351a9cc01492f0541d9149
- The profiler gives the JS engine a reference to the pseudo-stack via
SetContextProfiilngStack(). That function now takes a |PseudoStack*| instead
of a |ProfileEntry*| and pointer to the stack pointer.
- PseudoStack now has a |kMaxEntries| field, which is easier to work with than
|mozilla::ArrayLength(entries)|.
- AddressOfStackPointer() is no longer needed.
- The patch also neatens up the push operations significantly. PseudoStack now
has pushCppFrame() and pushJsFrame(), which nicely encapsulate the two main
cases. These delegate to the updated initCppFrame() and initJsFrame()
functions in ProfileEntry.
- Renames max_stck in testProfileStrings.cpp as peakStackPointer, which is a
clearer name.
- Removes a couple of checks from testProfileStrings.cpp. These checks made
sense when the pseudo-stack was accessed via raw manipulation, but are not
applicable now because we can't artificially limit the maximum stack size so
easily.
This includes renaming its fields to match SpiderMonkey naming conventions
instead of Gecko naming conventions.
This patch is just about moving the code. The next patch will change
SpiderMonkey to actually use PseudoStack directly.
--HG--
extra : rebase_source : 27e77ddf950201eb6bdba60003218056442cf7ab
I think all of these assertions are now unnecessary.
MozReview-Commit-ID: 9EI195QsizN
--HG--
extra : rebase_source : ddeb80dfc61ff843b6ba4b35f73d005ca060e429
extra : source : 327c145ded03d39970351a9cc01492f0541d9149
If NotifyObservers is called on a background thread, dispatch a runnable to
the main thread which will send the observer notification. This can result in
awkward orders of the notifications if the profiler functions are called in
quick succession on both the main thread and another thread, but I'm not sure
what to do about that.
MozReview-Commit-ID: GlkVwGTa2b4
--HG--
extra : rebase_source : 628ae91c3adef85c7fb07441b6573eda55e7ba48
extra : intermediate-source : 8c96ac2d762fbed161b6e577f845c1b58ec8bc17
extra : source : b8c27d6137a43ecd031f0b58c62752b7a9cefacb
ProfileEntry has |string|, which can be static or dynamic, and |dynamicString|.
If |string| is dynamic, the FRAME_LABEL_COPY flag must be set, and it will be
copied into profiler output.
But there is only one place that uses dynamic |string| values, in SpiderMonkey.
And that place doesn't use |dynamicString|. So this patch changes that place to
use an empty |string| and put the old dynamic |string| value in
|dynamicString|. This in turn removes the need for FRAME_LABEL_COPY.
One minor wrinkle is that when |dynamicString| is used the old code put a space
between |string| and |dynamicString|. The new code omits the space if |string|
is empty.
The patch also renames ProfileEntry::string as ProfileEntry::label_, which
better matches how it's used, and ProfileEntry::dynamicString as
ProfileEntry::dynamicString_ so the getter can be renamed dynamicString().
profiler_resume() mistakenly has the PSAutoLock outside the scope created
specially for it. This patch move the PSAutoLock inside the scope, making it
the same as profiler_pause().
I think all of these assertions are now unnecessary.
MozReview-Commit-ID: 9EI195QsizN
--HG--
extra : rebase_source : 2a9969dd9e48873f6ec333a5ddfa32b6d938de80
extra : histedit_source : ea4dab2b111e465d3a1e29996cc7f10eb8cdbb67%2C8ab59878e4b1b5a715e0738408c26ac3aa0501e7
If NotifyObservers is called on a background thread, dispatch a runnable to
the main thread which will send the observer notification. This can result in
awkward orders of the notifications if the profiler functions are called in
quick succession on both the main thread and another thread, but I'm not sure
what to do about that.
MozReview-Commit-ID: GlkVwGTa2b4
--HG--
extra : rebase_source : 30537a31cc828ad7828e41c2885d84a8299a5e70
extra : source : b8c27d6137a43ecd031f0b58c62752b7a9cefacb
extra : histedit_source : f1340ef14acde098de281b81d5ea89e386e7ab42
It's a software memory barrier, and not a very strong one. If the values it is
protecting are Atomic, that provides a stronger hardware memory barrier.
This patch removes it, and changes one of the values it was protecting from
|volatile| to Atomic. (The other value it was protecting was already Atomic.)
Sylvestre Ledru
Carsten "Tomcat" Book
Nicholas Nethercote
Markus Stange
Kan-Ru Chen
Phil Ringnalda
Iris Hsiao
Michael Layzell