BaselineCompiler: Have a single allocator per compartment for optimized stubs

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

6 years ago
Right now every BaselineScript has an ICStubSpace (a LifoAlloc) for optimized stubs and one for fallback stubs (also includes stubs that can make calls).

On GC, if a script is not on the stack, its BaselineScript is destroyed. If the script is active on the stack, we only purge all optimized stubs. This means discardJitCode always destroys all optimized stubs, whether the script is active or not, and we can move the ICStubSpace for optimized stubs from BaselineScript to IonCompartment.

This should be pretty straight-forward and is a nice win because it (1) gets rid of a LifoAlloc for every BaselineScript and (2) avoids allocated-but-unused memory at the end of LifoAlloc chunks.
Assignee

Comment 1

6 years ago
Posted patch PatchSplinter Review
As described in comment 0, optimized stubs are now allocated per-compartment.

njn, asking you to review the memory reporter changes. I split baseline-stubs in baseline-optimized-stubs and baseline-fallback-stubs. These are now allocated differently, and optimized stubs are always freed on GC, so it's useful to distinguish the two when analyzing memory usage.
Attachment #721781 - Flags: review?(n.nethercote)
Attachment #721781 - Flags: review?(kvijayan)
Comment on attachment 721781 [details] [diff] [review]
Patch

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

Nice.  No comments - everything looks pretty good.
Attachment #721781 - Flags: review?(kvijayan) → review+
Comment on attachment 721781 [details] [diff] [review]
Patch

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

Is ICStubSpace a compile-time-only structure, or does it persist to runtime?

::: js/src/ion/BaselineIC.h
@@ +894,5 @@
>  
>      ICStubSpace *getStubSpace(JSScript *script) {
> +        if (ICStub::CanMakeCalls(kind))
> +            return script->baselineScript()->fallbackStubSpace();
> +        return script->compartment()->ionCompartment()->optimizedStubSpace();

I like the ?: form better!
Attachment #721781 - Flags: review?(n.nethercote) → review+
Assignee

Comment 4

6 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/e696d62133a4

(In reply to Nicholas Nethercote [:njn] from comment #3)
> Is ICStubSpace a compile-time-only structure, or does it persist to runtime?

It persists to runtime. During compilation, the compiler allocates IC fallback stubs in an ICStubSpace. When compilation is done, these are copied to the ICStubSpace stored in the BaselineScript (attached to JSScript). At runtime, IC's can allocate both new optimized stubs and new fallback stubs.

> ::: js/src/ion/BaselineIC.h
> I like the ?: form better!

Me too, but unfortunately Clang/C++ diagree. If you have

class Base {};
class Derived1 : public Base {};
class Derived2 : public Base {};

You can't do "x ? pointer-to-Derived1 : pointer-to-Derived2", they have to be the same type or one has to be convertible to the other.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.