Open
Bug 1514113
Opened 6 years ago
Updated 2 years ago
Move ReprotectRegion and mprotect off the main thread
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Core
JavaScript Engine: JIT
Tracking
()
NEW
Performance Impact | low |
People
(Reporter: cpeterson, Unassigned)
References
(Blocks 1 open bug)
Details
In his analysis of Firefox Reality performance on the Oculus Go headset in bug 1498485 comment 25, Kannan says:
> 4. We _need_ to get ReprotectRegion and mprotect off the main thread. We are spending 4% of our total time in this call alone.
Comment 1•6 years ago
|
||
Just a note - I'd like to coordinate this with the scheduler work that we have yet to plan out for 2019.
Comment 2•6 years ago
|
||
AIUI ReprotectRegion is used to change the permissions of code generated by the JITs. It's not clear to me that that can be moved off thread, but needinfoing Jan here as he will know more.
Flags: needinfo?(jdemooij)
Comment 3•6 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #2)
> AIUI ReprotectRegion is used to change the permissions of code generated by
> the JITs. It's not clear to me that that can be moved off thread, but
> needinfoing Jan here as he will know more.
Yeah it's complicated. Maybe it could happen in parallel with other work that happens on the main thread before we return to JS, but it's not trivial. Or maybe we could batch them; Ion IC stubs for example.
FWIW bug 1499324 should improve this a bit by Baseline compiling fewer scripts.
Flags: needinfo?(jdemooij)
Comment 4•6 years ago
|
||
Note that removing permissions from memory generally involves interrupting any other CPUs running in the same address space to invalidate TLBs, so an off-thread mprotect could still have a cost. (The context for this bug is on ARM, which I know a lot less about at the system level than x86, but I assume similar principles apply.)
Comment 6•6 years ago
|
||
Yeah this is not an easy fix, but we can take it bit by bit.
In the early stages, moving this off of thread will at least take the syscall overhead asynchronous.
Ultimately, though - we want to adopt a multi-process approach to this: have a "compile server" that generates code, which is responsible for allocating writable memory that is mapped as executable in the client process.
Frankly, given the recent findings on scheduling being a much bigger issue, and the fact that this shows up as maybe a 2% cost at best, it's more appropriate to make this change are part of a coherent set of changes to the way we generate code, which is a longer-term effort.
Flags: needinfo?(kvijayan)
Updated•6 years ago
|
Whiteboard: [qf] → [qf:p4]
Updated•6 years ago
|
Component: JavaScript: GC → JavaScript Engine: JIT
Updated•3 years ago
|
Performance Impact: --- → P3
Whiteboard: [qf:p4]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•