Closed Bug 1362786 Opened 5 years ago Closed 4 years ago

getSymbolsFromNM should run on a worker thread


(WebExtensions :: Experiments, defect, P1)



(firefox56 fixed)

Tracking Status
firefox56 --- fixed


(Reporter: mstange, Assigned: dthayer)



(Whiteboard: triaged)


(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:
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.
See Also: → 1362788
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.
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.
Ok, let's give it a try then.
should kris or john thayer (sp?) take this?
Flags: needinfo?(amckay)
Priority: -- → P1
Whiteboard: triaged
Is this something run when the gecko profiler is run, or all the time?
Flags: needinfo?(amckay)
The former.
Would you be able to look at this Doug?
Flags: needinfo?(dothayer)
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.
(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
Flags: needinfo?(dothayer)
Comment on attachment 8888920 [details]
Bug 1362786 - (1) Increase subprocess BUFFER_SIZE
Attachment #8888920 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8888921 [details]
Bug 1362786 - (2) Run NMParser in worker
Attachment #8888921 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8888922 [details]
Bug 1362786 - (3) Run CppFiltParser in worker
Attachment #8888922 - Flags: review?(kmaglione+bmo) → review+
Pushed by
(1) Increase subprocess BUFFER_SIZE r=kmag
(2) Run NMParser in worker r=kmag
(3) Run CppFiltParser in worker r=kmag
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.