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)

enhancement

Tracking

()

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.
Just a note - I'd like to coordinate this with the scheduler work that we have yet to plan out for 2019.
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)
(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)
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.)
Kannan, can you look into this JS bug.
Flags: needinfo?(kvijayan)
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)
Whiteboard: [qf] → [qf:p4]
Component: JavaScript: GC → JavaScript Engine: JIT

Setting this bug as P3 based on comment 6.

Priority: -- → P3
Performance Impact: --- → P3
Whiteboard: [qf:p4]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.