Closed
Bug 1362786
Opened 7 years ago
Closed 7 years ago
getSymbolsFromNM should run on a worker thread
Categories
(WebExtensions :: Experiments, defect, P1)
WebExtensions
Experiments
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: mstange, Assigned: alexical)
References
Details
(Whiteboard: triaged)
Attachments
(3 files)
It's creating a lot of traffic on the main thread, and about 300ms of jank at the end for libxul. See this profile: https://perfht.ml/2qbvEAd
Comment 1•7 years ago
|
||
I'd initially planned to make a version of the Subprocess.jsm API available to workers, but I never got around to it. It should probably work well enough to transfer the ArrayBuffers from the main thread to a worker and do the processing there, though. Although, it also looks like pathSearch is surprisingly expensive, largely just from creating the environment object. We should probably do that once, rather than once for every nm call.
Reporter | ||
Comment 2•7 years ago
|
||
Would this proposal still cause us to run code on the main thread for every chunk that we read? It would be good to avoid that, even just to reduce GC pressure for the main thread. The period of jank that I mentioned in comment 0 was actually 400ms long, the last 100ms being garbage collection.
Comment 3•7 years ago
|
||
Yes, but they could be quite big chunks. And I don't think GC would be a major issue, since we'd be transferring the ArrayBuffers to the worker thread, and wouldn't be creating very much main thread garbage in the process.
Reporter | ||
Comment 4•7 years ago
|
||
Ok, let's give it a try then.
Comment 5•7 years ago
|
||
should kris or john thayer (sp?) take this?
Flags: needinfo?(amckay)
Priority: -- → P1
Whiteboard: triaged
Comment 6•7 years ago
|
||
Is this something run when the gecko profiler is run, or all the time?
Flags: needinfo?(amckay)
Reporter | ||
Comment 7•7 years ago
|
||
The former.
Reporter | ||
Comment 9•7 years ago
|
||
Tangentially related: getSymbolsFromNM also seems a lot slower than it needs to be, at least for libxul. Last time I looked at a profile for this, the cost mostly seemed to be in the structured cloning of the chunks. I think increasing the size of the chunks (so that there are fewer of them) might already help with the speed. If I run "nm .../XUL > somefile", it takes less than half a second on my machine, so I think there's a lot of room for improvement here.
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Andy McKay [:andym] from comment #8) > Would you be able to look at this Doug? Yeah I can take this.
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
Flags: needinfo?(dothayer)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8888920 [details] Bug 1362786 - (1) Increase subprocess BUFFER_SIZE https://reviewboard.mozilla.org/r/159946/#review167008
Attachment #8888920 -
Flags: review?(kmaglione+bmo) → review+
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8888921 [details] Bug 1362786 - (2) Run NMParser in worker https://reviewboard.mozilla.org/r/159948/#review167022
Attachment #8888921 -
Flags: review?(kmaglione+bmo) → review+
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8888922 [details] Bug 1362786 - (3) Run CppFiltParser in worker https://reviewboard.mozilla.org/r/159950/#review167024
Attachment #8888922 -
Flags: review?(kmaglione+bmo) → review+
Comment 17•7 years ago
|
||
Pushed by dothayer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/242a12eae20e (1) Increase subprocess BUFFER_SIZE r=kmag https://hg.mozilla.org/integration/autoland/rev/71b419e1788d (2) Run NMParser in worker r=kmag https://hg.mozilla.org/integration/autoland/rev/c1a15605e8a2 (3) Run CppFiltParser in worker r=kmag
Comment 18•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/242a12eae20e https://hg.mozilla.org/mozilla-central/rev/71b419e1788d https://hg.mozilla.org/mozilla-central/rev/c1a15605e8a2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•