1416572 - Use a single JitCode instance for all VMFunction wrappers

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

We create VM wrappers for all VMFunctions when we initialize a JitRuntime. There are currently 263 of these wrappers and for each of them we have to initialize a MacroAssembler, allocate a JitCode GC thing, mprotect the code, etc. This can be pretty slow, especially on mobile.

Once upon a time, we used to allocate VMFunction wrappers lazily and so it was necessary for each of them to have its own JitCode. Now that wrappers are never collected and all have the same lifetime, we can just use a single MacroAssembler/JitCode and for each wrapper save the offset into this buffer.

On OS X this improves the time to generate all VMFunction wrappers from 1.0-1.5 ms to 0.15 ms or so.

There's more we can improve after this:

* We can move the other JitRuntime trampolines into this shared JitCode instance as well.

* After that we could consider getting rid of this shared JitCode instance and just share trampoline/wrapper code across all runtimes. That's not too easy because trampolines currently bake in JSRuntime* pointers, but we could pass the runtime or context as argument.

* Exit frames currently store the wrapper's JitCode so the GC can mark it. This used to be necessary but since wrappers are never collected nowadays, we could get rid of this field (and then the VMFunction* pointer could be used to store the ExitFrameToken).

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> This can be pretty slow, especially on mobile.

On mobile this was extra slow because of the icache flushing we used to do for each wrapper we allocate. We will now flush the whole range at once.

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8927666 [details] [diff] [review]
Patch

Review of attachment 8927666 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/Ion.cpp
@@ +748,5 @@
>  
> +    uint32_t offset = p->value();
> +    MOZ_ASSERT(offset < functionWrapperCode_->instructionsSize());
> +
> +    return functionWrapperCode_->raw() + offset;

follow-up: It is weird to see non-checked pointers. I suggest we change the return type to JitCodeInnerPtr, with something like:

  using JitCodeInnerPtr = NotNull<uint8_t*>;

I guess doing that for JitCode::raw() might cause us to propagate this new type to the MacroAssembler, for the best.

::: js/src/jit/x86-shared/MacroAssembler-x86-shared.h
@@ +540,5 @@
>      }
>      void jump(JitCode* code) {
>          jmp(code);
>      }
> +    void jump(ImmPtr code) {

nit: Having a JitCodeInnerPtr should help remove this use case.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8d8a1764cba
Use a single JitCode instance for all VMFunction wrappers. r=nbp

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> > +    void jump(ImmPtr code) {
> 
> nit: Having a JitCodeInnerPtr should help remove this use case.

I'm not sure if it's worth the trouble - having a jump(ImmPtr) method doesn't seem too unreasonable to me.

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c8d8a1764cba