1514113 - Move ReprotectRegion and mprotect off the main thread

Status: NEW

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Chris Peterson [:cpeterson]]

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

[Kannan Vijayan [:djvj]]

Just a note - I'd like to coordinate this with the scheduler work that we have yet to plan out for 2019.

Comment 2

[Jon Coppeard (:jonco)]

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.

Comment 3

[Jan de Mooij [:jandem]]

(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.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

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 5

[Jean Gong :jgong]

Kannan, can you look into this JS bug.

Comment 6

[Kannan Vijayan [:djvj]]

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.

Comment 7

[Nicolas B. Pierron [:nbp]]

Setting this bug as P3 based on comment 6.