1468789 - No obvious way to map from slow JS to the relevant web page

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P2)

Description

[Boris Zbarsky [:bzbarsky]]

I am looking at a profile in which all the JS stacks are from https://static.licdn.com/sc/h/br/8gesdixxb0mdd0yupx7pukf2q

Unfortunately, that tells me nothing about which site is involved...  Would it be possible to expose that information as well?

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Perhaps the docshell info from bug 1417976 will help with this?

Comment 2

[Julien Wajsberg [:julienw]]

That bug is for markers while here it's about samples. But yeah, the source of info is likely the same :)

Comment 3

[Markus Stange [:mstange]]

Are you interested in this information per stack frame or per sample? If we stored the tab group of the currently-executing runnable, would that be enough information? Would we also need a list / tree of documents + urls that are in a given tab group?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Are you interested in this information per stack frame or per sample?

Which one is easier?

Really, I guess, I want to know the url of the toplevel window involved or something like that, for my initial use case.  That should be more or less an invariant across all stackframes in a sample as long as we don't have event loops spinning and cross-window calls, so I don't care too much where the info is attached, I think.

That said, in terms of UI I'm looking at a stackframe that has a bunch of time spent in it, not at a specific sample.

> If we stored the tab group of the currently-executing runnable, would that be enough information?

I'm not sure, because I suspect a lot of my tabs are in the same tabgroup (got opened by me clicking a link in my RSS reader).

Comment 5

[Markus Stange [:mstange]]

Do you know how we might obtain that information? Do we need to instrument every C++ -> JS call site? I suppose we could start by instrumenting just DOM event dispatching and rAF callback firing, and see how far that gets us.

Comment 6

[Markus Stange [:mstange]]

I brought up tab groups because I was hoping that we could piggyback on the Quantum DOM runnable labeling work for this.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Do you know how we might obtain that information?

What data structure are you starting with?

Comment 8

