From e622792d83bd158158e0bbf2ef74bbf311a3e78b Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Wed, 23 Mar 2011 11:57:44 -0700 Subject: [PATCH] Bug 569422 - Allocate js::Shape from the GC heap (r=brendan) --- js/src/jsapi.cpp | 12 +- js/src/jsapi.h | 1 + js/src/jscompartment.cpp | 17 +- js/src/jsdbgapi.cpp | 4 +- js/src/jsgc.cpp | 36 ++-- js/src/jsgc.h | 8 +- js/src/jsgcinlines.h | 62 +++++- js/src/jsgcstats.cpp | 5 + js/src/jsobj.h | 6 +- js/src/jsobjinlines.h | 16 +- js/src/jsparse.cpp | 2 +- js/src/jspropertytree.cpp | 386 +++++++++++--------------------------- js/src/jspropertytree.h | 15 +- js/src/jsscope.cpp | 46 ++++- js/src/jsscope.h | 59 ++---- js/src/jsscopeinlines.h | 2 + js/src/jsscript.cpp | 4 +- js/src/jstracer.cpp | 12 +- 18 files changed, 293 insertions(+), 400 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 8fe692f474b8..66b4cf676bde 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2286,6 +2286,10 @@ JS_PrintTraceThingInfo(char *buf, size_t bufsize, JSTracer *trc, void *thing, ui : "string"; break; + case JSTRACE_SHAPE: + name = "shape"; + break; + #if JS_HAS_XML_SUPPORT case JSTRACE_XML: name = "xml"; @@ -2341,6 +2345,12 @@ JS_PrintTraceThingInfo(char *buf, size_t bufsize, JSTracer *trc, void *thing, ui break; } + case JSTRACE_SHAPE: + { + JS_snprintf(buf, bufsize, ""); + break; + } + #if JS_HAS_XML_SUPPORT case JSTRACE_XML: { @@ -3982,7 +3992,7 @@ prop_iter_trace(JSTracer *trc, JSObject *obj) if (obj->getSlot(JSSLOT_ITER_INDEX).toInt32() < 0) { /* Native case: just mark the next property to visit. */ - ((Shape *) pdata)->trace(trc); + MarkShape(trc, (Shape *)pdata, "prop iter shape"); } else { /* Non-native case: mark each id in the JSIdArray private. */ JSIdArray *ida = (JSIdArray *) pdata; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 9887313c93b4..eeadeb03e578 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -1576,6 +1576,7 @@ JS_SetExtraGCRoots(JSRuntime *rt, JSTraceDataOp traceOp, void *data); /* Trace kinds to pass to JS_Tracing. */ #define JSTRACE_OBJECT 0 #define JSTRACE_STRING 1 +#define JSTRACE_SHAPE 2 /* * Use the following macros to check if a particular jsval is a traceable diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 4c6e2e7fdcae..8ec67dbbb0df 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -98,8 +98,6 @@ JSCompartment::JSCompartment(JSRuntime *rt) JSCompartment::~JSCompartment() { - propertyTree.finish(); - #if ENABLE_YARR_JIT js_delete(regExpAllocator); #endif @@ -134,9 +132,6 @@ JSCompartment::init() if (!crossCompartmentWrappers.init()) return false; - if (!propertyTree.init()) - return false; - #ifdef DEBUG if (rt->meterEmptyShapes()) { if (!emptyShapes.init()) @@ -477,17 +472,17 @@ JSCompartment::sweep(JSContext *cx, uint32 releaseInterval) } /* Remove dead empty shapes. */ - if (emptyArgumentsShape && !emptyArgumentsShape->marked()) + if (emptyArgumentsShape && IsAboutToBeFinalized(cx, emptyArgumentsShape)) emptyArgumentsShape = NULL; - if (emptyBlockShape && !emptyBlockShape->marked()) + if (emptyBlockShape && IsAboutToBeFinalized(cx, emptyBlockShape)) emptyBlockShape = NULL; - if (emptyCallShape && !emptyCallShape->marked()) + if (emptyCallShape && IsAboutToBeFinalized(cx, emptyCallShape)) emptyCallShape = NULL; - if (emptyDeclEnvShape && !emptyDeclEnvShape->marked()) + if (emptyDeclEnvShape && IsAboutToBeFinalized(cx, emptyDeclEnvShape)) emptyDeclEnvShape = NULL; - if (emptyEnumeratorShape && !emptyEnumeratorShape->marked()) + if (emptyEnumeratorShape && IsAboutToBeFinalized(cx, emptyEnumeratorShape)) emptyEnumeratorShape = NULL; - if (emptyWithShape && !emptyWithShape->marked()) + if (emptyWithShape && IsAboutToBeFinalized(cx, emptyWithShape)) emptyWithShape = NULL; #ifdef JS_TRACER diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index dc1331d04461..6058733c81fc 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -646,9 +646,9 @@ js_TraceWatchPoints(JSTracer *trc) &wp->links != &rt->watchPointList; wp = (JSWatchPoint *)wp->links.next) { if (wp->object->isMarked()) { - if (!wp->shape->marked()) { + if (!wp->shape->isMarked()) { modified = true; - wp->shape->trace(trc); + MarkShape(trc, wp->shape, "shape"); } if (wp->shape->hasSetterValue() && wp->setter) { if (!CastAsObject(wp->setter)->isMarked()) { diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 9cb03acfc28d..fbcc25decef4 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -108,13 +108,14 @@ using namespace js::gc; */ JS_STATIC_ASSERT(JSTRACE_OBJECT == 0); JS_STATIC_ASSERT(JSTRACE_STRING == 1); -JS_STATIC_ASSERT(JSTRACE_XML == 2); +JS_STATIC_ASSERT(JSTRACE_SHAPE == 2); +JS_STATIC_ASSERT(JSTRACE_XML == 3); /* - * JS_IS_VALID_TRACE_KIND assumes that JSTRACE_STRING is the last non-xml + * JS_IS_VALID_TRACE_KIND assumes that JSTRACE_SHAPE is the last non-xml * trace kind when JS_HAS_XML_SUPPORT is false. */ -JS_STATIC_ASSERT(JSTRACE_STRING + 1 == JSTRACE_XML); +JS_STATIC_ASSERT(JSTRACE_SHAPE + 1 == JSTRACE_XML); /* * Everything we store in the heap must be a multiple of the cell size. @@ -123,6 +124,7 @@ JS_STATIC_ASSERT(sizeof(JSString) % sizeof(FreeCell) == 0); JS_STATIC_ASSERT(sizeof(JSShortString) % sizeof(FreeCell) == 0); JS_STATIC_ASSERT(sizeof(JSObject) % sizeof(FreeCell) == 0); JS_STATIC_ASSERT(sizeof(JSFunction) % sizeof(FreeCell) == 0); +JS_STATIC_ASSERT(sizeof(Shape) % sizeof(FreeCell) == 0); #ifdef JSXML JS_STATIC_ASSERT(sizeof(JSXML) % sizeof(FreeCell) == 0); #endif @@ -135,6 +137,7 @@ JS_STATIC_ASSERT(sizeof(Arena) == 4096); JS_STATIC_ASSERT(sizeof(Arena) == 4096); JS_STATIC_ASSERT(sizeof(Arena) == 4096); JS_STATIC_ASSERT(sizeof(Arena) == 4096); +JS_STATIC_ASSERT(sizeof(Arena) == 4096); JS_STATIC_ASSERT(sizeof(Arena) == 4096); #ifdef JS_GCMETER @@ -241,6 +244,7 @@ checkArenaListsForThing(JSCompartment *comp, void *thing) comp->arenas[FINALIZE_OBJECT12].arenasContainThing(thing) || comp->arenas[FINALIZE_OBJECT16].arenasContainThing(thing) || comp->arenas[FINALIZE_FUNCTION].arenasContainThing(thing) || + comp->arenas[FINALIZE_FUNCTION].arenasContainThing(thing) || #if JS_HAS_XML_SUPPORT comp->arenas[FINALIZE_XML].arenasContainThing(thing) || #endif @@ -677,6 +681,9 @@ MarkIfGCThingWord(JSTracer *trc, jsuword w, uint32 &thingKind) case FINALIZE_FUNCTION: test = MarkCell(cell, trc); break; + case FINALIZE_SHAPE: + test = MarkCell(cell, trc); + break; #if JS_HAS_XML_SUPPORT case FINALIZE_XML: test = MarkCell(cell, trc); @@ -1198,6 +1205,8 @@ RefillFinalizableFreeList(JSContext *cx, unsigned thingKind) return RefillTypedFreeList(cx, thingKind); case FINALIZE_FUNCTION: return RefillTypedFreeList(cx, thingKind); + case FINALIZE_SHAPE: + return RefillTypedFreeList(cx, thingKind); #if JS_HAS_XML_SUPPORT case FINALIZE_XML: return RefillTypedFreeList(cx, thingKind); @@ -1269,6 +1278,11 @@ JS_TraceChildren(JSTracer *trc, void *thing, uint32 kind) break; } + case JSTRACE_SHAPE: { + MarkChildren(trc, (Shape *)thing); + break; + } + #if JS_HAS_XML_SUPPORT case JSTRACE_XML: MarkChildren(trc, (JSXML *)thing); @@ -1408,6 +1422,9 @@ GCMarker::markDelayedChildren() case FINALIZE_FUNCTION: reinterpret_cast *>(a)->markDelayedChildren(this); break; + case FINALIZE_SHAPE: + reinterpret_cast *>(a)->markDelayedChildren(this); + break; #if JS_HAS_XML_SUPPORT case FINALIZE_XML: reinterpret_cast *>(a)->markDelayedChildren(this); @@ -1512,7 +1529,7 @@ AutoGCRooter::trace(JSTracer *trc) return; case SHAPE: - static_cast(this)->shape->trace(trc); + MarkShape(trc, static_cast(this)->shape, "js::AutoShapeRooter.val"); return; case PARSER: @@ -1934,6 +1951,7 @@ JSCompartment::finalizeObjectArenaLists(JSContext *cx) FinalizeArenaList(this, cx, FINALIZE_OBJECT12); FinalizeArenaList(this, cx, FINALIZE_OBJECT16); FinalizeArenaList(this, cx, FINALIZE_FUNCTION); + FinalizeArenaList(this, cx, FINALIZE_SHAPE); #if JS_HAS_XML_SUPPORT FinalizeArenaList(this, cx, FINALIZE_XML); #endif @@ -2286,14 +2304,6 @@ MarkAndSweepCompartment(JSContext *cx, JSCompartment *comp, JSGCInvocationKind g comp->finalizeStringArenaLists(cx); TIMESTAMP(sweepStringEnd); - /* - * Unmark all shapes. Even a per-compartment GC can mark shapes in other - * compartments, and we need to clear these bits. See bug 635873. This will - * be fixed in bug 569422. - */ - for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); ++c) - (*c)->propertyTree.unmarkShapes(cx); - PropertyTree::dumpShapes(cx); TIMESTAMP(sweepShapeEnd); @@ -2419,7 +2429,7 @@ MarkAndSweep(JSContext *cx, JSGCInvocationKind gckind GCTIMER_PARAM) * unreachable compartments. */ for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); ++c) - (*c)->propertyTree.sweepShapes(cx); + (*c)->propertyTree.dumpShapeStats(); PropertyTree::dumpShapes(cx); TIMESTAMP(sweepShapeEnd); diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 1f2dd4b222c1..657241688e61 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -93,6 +93,7 @@ enum FinalizeKind { FINALIZE_OBJECT16, FINALIZE_OBJECT_LAST = FINALIZE_OBJECT16, FINALIZE_FUNCTION, + FINALIZE_SHAPE, #if JS_HAS_XML_SUPPORT FINALIZE_XML, #endif @@ -513,12 +514,12 @@ GetArena(Cell *cell) return reinterpret_cast *>(cell->arena()); } -#define JSTRACE_XML 2 +#define JSTRACE_XML 3 /* * One past the maximum trace kind. */ -#define JSTRACE_LIMIT 3 +#define JSTRACE_LIMIT 4 /* * Lower limit after which we limit the heap growth @@ -546,6 +547,7 @@ GetFinalizableTraceKind(size_t thingKind) JSTRACE_OBJECT, /* FINALIZE_OBJECT12 */ JSTRACE_OBJECT, /* FINALIZE_OBJECT16 */ JSTRACE_OBJECT, /* FINALIZE_FUNCTION */ + JSTRACE_SHAPE, /* FINALIZE_SHAPE */ #if JS_HAS_XML_SUPPORT /* FINALIZE_XML */ JSTRACE_XML, #endif @@ -1071,7 +1073,7 @@ namespace gc { #if JS_HAS_XML_SUPPORT # define JS_IS_VALID_TRACE_KIND(kind) ((uint32)(kind) < JSTRACE_LIMIT) #else -# define JS_IS_VALID_TRACE_KIND(kind) ((uint32)(kind) <= JSTRACE_STRING) +# define JS_IS_VALID_TRACE_KIND(kind) ((uint32)(kind) <= JSTRACE_SHAPE) #endif /* diff --git a/js/src/jsgcinlines.h b/js/src/jsgcinlines.h index c2f240db2afe..1ac24c7436a0 100644 --- a/js/src/jsgcinlines.h +++ b/js/src/jsgcinlines.h @@ -102,6 +102,9 @@ JSAtom::isStatic(const void *ptr) } namespace js { + +struct Shape; + namespace gc { inline uint32 @@ -170,7 +173,8 @@ NewFinalizableGCThing(JSContext *cx, unsigned thingKind) #ifdef JS_THREADSAFE JS_ASSERT_IF((cx->compartment == cx->runtime->atomsCompartment), (thingKind == js::gc::FINALIZE_STRING) || - (thingKind == js::gc::FINALIZE_SHORT_STRING)); + (thingKind == js::gc::FINALIZE_SHORT_STRING) || + (thingKind == js::gc::FINALIZE_SHAPE)); #endif METER(cx->compartment->compartmentStats[thingKind].alloc++); @@ -231,6 +235,12 @@ js_NewGCFunction(JSContext *cx) return fun; } +inline js::Shape * +js_NewGCShape(JSContext *cx) +{ + return NewFinalizableGCThing(cx, js::gc::FINALIZE_SHAPE); +} + #if JS_HAS_XML_SUPPORT inline JSXML * js_NewGCXML(JSContext *cx) @@ -251,6 +261,9 @@ TypedMarker(JSTracer *trc, JSObject *thing); static JS_ALWAYS_INLINE void TypedMarker(JSTracer *trc, JSFunction *thing); +static JS_ALWAYS_INLINE void +TypedMarker(JSTracer *trc, const Shape *thing); + static JS_ALWAYS_INLINE void TypedMarker(JSTracer *trc, JSShortString *thing); @@ -324,6 +337,16 @@ MarkObject(JSTracer *trc, JSObject &obj, const char *name) Mark(trc, &obj); } +static inline void +MarkShape(JSTracer *trc, const Shape *shape, const char *name) +{ + JS_ASSERT(trc); + JS_ASSERT(shape); + JS_SET_TRACING_NAME(trc, name); + JS_ASSERT(GetArena((Cell *)shape)->assureThingIsAligned((void *)shape)); + Mark(trc, shape); +} + void MarkObjectSlots(JSTracer *trc, JSObject *obj); @@ -344,7 +367,7 @@ MarkChildren(JSTracer *trc, JSObject *obj) int count = FINALIZE_OBJECT_LAST - FINALIZE_OBJECT0 + 1; for (int i = 0; i < count; i++) { if (obj->emptyShapes[i]) - obj->emptyShapes[i]->trace(trc); + MarkShape(trc, obj->emptyShapes[i], "emptyShape"); } } @@ -376,6 +399,12 @@ MarkChildren(JSTracer *trc, JSString *str) } } +static inline void +MarkChildren(JSTracer *trc, const Shape *shape) +{ + shape->markChildren(trc); +} + #ifdef JS_HAS_XML_SUPPORT static inline void MarkChildren(JSTracer *trc, JSXML *xml) @@ -449,6 +478,30 @@ TypedMarker(JSTracer *trc, JSShortString *thing) (void) thing->markIfUnmarked(); } +static JS_ALWAYS_INLINE void +TypedMarker(JSTracer *trc, const Shape *thing) +{ + JS_ASSERT(thing); + JS_ASSERT(JSTRACE_SHAPE == GetFinalizableTraceKind(thing->arena()->header()->thingKind)); + + GCMarker *gcmarker = static_cast(trc); + if (!thing->markIfUnmarked(gcmarker->getMarkColor())) + return; + + /* + * We regenerate the shape number early. If we did it inside MarkChildren, + * then it might be called multiple times during delayed marking, which + * would be incorrect. However, this does mean that Shape::regenerate + * shouldn't use too much stack. + */ + thing->regenerate(trc); + + if (RecursionTooDeep(gcmarker)) + gcmarker->delayMarkingChildren(thing); + else + MarkChildren(trc, thing); +} + static inline void MarkAtomRange(JSTracer *trc, size_t len, JSAtom **vec, const char *name) { @@ -518,6 +571,9 @@ MarkKind(JSTracer *trc, void *thing, uint32 kind) case JSTRACE_STRING: MarkString(trc, reinterpret_cast(thing)); break; + case JSTRACE_SHAPE: + Mark(trc, reinterpret_cast(thing)); + break; #if JS_HAS_XML_SUPPORT case JSTRACE_XML: Mark(trc, reinterpret_cast(thing)); @@ -565,7 +621,7 @@ MarkShapeRange(JSTracer *trc, const Shape **beg, const Shape **end, const char * { for (const Shape **sp = beg; sp < end; ++sp) { JS_SET_TRACING_INDEX(trc, name, sp - beg); - (*sp)->trace(trc); + MarkShape(trc, *sp, name); } } diff --git a/js/src/jsgcstats.cpp b/js/src/jsgcstats.cpp index 83d379efa35c..fb6aa7ac45e1 100644 --- a/js/src/jsgcstats.cpp +++ b/js/src/jsgcstats.cpp @@ -119,6 +119,7 @@ static const char *const GC_ARENA_NAMES[] = { "object_12", "object_16", "function", + "shape", #if JS_HAS_XML_SUPPORT "xml", #endif @@ -360,6 +361,10 @@ GCMarker::dumpConservativeRoots() fprintf(fp, "object %s", obj->getClass()->name); break; } + case JSTRACE_SHAPE: { + fprintf(fp, "shape"); + break; + } case JSTRACE_STRING: { JSString *str = (JSString *) i->thing; if (str->isLinear()) { diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 7b61714b0bad..680bdf2f7eb3 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -210,9 +210,9 @@ MeterEntryCount(uintN count); } /* namespace js */ -struct JSObjectMap { - uint32 shape; /* shape identifier */ - uint32 slotSpan; /* one more than maximum live slot number */ +struct JSObjectMap : public js::gc::Cell { + mutable uint32 shape; /* shape identifier */ + uint32 slotSpan; /* one more than maximum live slot number */ static JS_FRIEND_DATA(JSObjectMap) sharedNonNative; diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 483bf2c03e0a..db504b67d9f2 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -134,16 +134,13 @@ JSObject::trace(JSTracer *trc) JSContext *cx = trc->context; js::Shape *shape = lastProp; + MarkShape(trc, shape, "shape"); + if (IS_GC_MARKING_TRACER(trc) && cx->runtime->gcRegenShapes) { /* - * Either this object has its own shape, which must be regenerated, or - * it must have the same shape as lastProp. + * MarkShape will regenerate the shape if need be. However, we need to + * regenerate our shape if hasOwnShape() is true. */ - if (!shape->hasRegenFlag()) { - shape->shape = js_RegenerateShapeForGC(cx->runtime); - shape->setRegenFlag(); - } - uint32 newShape = shape->shape; if (hasOwnShape()) { newShape = js_RegenerateShapeForGC(cx->runtime); @@ -151,11 +148,6 @@ JSObject::trace(JSTracer *trc) } objShape = newShape; } - - /* Trace our property tree or dictionary ancestor line. */ - do { - shape->trace(trc); - } while ((shape = shape->parent) != NULL && !shape->marked()); } inline void diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 2e1f8b4de882..2d00e9faef73 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -358,7 +358,7 @@ Parser::trace(JSTracer *trc) } if (emptyCallShape) - emptyCallShape->trace(trc); + MarkShape(trc, emptyCallShape, "emptyCallShape"); for (JSTreeContext *tc = this->tc; tc; tc = tc->parent) tc->trace(trc); diff --git a/js/src/jspropertytree.cpp b/js/src/jspropertytree.cpp index 6376ebe642f1..cd8ac099ab9f 100644 --- a/js/src/jspropertytree.cpp +++ b/js/src/jspropertytree.cpp @@ -40,7 +40,6 @@ #include #include "jstypes.h" -#include "jsarena.h" #include "jsprf.h" #include "jsapi.h" #include "jscntxt.h" @@ -65,40 +64,14 @@ ShapeHasher::match(const Key k, const Lookup l) return l->matches(k); } -bool -PropertyTree::init() -{ - JS_InitArenaPool(&arenaPool, "properties", - 256 * sizeof(Shape), sizeof(void *), NULL); - return true; -} - -void -PropertyTree::finish() -{ - JS_FinishArenaPool(&arenaPool); -} - Shape * PropertyTree::newShape(JSContext *cx) { - Shape *shape; - - shape = freeList; - if (shape) { - shape->removeFree(); - } else { - JS_ARENA_ALLOCATE_CAST(shape, Shape *, &arenaPool, sizeof(Shape)); - if (!shape) { - JS_ReportOutOfMemory(cx); - return NULL; - } + Shape *shape = js_NewGCShape(cx); + if (!shape) { + JS_ReportOutOfMemory(cx); + return NULL; } - -#ifdef DEBUG - shape->compartment = compartment; -#endif - JS_COMPARTMENT_METER(compartment->livePropTreeNodes++); JS_COMPARTMENT_METER(compartment->totalPropTreeNodes++); return shape; @@ -136,7 +109,7 @@ PropertyTree::insertChild(JSContext *cx, Shape *parent, Shape *child) JS_ASSERT(!JSID_IS_VOID(parent->id)); JS_ASSERT(!JSID_IS_VOID(child->id)); JS_ASSERT(cx->compartment == compartment); - JS_ASSERT(child->compartment == parent->compartment); + JS_ASSERT(child->compartment() == parent->compartment()); KidsPointer *kidp = &parent->kids; @@ -173,19 +146,15 @@ PropertyTree::insertChild(JSContext *cx, Shape *parent, Shape *child) } void -PropertyTree::removeChild(Shape *child) +Shape::removeChild(Shape *child) { JS_ASSERT(!child->inDictionary()); + JS_ASSERT(!JSID_IS_VOID(id)); - Shape *parent = child->parent; - JS_ASSERT(parent); - JS_ASSERT(!JSID_IS_VOID(parent->id)); - - KidsPointer *kidp = &parent->kids; + KidsPointer *kidp = &kids; if (kidp->isShape()) { - Shape *kid = kidp->toShape(); - if (kid == child) - parent->kids.setNull(); + JS_ASSERT(kidp->toShape() == child); + kids.setNull(); return; } @@ -300,8 +269,6 @@ Shape::dump(JSContext *cx, FILE *fp) const DUMP_FLAG(ALIAS, alias); DUMP_FLAG(HAS_SHORTID, has_shortid); DUMP_FLAG(METHOD, method); - DUMP_FLAG(MARK, mark); - DUMP_FLAG(SHAPE_REGEN, shape_regen); DUMP_FLAG(IN_DICTIONARY, in_dictionary); #undef DUMP_FLAG fputs(") ", fp); @@ -369,270 +336,145 @@ Shape::dumpSubtree(JSContext *cx, int level, FILE *fp) const #endif /* DEBUG */ -JS_ALWAYS_INLINE void -js::PropertyTree::orphanChildren(Shape *shape) +void +Shape::finalize(JSContext *cx) { - KidsPointer *kidp = &shape->kids; +#ifdef DEBUG + if ((flags & SHARED_EMPTY) && cx->runtime->meterEmptyShapes()) + compartment()->emptyShapes.remove((EmptyShape *)this); +#endif - JS_ASSERT(!kidp->isNull()); - - if (kidp->isShape()) { - Shape *kid = kidp->toShape(); - - if (!JSID_IS_VOID(kid->id)) { - JS_ASSERT(kid->parent == shape); - kid->parent = NULL; - } + if (inDictionary()) { + JS_COMPARTMENT_METER(compartment()->liveDictModeNodes--); } else { - KidsHash *hash = kidp->toHash(); + if (parent && parent->isMarked()) + parent->removeChild(this); - for (KidsHash::Range range = hash->all(); !range.empty(); range.popFront()) { - Shape *kid = range.front(); - if (!JSID_IS_VOID(kid->id)) { - JS_ASSERT(kid->parent == shape); - kid->parent = NULL; - } - } - - hash->~KidsHash(); - js_free(hash); + if (kids.isHash()) + js_delete(kids.toHash()); } - kidp->setNull(); + freeTable(cx); + JS_COMPARTMENT_METER(compartment()->livePropTreeNodes--); } void -js::PropertyTree::sweepShapes(JSContext *cx) +js::PropertyTree::dumpShapeStats() { +#ifdef DEBUG JSRuntime *rt = compartment->rt; -#ifdef DEBUG JSBasicStats bs; - uint32 livePropCapacity = 0, totalLiveCount = 0; static FILE *logfp; if (!logfp) { if (const char *filename = rt->propTreeStatFilename) logfp = fopen(filename, "w"); + if (!logfp) + return; } - if (logfp) { - JS_BASIC_STATS_INIT(&bs); + JS_BASIC_STATS_INIT(&bs); - uint32 empties; - { - typedef JSCompartment::EmptyShapeSet HS; + uint32 empties; + { + typedef JSCompartment::EmptyShapeSet HS; - HS &h = compartment->emptyShapes; - empties = h.count(); - MeterKidCount(&bs, empties); - for (HS::Range r = h.all(); !r.empty(); r.popFront()) - meter(&bs, r.front()); - } - - double props = rt->liveObjectPropsPreSweep; - double nodes = compartment->livePropTreeNodes; - double dicts = compartment->liveDictModeNodes; - - /* Empty scope nodes are never hashed, so subtract them from nodes. */ - JS_ASSERT(nodes - dicts == bs.sum); - nodes -= empties; - - double sigma; - double mean = JS_MeanAndStdDevBS(&bs, &sigma); - - fprintf(logfp, - "props %g nodes %g (dicts %g) beta %g meankids %g sigma %g max %u\n", - props, nodes, dicts, nodes / props, mean, sigma, bs.max); - - JS_DumpHistogram(&bs, logfp); - } -#endif - - /* - * Sweep the heap clean of all unmarked nodes. Here we will find nodes - * already GC'ed from the root ply, but we will avoid re-orphaning their - * kids, because the kids member will already be null. - */ - JSArena **ap = &arenaPool.first.next; - while (JSArena *a = *ap) { - Shape *limit = (Shape *) a->avail; - uintN liveCount = 0; - - for (Shape *shape = (Shape *) a->base; shape < limit; shape++) { - /* If the id is null, shape is already on the freelist. */ - if (JSID_IS_VOID(shape->id)) - continue; - - /* - * If the mark bit is set, shape is alive, so clear the mark bit - * and continue the while loop. - * - * Regenerate shape->shape if it hasn't already been refreshed - * during the mark phase, when live scopes' lastProp members are - * followed to update both scope->shape and lastProp->shape. - */ - if (shape->marked()) { - shape->clearMark(); - if (rt->gcRegenShapes) { - if (shape->hasRegenFlag()) - shape->clearRegenFlag(); - else - shape->shape = js_RegenerateShapeForGC(rt); - } - liveCount++; - continue; - } - -#ifdef DEBUG - if ((shape->flags & Shape::SHARED_EMPTY) && - rt->meterEmptyShapes()) { - compartment->emptyShapes.remove((EmptyShape *) shape); - } -#endif - - if (shape->inDictionary()) { - JS_COMPARTMENT_METER(compartment->liveDictModeNodes--); - } else { - /* - * Here, shape is garbage to collect, but its parent might not - * be, so we may have to remove it from its parent's kids hash - * or kid singleton pointer set. - * - * Without a separate mark-clearing pass, we can't tell whether - * shape->parent is live at this point, so we must remove shape - * if its parent member is non-null. A saving grace: if shape's - * parent is dead and swept by this point, shape->parent will - * be null -- in the next paragraph, we null all of a property - * tree node's kids' parent links when sweeping that node. - */ - if (shape->parent) - removeChild(shape); - - if (!shape->kids.isNull()) - orphanChildren(shape); - } - - /* - * Note that Shape::insertFree nulls shape->id so we know that - * shape is on the freelist. - */ - shape->freeTable(cx); - shape->insertFree(&freeList); - JS_COMPARTMENT_METER(compartment->livePropTreeNodes--); - } - - /* If a contains no live properties, return it to the malloc heap. */ - if (liveCount == 0) { - for (Shape *shape = (Shape *) a->base; shape < limit; shape++) - shape->removeFree(); - JS_ARENA_DESTROY(&arenaPool, a, ap); - } else { -#ifdef DEBUG - livePropCapacity += limit - (Shape *) a->base; - totalLiveCount += liveCount; -#endif - ap = &a->next; - } + HS &h = compartment->emptyShapes; + empties = h.count(); + MeterKidCount(&bs, empties); + for (HS::Range r = h.all(); !r.empty(); r.popFront()) + meter(&bs, r.front()); } -#ifdef DEBUG - if (logfp) { + double props = rt->liveObjectPropsPreSweep; + double nodes = compartment->livePropTreeNodes; + double dicts = compartment->liveDictModeNodes; + + /* Empty scope nodes are never hashed, so subtract them from nodes. */ + JS_ASSERT(nodes - dicts == bs.sum); + nodes -= empties; + + double sigma; + double mean = JS_MeanAndStdDevBS(&bs, &sigma); + + fprintf(logfp, + "props %g nodes %g (dicts %g) beta %g meankids %g sigma %g max %u\n", + props, nodes, dicts, nodes / props, mean, sigma, bs.max); + + JS_DumpHistogram(&bs, logfp); + + /* This data is global, so only print it once per GC. */ + if (compartment == rt->atomsCompartment) { fprintf(logfp, "\nProperty tree stats for gcNumber %lu\n", (unsigned long) rt->gcNumber); - fprintf(logfp, "arenautil %g%%\n", - (totalLiveCount && livePropCapacity) - ? (totalLiveCount * 100.0) / livePropCapacity - : 0.0); - #define RATE(f1, f2) (((double)js_scope_stats.f1 / js_scope_stats.f2) * 100.0) - /* This data is global, so only print it once per GC. */ - if (compartment == rt->atomsCompartment) { - fprintf(logfp, - "Scope search stats:\n" - " searches: %6u\n" - " hits: %6u %5.2f%% of searches\n" - " misses: %6u %5.2f%%\n" - " hashes: %6u %5.2f%%\n" - " hashHits: %6u %5.2f%% (%5.2f%% of hashes)\n" - " hashMisses: %6u %5.2f%% (%5.2f%%)\n" - " steps: %6u %5.2f%% (%5.2f%%)\n" - " stepHits: %6u %5.2f%% (%5.2f%%)\n" - " stepMisses: %6u %5.2f%% (%5.2f%%)\n" - " initSearches: %6u\n" - " changeSearches: %6u\n" - " tableAllocFails: %6u\n" - " toDictFails: %6u\n" - " wrapWatchFails: %6u\n" - " adds: %6u\n" - " addFails: %6u\n" - " puts: %6u\n" - " redundantPuts: %6u\n" - " putFails: %6u\n" - " changes: %6u\n" - " changeFails: %6u\n" - " compresses: %6u\n" - " grows: %6u\n" - " removes: %6u\n" - " removeFrees: %6u\n" - " uselessRemoves: %6u\n" - " shrinks: %6u\n", - js_scope_stats.searches, - js_scope_stats.hits, RATE(hits, searches), - js_scope_stats.misses, RATE(misses, searches), - js_scope_stats.hashes, RATE(hashes, searches), - js_scope_stats.hashHits, RATE(hashHits, searches), RATE(hashHits, hashes), - js_scope_stats.hashMisses, RATE(hashMisses, searches), RATE(hashMisses, hashes), - js_scope_stats.steps, RATE(steps, searches), RATE(steps, hashes), - js_scope_stats.stepHits, RATE(stepHits, searches), RATE(stepHits, hashes), - js_scope_stats.stepMisses, RATE(stepMisses, searches), RATE(stepMisses, hashes), - js_scope_stats.initSearches, - js_scope_stats.changeSearches, - js_scope_stats.tableAllocFails, - js_scope_stats.toDictFails, - js_scope_stats.wrapWatchFails, - js_scope_stats.adds, - js_scope_stats.addFails, - js_scope_stats.puts, - js_scope_stats.redundantPuts, - js_scope_stats.putFails, - js_scope_stats.changes, - js_scope_stats.changeFails, - js_scope_stats.compresses, - js_scope_stats.grows, - js_scope_stats.removes, - js_scope_stats.removeFrees, - js_scope_stats.uselessRemoves, - js_scope_stats.shrinks); - } + fprintf(logfp, + "Scope search stats:\n" + " searches: %6u\n" + " hits: %6u %5.2f%% of searches\n" + " misses: %6u %5.2f%%\n" + " hashes: %6u %5.2f%%\n" + " hashHits: %6u %5.2f%% (%5.2f%% of hashes)\n" + " hashMisses: %6u %5.2f%% (%5.2f%%)\n" + " steps: %6u %5.2f%% (%5.2f%%)\n" + " stepHits: %6u %5.2f%% (%5.2f%%)\n" + " stepMisses: %6u %5.2f%% (%5.2f%%)\n" + " initSearches: %6u\n" + " changeSearches: %6u\n" + " tableAllocFails: %6u\n" + " toDictFails: %6u\n" + " wrapWatchFails: %6u\n" + " adds: %6u\n" + " addFails: %6u\n" + " puts: %6u\n" + " redundantPuts: %6u\n" + " putFails: %6u\n" + " changes: %6u\n" + " changeFails: %6u\n" + " compresses: %6u\n" + " grows: %6u\n" + " removes: %6u\n" + " removeFrees: %6u\n" + " uselessRemoves: %6u\n" + " shrinks: %6u\n", + js_scope_stats.searches, + js_scope_stats.hits, RATE(hits, searches), + js_scope_stats.misses, RATE(misses, searches), + js_scope_stats.hashes, RATE(hashes, searches), + js_scope_stats.hashHits, RATE(hashHits, searches), RATE(hashHits, hashes), + js_scope_stats.hashMisses, RATE(hashMisses, searches), RATE(hashMisses, hashes), + js_scope_stats.steps, RATE(steps, searches), RATE(steps, hashes), + js_scope_stats.stepHits, RATE(stepHits, searches), RATE(stepHits, hashes), + js_scope_stats.stepMisses, RATE(stepMisses, searches), RATE(stepMisses, hashes), + js_scope_stats.initSearches, + js_scope_stats.changeSearches, + js_scope_stats.tableAllocFails, + js_scope_stats.toDictFails, + js_scope_stats.wrapWatchFails, + js_scope_stats.adds, + js_scope_stats.addFails, + js_scope_stats.puts, + js_scope_stats.redundantPuts, + js_scope_stats.putFails, + js_scope_stats.changes, + js_scope_stats.changeFails, + js_scope_stats.compresses, + js_scope_stats.grows, + js_scope_stats.removes, + js_scope_stats.removeFrees, + js_scope_stats.uselessRemoves, + js_scope_stats.shrinks); + } #undef RATE - fflush(logfp); - } + fflush(logfp); #endif /* DEBUG */ } -void -js::PropertyTree::unmarkShapes(JSContext *cx) -{ - JSArena **ap = &arenaPool.first.next; - while (JSArena *a = *ap) { - Shape *limit = (Shape *) a->avail; - - for (Shape *shape = (Shape *) a->base; shape < limit; shape++) { - /* If the id is null, shape is already on the freelist. */ - if (JSID_IS_VOID(shape->id)) - continue; - - shape->clearMark(); - } - ap = &a->next; - } -} - void js::PropertyTree::dumpShapes(JSContext *cx) { diff --git a/js/src/jspropertytree.h b/js/src/jspropertytree.h index c0f4abd8fc93..89e91645cac1 100644 --- a/js/src/jspropertytree.h +++ b/js/src/jspropertytree.h @@ -40,7 +40,6 @@ #ifndef jspropertytree_h___ #define jspropertytree_h___ -#include "jsarena.h" #include "jshashtable.h" #include "jsprvtd.h" @@ -102,11 +101,8 @@ class PropertyTree friend struct ::JSFunction; JSCompartment *compartment; - JSArenaPool arenaPool; - js::Shape *freeList; bool insertChild(JSContext *cx, js::Shape *parent, js::Shape *child); - void removeChild(js::Shape *child); PropertyTree(); @@ -114,21 +110,14 @@ class PropertyTree enum { MAX_HEIGHT = 128 }; PropertyTree(JSCompartment *comp) - : compartment(comp), freeList(NULL) + : compartment(comp) { - PodZero(&arenaPool); } - bool init(); - void finish(); - js::Shape *newShape(JSContext *cx); js::Shape *getChild(JSContext *cx, js::Shape *parent, const js::Shape &child); - void orphanChildren(js::Shape *shape); - void sweepShapes(JSContext *cx); - void unmarkShapes(JSContext *cx); - + void dumpShapeStats(); static void dumpShapes(JSContext *cx); #ifdef DEBUG static void meter(JSBasicStats *bs, js::Shape *node); diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 7937792d3888..19eedad755b7 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -571,6 +571,9 @@ bool JSObject::toDictionaryMode(JSContext *cx) { JS_ASSERT(!inDictionaryMode()); + + /* We allocate the shapes from cx->compartment, so make sure it's right. */ + JS_ASSERT(compartment() == cx->compartment); if (!Shape::newDictionaryList(cx, &lastProp)) return false; @@ -1429,12 +1432,28 @@ PrintPropertyMethod(JSTracer *trc, char *buf, size_t bufsize) } #endif -void -Shape::trace(JSTracer *trc) const -{ - if (IS_GC_MARKING_TRACER(trc)) - mark(); +/* + * A few notes on shape marking: + * + * We want to make sure that we regenerate the shape number exactly once per + * shape-regenerating GC. Since delayed marking calls MarkChildren many times, + * we handle regeneration in the PreMark stage. + * + * We also want to make sure to mark iteratively up the parent chain, not + * recursively. So marking is split into markChildren and markChildrenNotParent. + */ +void +Shape::regenerate(JSTracer *trc) const +{ + JSRuntime *rt = trc->context->runtime; + if (IS_GC_MARKING_TRACER(trc) && rt->gcRegenShapes) + shape = js_RegenerateShapeForGC(rt); +} + +void +Shape::markChildrenNotParent(JSTracer *trc) const +{ MarkId(trc, id, "id"); if (attrs & (JSPROP_GETTER | JSPROP_SETTER)) { @@ -1453,3 +1472,20 @@ Shape::trace(JSTracer *trc) const Mark(trc, &methodObject()); } } + +void +Shape::markChildren(JSTracer *trc) const +{ + markChildrenNotParent(trc); + + for (Shape *shape = parent; shape; shape = shape->parent) { + if (IS_GC_MARKING_TRACER(trc)) { + GCMarker *gcmarker = static_cast(trc); + if (!shape->markIfUnmarked(gcmarker->getMarkColor())) + break; + } + + shape->regenerate(trc); + shape->markChildrenNotParent(trc); + } +} diff --git a/js/src/jsscope.h b/js/src/jsscope.h index bf372e81da98..252a0d029125 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -316,10 +316,6 @@ struct Shape : public JSObjectMap jsid id; -#ifdef DEBUG - JSCompartment *compartment; -#endif - protected: union { js::PropertyOp rawGetter; /* getter and setter hooks or objects */ @@ -432,25 +428,6 @@ struct Shape : public JSObjectMap parent = p; } - void insertFree(js::Shape **freep) { -#ifdef DEBUG - memset(this, JS_FREE_PATTERN, sizeof *this); -#endif - id = JSID_VOID; - parent = *freep; - if (parent) - parent->listp = &parent; - listp = freep; - *freep = this; - } - - void removeFree() { - JS_ASSERT(JSID_IS_VOID(id)); - *listp = parent; - if (parent) - parent->listp = listp; - } - public: const js::Shape *previous() const { return parent; @@ -493,22 +470,13 @@ struct Shape : public JSObjectMap * with these bits. */ enum { - /* GC mark flag. */ - MARK = 0x01, - - SHARED_EMPTY = 0x02, - - /* - * Set during a shape-regenerating GC if the shape has already been - * regenerated. - */ - SHAPE_REGEN = 0x04, + SHARED_EMPTY = 0x01, /* Property stored in per-object dictionary, not shared property tree. */ - IN_DICTIONARY = 0x08, + IN_DICTIONARY = 0x02, /* Prevent unwanted mutation of shared Bindings::lastBinding nodes. */ - FROZEN = 0x10 + FROZEN = 0x04 }; Shape(jsid id, js::PropertyOp getter, js::StrictPropertyOp setter, uint32 slot, uintN attrs, @@ -517,17 +485,6 @@ struct Shape : public JSObjectMap /* Used by EmptyShape (see jsscopeinlines.h). */ Shape(JSCompartment *comp, Class *aclasp); - public: - bool marked() const { return (flags & MARK) != 0; } - - protected: - void mark() const { flags |= MARK; } - void clearMark() { flags &= ~MARK; } - - bool hasRegenFlag() const { return (flags & SHAPE_REGEN) != 0; } - void setRegenFlag() { flags |= SHAPE_REGEN; } - void clearRegenFlag() { flags &= ~SHAPE_REGEN; } - bool inDictionary() const { return (flags & IN_DICTIONARY) != 0; } bool frozen() const { return (flags & FROZEN) != 0; } void setFrozen() { flags |= FROZEN; } @@ -591,7 +548,9 @@ struct Shape : public JSObjectMap inline bool isSharedPermanent() const; - void trace(JSTracer *trc) const; + void regenerate(JSTracer *trc) const; + void markChildrenNotParent(JSTracer *trc) const; + void markChildren(JSTracer *trc) const; bool hasSlot() const { return (attrs & JSPROP_SHARED) == 0; } @@ -642,6 +601,9 @@ struct Shape : public JSObjectMap void dump(JSContext *cx, FILE *fp) const; void dumpSubtree(JSContext *cx, int level, FILE *fp) const; #endif + + void finalize(JSContext *cx); + void removeChild(js::Shape *child); }; struct EmptyShape : public js::Shape @@ -779,7 +741,7 @@ JSObject::setLastProperty(const js::Shape *shape) JS_ASSERT(!inDictionaryMode()); JS_ASSERT(!JSID_IS_VOID(shape->id)); JS_ASSERT_IF(lastProp, !JSID_IS_VOID(lastProp->id)); - JS_ASSERT(shape->compartment == compartment()); + JS_ASSERT(shape->compartment() == compartment()); lastProp = const_cast(shape); } @@ -830,6 +792,7 @@ Shape::insertIntoDictionary(js::Shape **dictp) JS_ASSERT_IF(*dictp, (*dictp)->inDictionary()); JS_ASSERT_IF(*dictp, (*dictp)->listp == dictp); JS_ASSERT_IF(*dictp, !JSID_IS_VOID((*dictp)->id)); + JS_ASSERT_IF(*dictp, compartment() == (*dictp)->compartment()); setParent(*dictp); if (parent) diff --git a/js/src/jsscopeinlines.h b/js/src/jsscopeinlines.h index 4d7ac0adfc62..f7b9e0799896 100644 --- a/js/src/jsscopeinlines.h +++ b/js/src/jsscopeinlines.h @@ -47,7 +47,9 @@ #include "jsfun.h" #include "jsobj.h" #include "jsscope.h" +#include "jsgc.h" +#include "jsgcinlines.h" #include "jscntxtinlines.h" #include "jsobjinlines.h" diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 7df8ebbfc98e..29d013dc277b 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -290,8 +290,8 @@ Bindings::makeImmutable() void Bindings::trace(JSTracer *trc) { - for (const Shape *shape = lastBinding; shape; shape = shape->previous()) - shape->trace(trc); + if (lastBinding) + MarkShape(trc, lastBinding, "shape"); } } /* namespace js */ diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 4f80c927faf4..09bc660b9ee1 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2829,16 +2829,6 @@ TraceMonitor::flush() needFlush = JS_FALSE; } -inline bool -IsShapeAboutToBeFinalized(JSContext *cx, const js::Shape *shape) -{ - JSRuntime *rt = cx->runtime; - if (rt->gcCurrentCompartment != NULL) - return false; - - return !shape->marked(); -} - inline bool HasUnreachableGCThings(JSContext *cx, TreeFragment *f) { @@ -2859,7 +2849,7 @@ HasUnreachableGCThings(JSContext *cx, TreeFragment *f) const Shape** shapep = f->shapes.data(); for (unsigned len = f->shapes.length(); len; --len) { const Shape* shape = *shapep++; - if (IsShapeAboutToBeFinalized(cx, shape)) + if (IsAboutToBeFinalized(cx, shape)) return true; } return false;