Bug 569422 - Allocate js::Shape from the GC heap (r=brendan)

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, "<shape>");
+            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;

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

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

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()) {

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<JSExternalString>) == 4096);
 JS_STATIC_ASSERT(sizeof(Arena<JSShortString>)    == 4096);
 JS_STATIC_ASSERT(sizeof(Arena<JSObject>)         == 4096);
 JS_STATIC_ASSERT(sizeof(Arena<JSFunction>)       == 4096);
+JS_STATIC_ASSERT(sizeof(Arena<Shape>)            == 4096);
 JS_STATIC_ASSERT(sizeof(Arena<JSXML>)            == 4096);
 
 #ifdef JS_GCMETER
@@ -241,6 +244,7 @@ checkArenaListsForThing(JSCompartment *comp, void *thing)
         comp->arenas[FINALIZE_OBJECT12].arenasContainThing<JSObject_Slots12>(thing) ||
         comp->arenas[FINALIZE_OBJECT16].arenasContainThing<JSObject_Slots16>(thing) ||
         comp->arenas[FINALIZE_FUNCTION].arenasContainThing<JSFunction>(thing) ||
+        comp->arenas[FINALIZE_FUNCTION].arenasContainThing<Shape>(thing) ||
 #if JS_HAS_XML_SUPPORT
         comp->arenas[FINALIZE_XML].arenasContainThing<JSXML>(thing) ||
 #endif
@@ -677,6 +681,9 @@ MarkIfGCThingWord(JSTracer *trc, jsuword w, uint32 &thingKind)
       case FINALIZE_FUNCTION:
         test = MarkCell<JSFunction>(cell, trc);
         break;
+      case FINALIZE_SHAPE:
+        test = MarkCell<Shape>(cell, trc);
+        break;
 #if JS_HAS_XML_SUPPORT
       case FINALIZE_XML:
         test = MarkCell<JSXML>(cell, trc);
@@ -1198,6 +1205,8 @@ RefillFinalizableFreeList(JSContext *cx, unsigned thingKind)
         return RefillTypedFreeList<JSShortString>(cx, thingKind);
       case FINALIZE_FUNCTION:
         return RefillTypedFreeList<JSFunction>(cx, thingKind);
