Closed Bug 1979114 Opened 9 months ago Closed 6 months ago

Implement fetching JS sources from the JS engine at the profile gathering phase and use webchannel to return them to the frontend

Categories

(Core :: Gecko Profiler, enhancement, P2)

Product:
Core
Component:
Gecko Profiler
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
146 Branch
Tracking Flags:
Tracking Status
firefox146 --- fixed

People

(Reporter: canova, Assigned: canova)

References

(Blocks 1 open bug)

Details

Attachments

(11 files)

Bug 1979114 - Propagate the profile generation information for child processes instead of duplicating them r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Add a JS API to get JS script sources r?iain
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Add IPC serialization and deserialization methods for the JS sources r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Include JS sources in the profile additional information r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Add a new JS sources feature to the profiler r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Collect the JS sources during profile serialization r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Convert the JS sources into JS objects in additionalInfo r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Fetch retrievable sources while converting sources to JS objects inside the parent process r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Add some tests for jsSource retrieval r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Make ProfileGenerationAdditionalInformation MoveOnly and remove the copy constructors of additional information structs r?mstange
48 bytes, text/x-phabricator-request
Details | Review
Bug 1979114 - Stream SourceTable for each process r?mstange
48 bytes, text/x-phabricator-request
Details | Review

Bug 1959977 mentions about adding sourceIds to the JS frames so we can match the sourceIds with their sources and retrieve the sources from the profiler frontend.

This bug is about doing this retrieval of the source code from the frontend. Since we have souceIds in the frotend, we can send webchannel requests with these IDs to Firefox to fetch the sources.

It's important to note that sourceId itself is not enough to fetch the source because these IDs are incremented locally per processs. So we need to send pid+sourceId to Firefox to be able to fetch it.

So the retrieval of the source code needs to happen during the profile collection time because in the future they might be destroyed. We will:

  1. Keep a set of RefPtr<ScriptSource> in the gecko profiler side of the JS engine so we can make sure that the sources live until the profile collection.
  2. (Bug 1959977) Collect sourceIds for every JS frames.
  3. At the end of the profiling session, while we are gathering the profiles, each process will ask the JS engine to retrieve the sourceId -> source data map and send it through the IPC to the parent process. (note that some sources are not possible to fetch from the child process, see step 5)
  4. We will accumulate every map inside pid -> (sourceId -> source data) map in the parent process.
  5. For the sources that were not possible to fetch inside the content process, we will request them from the parent process.
  6. Keep this extra data in the tab id -> profile with extra data weakmap that we have.

And we need to implement the webchannel request that returns this information:

  1. (bug 1916785) When we get a webchannel request with pid+sourceId data, find the source in the weakmap and return it.

Previously, we were generating the SharedLibraries inside the
profiler_get_profile_json and then we were trying to generate it again
inside the caller instead of using the Result value from it. Now, this
patch changes this to use the additional information we generate inside
the caller instead of generating it multiple times.

That way:

  1. We don't generate the same information multiple times.
  2. The information that we generated in the profiler calls inside
    platform.cpp file is not going to be lost.

This patch adds a new JS API for fetching the JS source data of the
JS frames that was captured during that profiling run.

We have a few constraints from the profiler:

  1. We have to be able to call it from a non-main thread. So we shouldn't
    rely on any MainThread only data.
  2. We shouldn't rely on a realm. We should directly return the source
    string if available.
  3. If the string is not available and if it's not possible to fetch it
    in a non-main thread, we should return the necessary information for
    that source, so the profiler can request it later, inside the parent
    process main thread.

Because of these constrains, lots of things had to be reimplemented to
fit inside these constraints.

ScriptSource::loadSource, ScriptSource::substring, and
ScriptSource::functionBodyString requires them be run on the main
thread, that's why we couldn't use them. I implemented
loadSourceOffMainThread, substringChars, and
functionBodyStringChars respectively that can be run outside of the
main thread.

Also I crated a struct to pass this to the profiler codebase. This will
be used to send the everything to the parent process.

For Retrievable<Unit> cases, we don't have the source code available
for them in this process. We can request them, but it requires calling
sourceHook, which also needs to be run on the main thread. It also sends
an IPC to the parent processs to request the data. Since we are
returning these sources to the parent process, we can delay fetching
them until we are on there. So we record all the necessary information
(filename) to fetch this data later, and sending that to the parent
process.

Since we would like to send the JS source information directly to the
parent process through IPC, we need a serialization and deserialization
code to transfer this data.

This patch adds the JS sources inside the profile additional information
so we can send them through IPC.

I add this feature now, so I can use it in the following patch when we
are retrieving the JS sources from the JS engine.

This feature is currently not on by default until we iron out all the
issues and defails.

