Closed
Bug 817107
Opened 12 years ago
Closed 12 years ago
BaselineCompiler: Mark stubcodes during codegeneration.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: djvj, Unassigned)
References
Details
Attachments
(1 file)
54.04 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•12 years ago
|
||
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.
Attachment #688012 -
Flags: review?(jdemooij)
Comment 2•12 years ago
|
||
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.
Attachment #688012 -
Flags: review?(jdemooij) → review+
Reporter | ||
Comment 3•12 years ago
|
||
Nits addressed and pushed: https://hg.mozilla.org/projects/ionmonkey/rev/d37fb0c8a33b
Reporter | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•