+      case FINALIZE_SHAPE:
+        return RefillTypedFreeList<Shape>(cx, thingKind);
 #if JS_HAS_XML_SUPPORT
       case FINALIZE_XML:
         return RefillTypedFreeList<JSXML>(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<Arena<JSFunction> *>(a)->markDelayedChildren(this);
             break;
+          case FINALIZE_SHAPE:
+            reinterpret_cast<Arena<Shape> *>(a)->markDelayedChildren(this);
+            break;
 #if JS_HAS_XML_SUPPORT
           case FINALIZE_XML:
             reinterpret_cast<Arena<JSXML> *>(a)->markDelayedChildren(this);
@@ -1512,7 +1529,7 @@ AutoGCRooter::trace(JSTracer *trc)
         return;
 
       case SHAPE:
-        static_cast<AutoShapeRooter *>(this)->shape->trace(trc);
+        MarkShape(trc, static_cast<AutoShapeRooter *>(this)->shape, "js::AutoShapeRooter.val");
         return;
 
       case PARSER:
@@ -1934,6 +1951,7 @@ JSCompartment::finalizeObjectArenaLists(JSContext *cx)
     FinalizeArenaList<JSObject_Slots12>(this, cx, FINALIZE_OBJECT12);
     FinalizeArenaList<JSObject_Slots16>(this, cx, FINALIZE_OBJECT16);
     FinalizeArenaList<JSFunction>(this, cx, FINALIZE_FUNCTION);
+    FinalizeArenaList<Shape>(this, cx, FINALIZE_SHAPE);
 #if JS_HAS_XML_SUPPORT
     FinalizeArenaList<JSXML>(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);

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<Arena<T> *>(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
 
 /*

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<js::Shape>(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<Shape>((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<GCMarker *>(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<JSString *>(thing));
             break;
+        case JSTRACE_SHAPE:
+            Mark(trc, reinterpret_cast<Shape *>(thing));
+            break;
 #if JS_HAS_XML_SUPPORT
         case JSTRACE_XML:
             Mark(trc, reinterpret_cast<JSXML *>(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);
     }
 }
 

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()) {

js/src/jsobj.h

@@ -210,9 +210,9 @@ MeterEntryCount(uintN count);
 
 } /* namespace js */
 
-struct JSObjectMap {
+struct JSObjectMap : public js::gc::Cell {
-    uint32 shape;       /* shape identifier */
+    mutable uint32 shape;  /* shape identifier */
-    uint32 slotSpan;    /* one more than maximum live slot number */
+    uint32 slotSpan;       /* one more than maximum live slot number */
 
     static JS_FRIEND_DATA(JSObjectMap) sharedNonNative;
 

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
+         * MarkShape will regenerate the shape if need be. However, we need to
-         * it must have the same shape as lastProp.
+         * 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

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);

js/src/jspropertytree.cpp

@@ -40,7 +40,6 @@
 #include <new>
 
 #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 *shape = js_NewGCShape(cx);
-
+    if (!shape) {
-    shape = freeList;
+        JS_ReportOutOfMemory(cx);
-    if (shape) {
+        return NULL;
-        shape->removeFree();
-    } else {
-        JS_ARENA_ALLOCATE_CAST(shape, Shape *, &arenaPool, sizeof(Shape));
-        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;
+    KidsPointer *kidp = &kids;
-    JS_ASSERT(parent);
-    JS_ASSERT(!JSID_IS_VOID(parent->id));
-
-    KidsPointer *kidp = &parent->kids;
     if (kidp->isShape()) {
-        Shape *kid = kidp->toShape();
+        JS_ASSERT(kidp->toShape() == child);
-        if (kid == child)
+        kids.setNull();
-            parent->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
+void
-js::PropertyTree::orphanChildren(Shape *shape)
+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 (inDictionary()) {
-
+        JS_COMPARTMENT_METER(compartment()->liveDictModeNodes--);
-    if (kidp->isShape()) {
-        Shape *kid = kidp->toShape();
-
-        if (!JSID_IS_VOID(kid->id)) {
-            JS_ASSERT(kid->parent == shape);
-            kid->parent = NULL;
-        }
     } else {
-        KidsHash *hash = kidp->toHash();
+        if (parent && parent->isMarked())
+            parent->removeChild(this);
 
-        for (KidsHash::Range range = hash->all(); !range.empty(); range.popFront()) {
+        if (kids.isHash())
-            Shape *kid = range.front();
+            js_delete(kids.toHash());
-            if (!JSID_IS_VOID(kid->id)) {
-                JS_ASSERT(kid->parent == shape);
-                kid->parent = NULL;
-            }
-        }
-
-        hash->~KidsHash();
-        js_free(hash);
     }
 
-    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;
+    uint32 empties;
-        {
+    {
-            typedef JSCompartment::EmptyShapeSet HS;
+        typedef JSCompartment::EmptyShapeSet HS;
 
-            HS &h = compartment->emptyShapes;
+        HS &h = compartment->emptyShapes;
-            empties = h.count();
+        empties = h.count();
-            MeterKidCount(&bs, empties);
+        MeterKidCount(&bs, empties);
-            for (HS::Range r = h.all(); !r.empty(); r.popFront())
+        for (HS::Range r = h.all(); !r.empty(); r.popFront())
-                meter(&bs, r.front());
+            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;
-        }
     }
 
-#ifdef DEBUG
+    double props = rt->liveObjectPropsPreSweep;
-    if (logfp) {
+    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. */
+        fprintf(logfp,
-        if (compartment == rt->atomsCompartment) {
+                "Scope search stats:\n"
-            fprintf(logfp,
+                "  searches:        %6u\n"
-                    "Scope search stats:\n"
+                "  hits:            %6u %5.2f%% of searches\n"
-                    "  searches:        %6u\n"
+                "  misses:          %6u %5.2f%%\n"
-                    "  hits:            %6u %5.2f%% of searches\n"
+                "  hashes:          %6u %5.2f%%\n"
-                    "  misses:          %6u %5.2f%%\n"
+                "  hashHits:        %6u %5.2f%% (%5.2f%% of hashes)\n"
-                    "  hashes:          %6u %5.2f%%\n"
+                "  hashMisses:      %6u %5.2f%% (%5.2f%%)\n"
-                    "  hashHits:        %6u %5.2f%% (%5.2f%% of hashes)\n"
+                "  steps:           %6u %5.2f%% (%5.2f%%)\n"
-                    "  hashMisses:      %6u %5.2f%% (%5.2f%%)\n"
+                "  stepHits:        %6u %5.2f%% (%5.2f%%)\n"
-                    "  steps:           %6u %5.2f%% (%5.2f%%)\n"
+                "  stepMisses:      %6u %5.2f%% (%5.2f%%)\n"
-                    "  stepHits:        %6u %5.2f%% (%5.2f%%)\n"
+                "  initSearches:    %6u\n"
-                    "  stepMisses:      %6u %5.2f%% (%5.2f%%)\n"
+                "  changeSearches:  %6u\n"
-                    "  initSearches:    %6u\n"
+                "  tableAllocFails: %6u\n"
-                    "  changeSearches:  %6u\n"
+                "  toDictFails:     %6u\n"
-                    "  tableAllocFails: %6u\n"
+                "  wrapWatchFails:  %6u\n"
-                    "  toDictFails:     %6u\n"
+                "  adds:            %6u\n"
-                    "  wrapWatchFails:  %6u\n"
+                "  addFails:        %6u\n"
-                    "  adds:            %6u\n"
+                "  puts:            %6u\n"
-                    "  addFails:        %6u\n"
+                "  redundantPuts:   %6u\n"
-                    "  puts:            %6u\n"
+                "  putFails:        %6u\n"
-                    "  redundantPuts:   %6u\n"
+                "  changes:         %6u\n"
-                    "  putFails:        %6u\n"
+                "  changeFails:     %6u\n"
-                    "  changes:         %6u\n"
+                "  compresses:      %6u\n"
-                    "  changeFails:     %6u\n"
+                "  grows:           %6u\n"
-                    "  compresses:      %6u\n"
+                "  removes:         %6u\n"
-                    "  grows:           %6u\n"
+                "  removeFrees:     %6u\n"
-                    "  removes:         %6u\n"
+                "  uselessRemoves:  %6u\n"
-                    "  removeFrees:     %6u\n"
+                "  shrinks:         %6u\n",
-                    "  uselessRemoves:  %6u\n"
+                js_scope_stats.searches,
-                    "  shrinks:         %6u\n",
+                js_scope_stats.hits, RATE(hits, searches),
-                    js_scope_stats.searches,
+                js_scope_stats.misses, RATE(misses, searches),
-                    js_scope_stats.hits, RATE(hits, searches),
+                js_scope_stats.hashes, RATE(hashes, searches),
-                    js_scope_stats.misses, RATE(misses, searches),
+                js_scope_stats.hashHits, RATE(hashHits, searches), RATE(hashHits, hashes),
-                    js_scope_stats.hashes, RATE(hashes, searches),
+                js_scope_stats.hashMisses, RATE(hashMisses, searches), RATE(hashMisses, hashes),
-                    js_scope_stats.hashHits, RATE(hashHits, searches), RATE(hashHits, hashes),
+                js_scope_stats.steps, RATE(steps, searches), RATE(steps, hashes),
-                    js_scope_stats.hashMisses, RATE(hashMisses, searches), RATE(hashMisses, hashes),
+                js_scope_stats.stepHits, RATE(stepHits, searches), RATE(stepHits, hashes),
-                    js_scope_stats.steps, RATE(steps, searches), RATE(steps, hashes),
+                js_scope_stats.stepMisses, RATE(stepMisses, searches), RATE(stepMisses, hashes),
-                    js_scope_stats.stepHits, RATE(stepHits, searches), RATE(stepHits, hashes),
+                js_scope_stats.initSearches,
-                    js_scope_stats.stepMisses, RATE(stepMisses, searches), RATE(stepMisses, hashes),
+                js_scope_stats.changeSearches,
-                    js_scope_stats.initSearches,
+                js_scope_stats.tableAllocFails,
-                    js_scope_stats.changeSearches,
+                js_scope_stats.toDictFails,
-                    js_scope_stats.tableAllocFails,
+                js_scope_stats.wrapWatchFails,
-                    js_scope_stats.toDictFails,
+                js_scope_stats.adds,
-                    js_scope_stats.wrapWatchFails,
+                js_scope_stats.addFails,
-                    js_scope_stats.adds,
+                js_scope_stats.puts,
-                    js_scope_stats.addFails,
+                js_scope_stats.redundantPuts,
-                    js_scope_stats.puts,
+                js_scope_stats.putFails,
-                    js_scope_stats.redundantPuts,
+                js_scope_stats.changes,
-                    js_scope_stats.putFails,
+                js_scope_stats.changeFails,
-                    js_scope_stats.changes,
+                js_scope_stats.compresses,
-                    js_scope_stats.changeFails,
+                js_scope_stats.grows,
-                    js_scope_stats.compresses,
+                js_scope_stats.removes,
-                    js_scope_stats.grows,
+                js_scope_stats.removeFrees,
-                    js_scope_stats.removes,
+                js_scope_stats.uselessRemoves,
-                    js_scope_stats.removeFrees,
+                js_scope_stats.shrinks);
-                    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)
 {

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 dumpShapeStats();
-    void sweepShapes(JSContext *cx);
-    void unmarkShapes(JSContext *cx);
-
     static void dumpShapes(JSContext *cx);
 #ifdef DEBUG
     static void meter(JSBasicStats *bs, js::Shape *node);

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
+ * A few notes on shape marking:
-{
+ *
-    if (IS_GC_MARKING_TRACER(trc))
+ * We want to make sure that we regenerate the shape number exactly once per
-        mark();
+ * 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<GCMarker *>(trc);
+            if (!shape->markIfUnmarked(gcmarker->getMarkColor()))
+                break;
+        }
+
+        shape->regenerate(trc);
+        shape->markChildrenNotParent(trc);
+    }
+}

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. */
+        SHARED_EMPTY    = 0x01,
-        MARK            = 0x01,
-
-        SHARED_EMPTY    = 0x02,
-
-        /*
-         * Set during a shape-regenerating GC if the shape has already been
-         * regenerated.
-         */
-        SHAPE_REGEN     = 0x04,
 
         /* 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<js::Shape *>(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)

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"
 

js/src/jsscript.cpp

@@ -290,8 +290,8 @@ Bindings::makeImmutable()
 void
 Bindings::trace(JSTracer *trc)
 {
-    for (const Shape *shape = lastBinding; shape; shape = shape->previous())
+    if (lastBinding)
-        shape->trace(trc);
+        MarkShape(trc, lastBinding, "shape");
 }
 
 } /* namespace js */

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;