[Markus Stange [:mstange]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #7)
> What data structure are you starting with?

Nothing, really, other than a TLS object (ProfilingStack) that we can store this information in.

If you're asking about the existing code that associates JavaScript stack frames with URLs, that code is in the JS engine and gets the information from the JSScript*:
https://searchfox.org/mozilla-central/rev/403038737ba75af3842ba6b43b6e2fb47eb06609/js/src/vm/GeckoProfiler.cpp#283
https://searchfox.org/mozilla-central/rev/403038737ba75af3842ba6b43b6e2fb47eb06609/js/src/jit/JitcodeMap.cpp#323

Comment 9

[Boris Zbarsky [:bzbarsky]]

OK, so you're starting with a JSScript/JSFunction.  From which you can get the global, check whether it's a window, if so get the top window and get its document's url, right?

Comment 10

[Greg Tatum [:gregtatum]]

I was chatting a bit with Markus about this bug. I wrote some notes which I will include here:

* We need some way to map JSScripts to docshell ids
* I should look at Nazim's work on keeping the list of the docshells in the profiler (in CorePS?)
* The JS Engine does not know about docshells
* We have the JSScript in the profiling stack frame, we can look at the this pointer at sampling time, then we need to figure out how to get the docshell id from there

Comment 11

[Markus Stange [:mstange]]

We also need to decide at what time we want to do the mapping.

The string "functionName (url:line)" for a JS frame gets computed
 - for interpreter frames: the first time this function is called (via GeckoProfilerThread::enter)
 - for JIT frames: when this function is compiled (via BaselineCompiler::compile or JitcodeIonTable::makeIonEntry)

This happens in the JS engine. I don't think we want to call out to the profiler to compute the docshell ID for the JSScript from these places - I'd imagine that it would happen way too often and have too much overhead.

Instead, maybe we could hook the creation + destruction of JS global for windows in Gecko, and notify the profiler every time this happens, so that the profiler could keep a map of JS globals -> nsID of the top window's DocShell. Then, at sampling time, when we encounter a JS frame in the stack, we can get the global from the JSScript (I hope this can be done off-thread?) and look up the global in the stored hashmap. This gives us the DocShell ID, which we can then write into the profile buffer. At JSON serialization time, we can put that ID into a new "docShellID" column in the frameTable.
I think this should work for interpreter frames.
For JIT frames, we don't have the JSScript at sampling time. We only have a JIT return address and we store that in the profile buffer. Then, at JSON serialization time, we map that JIT return address to a ProfiledFrameHandle, from which we can look up the JSScript for it. (The first patch in bug 1441689 adds the necessary infrastructure to get the JSScript from a ProfiledFrameHandle.) From here on out we can do the same as for interpreter frames: get the global, map it to the DocShell ID, write out the DocShell ID.

Comment 12

[Greg Tatum [:gregtatum]]

I'm not planning on working on this right now, but wanted to leave some local notes I had written for the future in case I do.

changeset:   432845:0893dcddbf94
user:        Greg Tatum <[e-mail]>
date:        Wed Aug 22 14:08:27 2018 -0500
summary:     WIP - Notes for identifying docshells for JS frames

diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -1985,16 +1985,21 @@ nsGlobalWindowOuter::SetNewDocument(nsID
   kungFuDeathGrip->DidInitializeContext();
 
   // We wait to fire the debugger hook until the window is all set up and hooked
   // up with the outer. See bug 969156.
   if (createdInnerWindow) {
     nsContentUtils::AddScriptRunner(
       NewRunnableMethod("nsGlobalWindowInner::FireOnNewGlobalObject",
                         newInnerWindow,
+/**
+ * Markus linked me here when referring to the JS global object.
+ * mstange: I’m referring to the JS definition: https://developer.mozilla.org/en-US/docs/Glossary/Global_object
+ * mstange: in terms of implementation, I think a `window` object maps to an `nsGlobalWindowInner` object
+ */
                         &nsGlobalWindowInner::FireOnNewGlobalObject));
   }
 
   if (newInnerWindow && !newInnerWindow->mHasNotifiedGlobalCreated && mDoc) {
     // We should probably notify. However if this is the, arguably bad,
     // situation when we're creating a temporary non-chrome-about-blank
     // document in a chrome docshell, don't notify just yet. Instead wait
     // until we have a real chrome doc.

Comment 13

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1468789 - Part 1: Add window id inside realm creation options. r?jandem

Comment 14

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1468789 - Part 2: Collect inner window id information for js interpreter frames and add a mechanism to get that for jit frames r?gerald,jandem

Depends on D51859

Comment 15

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1468789 - Part 3: Serialize innerWindowID for js/jit frames. r?gerald

Depends on D51860

Comment 16

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Front-end side of this work: https://github.com/firefox-devtools/profiler/pull/2300.
It's landed and deployed. Landing the platform side now.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c93e34bc1f3
Part 1: Add window id inside realm creation options. r=jandem,bzbarsky
https://hg.mozilla.org/integration/autoland/rev/1c3c775faf99
Part 2: Collect inner window id information for js interpreter frames and add a mechanism to get that for jit frames r=gerald,jandem,mstange
https://hg.mozilla.org/integration/autoland/rev/e4ed5d091e3d
Part 3: Serialize innerWindowID for js/jit frames. r=gerald

Comment 18

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0c93e34bc1f3
https://hg.mozilla.org/mozilla-central/rev/1c3c775faf99
https://hg.mozilla.org/mozilla-central/rev/e4ed5d091e3d

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

FYI, this appears to have caused an ~100K Base Content Heap Unclassified regression:

https://treeherder.mozilla.org/perf.html#/graphs?highlightAlerts=1&selected=1959119,592187,825.3047750241172,166.66384607410416,973899535&series=autoland,1959119,1,4&timerange=1209600&zoom=1573782938412,1573882085995,1438727.611398332,2094262.9739059655

I'm not sure why this didn't trigger a perf alert; perhaps it was below a threshold