665404 - Create JaegerCompartments lazily

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Spin-off from bug 661068 -- create JaegerCompartments lazily, because you often have small ones that don't need any method JITting, and currently when we create the TrampolineCompiler we allocate 64KB of memory.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: patch

Pretty straightforward.  Should getJaegerCompartment() be called jaegerCompartment() instead?  This would require decorating .jaegerCompartment.  (A similar question applies to getTraceMonitor() in bug 661068.)

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: patch, v2

The previous patch missed a necessary call to ensureJaegerCompartmentExists().

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 540395 [details] [diff] [review]
patch, v2

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

::: js/src/jscompartment.h
@@ +428,5 @@
> +
> +    js::mjit::JaegerCompartment *getJaegerCompartment() const {
> +        JS_ASSERT(jaegerCompartment);
> +        return jaegerCompartment;
> +    }

We have been using the style that "getX()" is a fallible operation and "x()" is the asserted-infallible field access.  The absence of a 'cx' parameter does also indicate infallibility, but it still look weird to see getX() used infallibly in expressions.  (I know we have some outliers like getParent/getProto; they should be changed/removed with objshrink.)  To resolve the name conflict, you can append a _ if you'd like.

::: js/src/methodjit/MethodJIT-inl.h
@@ +73,5 @@
>      }
> +    if (!cx->compartment->hasJaegerCompartment()) {
> +        bool ok = cx->compartment->ensureJaegerCompartmentExists(cx);
> +        if (!ok)
> +            return Compile_Abort;

Any reason you can't push this down into TryCompile() and factor out these two instances?  Ideally there would we one pinch point where this happens and mjit::Compiler::performCompilation looks reasonable.

Comment 4

[Nicholas Nethercote [inactive]]

> Any reason you can't push this down into TryCompile() and factor out these
> two instances?  Ideally there would we one pinch point where this happens
> and mjit::Compiler::performCompilation looks reasonable.

Good idea, I'll do that.  I'll also rename getJaegerCompartment() as jaegerCompartment().  (I did likewise with getTraceMonitor() in bug 661068.)

Comment 5

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/3bd218337175

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/3bd218337175