We'll be relying heavily on BHR pseudo-stacks to narrow down the causes of the apparent responsiveness regression in e10s (bug 1182637), so we should expand coverage of the BHR pseudostack. The labels are mostly in gfx according to BenWa. We noticed asyncPluginInit doesn't have labels, etc. We can generate the list of missing labels by triggering chromehang-style stackwalks from BHR. The chromehangs dash is another good starting point.

So if I understand correctly, I need to sprinkle magic PROFILER_LABEL* powder around the top n chromehang stacks and BHR pseudo stacks, right?

Experience shows that we do not have enough profiler labels to make BHR hang reports meaningful. This patch adds enough labels to let us exploit hang reports matching the 25 topmost chrome hangs. Review commit: https://reviewboard.mozilla.org/r/30965/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/30965/

Comment on attachment 8708086 [details] MozReview Request: Bug 1224374 - Profiler labels for the 25 top chrome hangs;r?benwa https://reviewboard.mozilla.org/r/30965/#review27813 Looks alright. None of these look like they are hot functions but if you're unsure please ask the module owner for feedback. Otherwise feel free to land!

Requesting additional feedback from marcoz for `NotificationController::WillRefresh`. Essentially, if it's a hot function, we might need to move the label.

Comment on attachment 8708086 [details] MozReview Request: Bug 1224374 - Profiler labels for the 25 top chrome hangs;r?benwa https://reviewboard.mozilla.org/r/30965/#review27983

(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #4) > Requesting additional feedback from marcoz for > `NotificationController::WillRefresh`. Essentially, if it's a hot function, > we might need to move the label. I'm OK with the label as is, but for a more technical review, especially what the "hot function" part is concerned, :surkov or :tbsaunde are better candidates to give this a look-over and decide whether the label should be moved or not.

Actually, I'm basically sure that it's not hot.

Review commit: https://reviewboard.mozilla.org/r/31021/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31021/

Comment on attachment 8708942 [details] MozReview Request: Bug 1224374 - Profiler labels for the top 26-100 chrome hangs;r?benwa https://reviewboard.mozilla.org/r/31021/#review28065 Please check with devtools if your labels are going to start showing up in the DevTools profiler. Some of this we don't want to expose to developers. I believe they might show anything that isn't category::other. If that's the case we may want to put some of these as category::other. ::: dom/plugins/base/nsJSNPRuntime.cpp:1679 (Diff revision 1) > + PROFILER_LABEL_FUNC(js::ProfileEntry::Category::JS); I'm not familiar with this code but these resolve/getter could very likely be hot. I'd suggest that you get a peer to review this. ::: dom/plugins/base/nsNPAPIPluginInstance.cpp:285 (Diff revision 1) > - > + You've added trailing whitespace here

Boris, what's your word on that? Are `NPObjWrapper_Resolve` and `NPObjectMember_GetProperty` too hot to add them to the label?

I wouldn't expect them to be particularly hot in the grand scheme of things. They only get called when manipulating plugin-provided objects from JS, which should actually be fairly rare on a "number of pages" basis. I have no idea how hot they would be on pages that _do_ perform such manipulation.

(In reply to Benoit Girard (:BenWa) from comment #9) > Please check with devtools if your labels are going to start showing up in > the DevTools profiler. Some of this we don't want to expose to developers. I > believe they might show anything that isn't category::other. If that's the > case we may want to put some of these as category::other. Apparently, they are not going to show up. (In reply to Boris Zbarsky [:bz] from comment #11) > I wouldn't expect them to be particularly hot in the grand scheme of things. > They only get called when manipulating plugin-provided objects from JS, > which should actually be fairly rare on a "number of pages" basis. > > I have no idea how hot they would be on pages that _do_ perform such > manipulation. I suspect that plug-in manipulation is not that fast regardless, so I'm for taking the chance.

Sounds good then

Comment on attachment 8708942 [details] MozReview Request: Bug 1224374 - Profiler labels for the top 26-100 chrome hangs;r?benwa Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31021/diff/1-2/

https://hg.mozilla.org/integration/mozilla-inbound/rev/0dc02cb0b604 https://hg.mozilla.org/integration/mozilla-inbound/rev/5f458e6e4997

Backed out for compile error on Windows: https://hg.mozilla.org/integration/mozilla-inbound/rev/f98e2cc0df1d 11:56:00 INFO - nsFilePicker.cpp 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/widget/windows/nsFilePicker.cpp(871) : error C3083: 'ProfileEntry': the symbol to the left of a '::' must be a type 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/widget/windows/nsFilePicker.cpp(871) : error C3083: 'Category': the symbol to the left of a '::' must be a type 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/widget/windows/nsFilePicker.cpp(871) : error C2039: 'OTHER' : is not a member of 'js' 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/widget/windows/nsFilePicker.cpp(871) : error C2065: 'OTHER' : undeclared identifier 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/widget/windows/nsFilePicker.cpp(871) : error C3861: 'PROFILER_LABEL_FUNC': identifier not found 11:56:00 INFO - c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/src/config/rules.mk:957: recipe for target 'nsFilePicker.obj' failed 11:56:00 INFO - mozmake.EXE[5]: *** [nsFilePicker.obj] Error 2

I'm on it, thanks.

Comment on attachment 8708086 [details] MozReview Request: Bug 1224374 - Profiler labels for the 25 top chrome hangs;r?benwa Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: Our BHR stack traces will remain largely useless, which will render the results of our next e10s experiment much less useful. [Describe test coverage new/current, TreeHerder]: [Risks and why]: Shouldn't be any. Just profiler labels. [String/UUID change made/needed]: This needs to be uplifted during the week.

Comment on attachment 8708942 [details] MozReview Request: Bug 1224374 - Profiler labels for the top 26-100 chrome hangs;r?benwa Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: [Describe test coverage new/current, TreeHerder]: [Risks and why]: [String/UUID change made/needed]:

Comment on attachment 8708942 [details] MozReview Request: Bug 1224374 - Profiler labels for the top 26-100 chrome hangs;r?benwa Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31021/diff/2-3/

https://hg.mozilla.org/integration/mozilla-inbound/rev/e245e8d31839 https://hg.mozilla.org/integration/mozilla-inbound/rev/e74405918e7a

FYI, this is going to need rebased patches for Aurora.

Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: [Describe test coverage new/current, TreeHerder]: [Risks and why]: [String/UUID change made/needed]:

https://hg.mozilla.org/mozilla-central/rev/e245e8d31839 https://hg.mozilla.org/mozilla-central/rev/e74405918e7a

Comment on attachment 8710278 [details] [diff] [review] Aurora version of the patch Improve the stack traces, taking it.