Finally, we implement the place where we actually request the JS sources
from the JS engine to the profiler land. We are iterating over all the
known JSContext that we recorded and request all information we have.
Then this will be send to the parent process, and the maps will be
merged.

We convert the JS sources into an object of objects that can hold
pid -> sourceId -> source text map. That way we can keep this object
inside the devtools JS codebase.

There were some ScriptSources with the Retrievable source type, that
we couldn't retrieve in the child process. Now that we are in the parent
process, we can actually request them in here from the JS engine.

JS engine uses the sourceHook to fetch this data and return it to us.
Then we put it inside this JS object

This table keeps [uuid, filename] mapping for each source, and each
location string will include the index into source table for the
individual source for the JS frames.

Pushed by canaltinova@gmail.com: https://github.com/mozilla-firefox/firefox/commit/be499e88c865 https://hg.mozilla.org/integration/autoland/rev/541b871b908a Propagate the profile generation information for child processes instead of duplicating them r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/8cfac1e837d9 https://hg.mozilla.org/integration/autoland/rev/c8a8a1619d07 Make ProfileGenerationAdditionalInformation MoveOnly and remove the copy constructors of additional information structs r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/4157a13ac228 https://hg.mozilla.org/integration/autoland/rev/b10d5dae6f40 Add a JS API to get JS script sources r=iain,arai https://github.com/mozilla-firefox/firefox/commit/74799a8be169 https://hg.mozilla.org/integration/autoland/rev/8fb2febc7e83 Add IPC serialization and deserialization methods for the JS sources r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/69d9edab72b8 https://hg.mozilla.org/integration/autoland/rev/590e6d57bb99 Include JS sources in the profile additional information r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/9ffc93a74c33 https://hg.mozilla.org/integration/autoland/rev/b242cb5e012b Add a new JS sources feature to the profiler r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/821c617b840c https://hg.mozilla.org/integration/autoland/rev/69b07ba9fbef Collect the JS sources during profile serialization r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/0d00dcebfc31 https://hg.mozilla.org/integration/autoland/rev/cf970c898695 Stream SourceTable for each process r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/ae7ec10a5d3a https://hg.mozilla.org/integration/autoland/rev/eefbae839f73 Convert the JS sources into JS objects in additionalInfo r=mstange,profiler-reviewers,iain https://github.com/mozilla-firefox/firefox/commit/0f8c64c34a94 https://hg.mozilla.org/integration/autoland/rev/0f5110f69b95 Fetch retrievable sources while converting sources to JS objects inside the parent process r=mstange,profiler-reviewers,iain https://github.com/mozilla-firefox/firefox/commit/e088da1ad9e2 https://hg.mozilla.org/integration/autoland/rev/e1651f9258f8 Add some tests for jsSource retrieval r=mstange,profiler-reviewers

Revert for causing Spidermonkey build bustages due to assertions on Interpreter.cpp.

