The cost of PROFILER_LABEL() is way too high

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: njn)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
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.
Reporter

Comment 1

2 years ago
profiler_call_enter and profiler_call_exit were uninlined in bug 1359000.

Nick, can we revert that decision?
Blocks: 1359000
Flags: needinfo?(n.nethercote)
Assignee

Comment 2

2 years ago
I'll look at this tomorrow.
Flags: needinfo?(n.nethercote)
Assignee

Comment 3

2 years ago
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

Updated

2 years ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Attachment #8866206 - Flags: review?(mstange) → review+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/17c6f55e1bdc
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

2 years ago
Depends on: 1365269
You need to log in before you can comment on or make changes to this bug.