Closed Bug 1362894 Opened 4 years ago Closed 4 years ago

The cost of PROFILER_LABEL() is way too high

Categories

(Core :: Gecko Profiler, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ehsan, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

On a perf profile of bug 944127 on a local build of trunk without the Gecko Profiler add-on installed, 1.57% of the total time is spent under PseudoStack::push() and 0.88% of the total time is spent under profiler_call_enter() and 0.34% of the total time is spent under profiler_call_exit(), and these are both coming from <https://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/layout/style/nsStyleSet.cpp#1101>.

Our first option is to remove this PROFILER_LABEL.  But there is a lot we can do to make these more efficient.

Perf says that one performance issue with profile_call_enter is that it's non-inline (same with profile_call_exit and in fact that is the *only* performance problem with it.)  The other issue is the TLS access there.  I'm not sure what we can do about that.  The profile us a lot less conclusive about push() because perf gets confused when annotating it and it shows me an unrelated function mostly.
profiler_call_enter and profiler_call_exit were uninlined in bug 1359000.

Nick, can we revert that decision?
Blocks: 1359000
Flags: needinfo?(n.nethercote)
I'll look at this tomorrow.
Flags: needinfo?(n.nethercote)
Bug 1359000 moved these functions from GeckoProfiler.h to platform.cpp, which
allowed a lot of follow-up simplifications. But it hurt performance.

This patch moves them back to GeckoProfiler.h and makes them |inline| again.
This required adding a second TLS pointer, sPseudoStack. Comments in the patch
explain why.
Attachment #8866206 - Flags: review?(mstange)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Attachment #8866206 - Flags: review?(mstange) → review+
https://hg.mozilla.org/mozilla-central/rev/17c6f55e1bdc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1365269
You need to log in before you can comment on or make changes to this bug.