task 2025-10-21T21:44:26.829+00:00] TEST-PASS | js/src/jit-test/tests/gc/bug-1215678.js | Success (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [11.4 s]
[task 2025-10-21T21:44:26.829+00:00] [1659] Assertion failure: cx->isExceptionPending() || cx->isPropagatingForcedReturn() || cx->hadUncatchableException(), at /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:391
[task 2025-10-21T21:44:26.829+00:00] #01: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19636ee]
[task 2025-10-21T21:44:26.829+00:00] #02: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1962d78]
[task 2025-10-21T21:44:26.829+00:00] #03: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19639c0]
[task 2025-10-21T21:44:26.829+00:00] #04: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964aee]
[task 2025-10-21T21:44:26.829+00:00] #05: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964ceb]
[task 2025-10-21T21:44:26.829+00:00] #06: JS_CallFunction(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)[/builds/worker/workspace/obj-spider/dist/bin/js +0x1aa0a0d]
[task 2025-10-21T21:44:26.829+00:00] #07: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7b858]
[task 2025-10-21T21:44:26.829+00:00] #08: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7b56b]
[task 2025-10-21T21:44:26.829+00:00] #09: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7ac1c]
[task 2025-10-21T21:44:26.829+00:00] #10: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ec73f6]
[task 2025-10-21T21:44:26.829+00:00] #11: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964169]
[task 2025-10-21T21:44:26.829+00:00] #12: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19639a1]
[task 2025-10-21T21:44:26.829+00:00] #13: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964aee]
[task 2025-10-21T21:44:26.829+00:00] #14: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1973ee3]
[task 2025-10-21T21:44:26.829+00:00] #15: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x196337c]
[task 2025-10-21T21:44:26.829+00:00] #16: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1962d6b]
[task 2025-10-21T21:44:26.829+00:00] #17: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19661e3]
[task 2025-10-21T21:44:26.829+00:00] #18: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1966799]
[task 2025-10-21T21:44:26.829+00:00] #19: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ab4b6b]
[task 2025-10-21T21:44:26.829+00:00] #20: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ab4d18]
[task 2025-10-21T21:44:26.829+00:00] #21: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x18f85d0]
[task 2025-10-21T21:44:26.829+00:00] #22: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x18f7d51]
[task 2025-10-21T21:44:26.829+00:00] #23: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1891015]
[task 2025-10-21T21:44:26.829+00:00] #24: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1888332]
[task 2025-10-21T21:44:26.829+00:00] Exit code: -11
[task 2025-10-21T21:44:26.829+00:00] FAIL - gc/bug-1234410.js
[task 2025-10-21T21:44:26.829+00:00] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-1234410.js | [1659] Assertion failure: cx->isExceptionPending() || cx->isPropagatingForcedReturn() || cx->hadUncatchableException(), at /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:391 (code -11, args "") [0.1 s]
[task 2025-10-21T21:44:26.829+00:00] INFO exit-status     : -11
[task 2025-10-21T21:44:26.829+00:00] INFO timed-out       : False
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> [1659] Assertion failure: cx->isExceptionPending() || cx->isPropagatingForcedReturn() || cx->hadUncatchableException(), at /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:391
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #01: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19636ee]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #02: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1962d78]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #03: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19639c0]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #04: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964aee]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #05: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964ceb]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #06: JS_CallFunction(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)[/builds/worker/workspace/obj-spider/dist/bin/js +0x1aa0a0d]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #07: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7b858]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #08: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7b56b]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #09: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1e7ac1c]
[task 2025-10-21T21:44:26.829+00:00] INFO stderr         2> #10: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ec73f6]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #11: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964169]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #12: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19639a1]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #13: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1964aee]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #14: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1973ee3]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #15: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x196337c]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #16: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1962d6b]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #17: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x19661e3]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #18: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1966799]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #19: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ab4b6b]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #20: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)[/builds/worker/workspace/obj-spider/dist/bin/js +0x1ab4d18]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #21: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x18f85d0]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #22: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x18f7d51]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #23: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1891015]
[task 2025-10-21T21:44:26.830+00:00] INFO stderr         2> #24: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1888332]
[task 2025-10-21T21:44:26.830+00:00] TEST-PASS | js/src/jit-test/tests/gc/bug-1215678.js | Success (code 3, args "") [11.5 s]
Flags: needinfo?(canaltinova)
Pushed by canaltinova@gmail.com: https://github.com/mozilla-firefox/firefox/commit/8119220fbc60 https://hg.mozilla.org/integration/autoland/rev/7399eca697f8 Propagate the profile generation information for child processes instead of duplicating them r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/60dddd0a2a48 https://hg.mozilla.org/integration/autoland/rev/de3e72a0b85d Make ProfileGenerationAdditionalInformation MoveOnly and remove the copy constructors of additional information structs r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/b825a49860d2 https://hg.mozilla.org/integration/autoland/rev/d2b130ae8b0f Add a JS API to get JS script sources r=iain,arai https://github.com/mozilla-firefox/firefox/commit/26749bd5991b https://hg.mozilla.org/integration/autoland/rev/61a61d2f62ae Add IPC serialization and deserialization methods for the JS sources r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/228757a213b5 https://hg.mozilla.org/integration/autoland/rev/bc7646f5c0ba Include JS sources in the profile additional information r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/e36f6ef97023 https://hg.mozilla.org/integration/autoland/rev/bd1508c678f5 Add a new JS sources feature to the profiler r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/9681c29277f9 https://hg.mozilla.org/integration/autoland/rev/1074755101ce Collect the JS sources during profile serialization r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/15b0f55ab57e https://hg.mozilla.org/integration/autoland/rev/4f67c422ee46 Stream SourceTable for each process r=mstange,profiler-reviewers https://github.com/mozilla-firefox/firefox/commit/b2fee3b5a19c https://hg.mozilla.org/integration/autoland/rev/74884dc48361 Convert the JS sources into JS objects in additionalInfo r=mstange,profiler-reviewers,iain https://github.com/mozilla-firefox/firefox/commit/3fe4e28cbeb4 https://hg.mozilla.org/integration/autoland/rev/cddbe6345bf9 Fetch retrievable sources while converting sources to JS objects inside the parent process r=mstange,profiler-reviewers,iain https://github.com/mozilla-firefox/firefox/commit/38c7e07c4038 https://hg.mozilla.org/integration/autoland/rev/4915afa07907 Add some tests for jsSource retrieval r=mstange,profiler-reviewers
Flags: needinfo?(canaltinova)
QA Whiteboard: [qa-triage-done-c147/b146]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: