817107 - BaselineCompiler: Mark stubcodes during codegeneration.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kannan Vijayan [:djvj]]

GC might happen when the baseline compiler is doing codegen for a script.  This may collect IonCode for the fallback stubs which have already been generated.  Currently we abort compilation if we notice that GC has occurred during codegen.

We should ensure that stubcodes referenced by an in-progress compiler are marked properly so the abort-on-gc can be avoided.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: Mark StubCodes during compilation and do proper stub allocation.

This patch contains two GC-related changes.

1. A new AutoRooter type is added to BaselineCompiler, which is used to traverse  the ICEntry vector that the compiler keeps during compilation, and mark the stubCodes for fallback stubs (and TypeMonitor_Fallback stubs) generated during compilation.

2. A new ICStubSpace class is added to BaselineIC.h, which wraps LifoAlloc and represents memory space used to allocate stub state.  Every BaselineScript is endowed with two of these spaces: one to hold fallback stub state (which persists across GCs), and one to hold optimized stub state (which may be cleared on GC).

BaselineCompiler (in BaselineCompiler-shared.h) also acquires as ICStubSpace, to use to allocate the fallback stub state.  When a BaselineScript is created, its fallbackStubSpace adopts the allocated memory in the BaselineCompiler's stubSpace.

To enable stub allocation from spaces, APIs for getStub, New (constructor on ICStub::Compiler variants), and a few other methods have changed to accept an ICStubSpace * argument.

All uses of getStub have been changed to get and pass the appropriate ICStubSpace.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 688012 [details] [diff] [review]
Mark StubCodes during compilation and do proper stub allocation.

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

Nice! Good to have these problems fixed. r=me with comments below addressed.

::: js/src/ion/BaselineCompiler.cpp
@@ +113,5 @@
> +        ICEntry &entry = icEntries_[i];
> +        JS_ASSERT(entry.firstStub() != NULL);
> +        JS_ASSERT(entry.firstStub()->isFallback());
> +
> +        // Mark the stubcode for the fallback stub's IonCode for the ICEntry.

We should call TraceStub here, maybe turn TraceStub into a (static) ICStub::Trace we can call here and in BaselineScript::trace.

::: js/src/ion/BaselineIC.h
@@ +209,5 @@
> +    inline void adoptFrom(ICStubSpace *other) {
> +        allocator_.transferFrom(&(other->allocator_));
> +    }
> +
> +    static inline ICStubSpace *fallbackStubSpaceFor(JSScript *script) {

Nit: capitalize first letter for static methods.

@@ +214,5 @@
> +        JS_ASSERT(script->hasBaselineScript());
> +        return script->baselineScript()->fallbackStubSpace();
> +    }
> +
> +    static inline ICStubSpace *stubSpaceFor(JSScript *script) {

And here.

@@ +342,5 @@
>  #undef DEF_ENUM_KIND
>          LIMIT
>      };
>  
> +    static inline bool isValidKind(Kind k) {

Same here.

@@ +347,4 @@
>          return (k > INVALID) && (k < LIMIT);
>      }
>  
> +    static const char *kindString(Kind k) {

Ditto.

@@ +900,5 @@
>      { }
>  
>    public:
> +    static inline ICTypeMonitor_TypeObject *New(
> +            ICStubSpace *space, IonCode *code, HandleTypeObject type)

Nit: format like

    static inline ICTypeMonitor_TypeObject *New(ICStubSpace *space, IonCode *code,
                                                HandleTypeObject type)

@@ +1418,1 @@
>                  delete stub;

Nit: we can remove "delete stub;" here and maybe elsewhere in this file.

::: js/src/ion/BaselineJIT.h
@@ +28,5 @@
>      // Code pointer containing the actual method.
>      HeapPtr<IonCode> method_;
>  
> +    // Allocated space for fallback stubs.
> +    ICStubSpace *fallbackStubSpace_;

We should store ICStubSpace here directly:

ICStubSpace optimizedStubSpace_;

For this to work you probably have to move the ICStubSpace definition to this file, and move the ICStubSpace method implementation to BaselineIC.cpp.

Comment 3

[Kannan Vijayan [:djvj]]

Nits addressed and pushed:
https://hg.mozilla.org/projects/ionmonkey/rev/d37fb0c8a33b