Bug 1127246 - Add a post barrier to the baseShapes table r=terrence a=sylvestre
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 02 Feb 2015 10:11:23 +0000
changeset 243661 e0a36c5bdf4c
parent 243656 f74e583e724f
child 243662 f8616422302f
push id4427
push userjcoppeard@mozilla.com
push date2015-02-03 10:13 +0000
treeherdermozilla-beta@e0a36c5bdf4c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, sylvestre
bugs1127246
milestone36.0
Bug 1127246 - Add a post barrier to the baseShapes table r=terrence a=sylvestre
js/public/HashTable.h
js/src/jscompartment.h
js/src/jsgc.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/public/HashTable.h
+++ b/js/public/HashTable.h
@@ -249,20 +249,23 @@ class HashMap
 
     // Infallibly rekey one entry, if necessary.
     // Requires template parameters Key and HashPolicy::Lookup to be the same type.
     void rekeyIfMoved(const Key &old_key, const Key &new_key) {
         if (old_key != new_key)
             rekeyAs(old_key, new_key, new_key);
     }
 
-    // Infallibly rekey one entry, if present.
-    void rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const Key &new_key) {
-        if (Ptr p = lookup(old_lookup))
+    // Infallibly rekey one entry if present, and return whether that happened.
+    bool rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const Key &new_key) {
+        if (Ptr p = lookup(old_lookup)) {
             impl.rekeyAndMaybeRehash(p, new_lookup, new_key);
+            return true;
+        }
+        return false;
     }
 
     // HashMap is movable
     HashMap(HashMap &&rhs) : impl(mozilla::Move(rhs.impl)) {}
     void operator=(HashMap &&rhs) {
         MOZ_ASSERT(this != &rhs, "self-move assignment is prohibited");
         impl = mozilla::Move(rhs.impl);
     }
@@ -467,20 +470,23 @@ class HashSet
 
     // Infallibly rekey one entry, if present.
     // Requires template parameters T and HashPolicy::Lookup to be the same type.
     void rekeyIfMoved(const Lookup &old_value, const T &new_value) {
         if (old_value != new_value)
             rekeyAs(old_value, new_value, new_value);
     }
 
-    // Infallibly rekey one entry, if present.
-    void rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const T &new_value) {
-        if (Ptr p = lookup(old_lookup))
+    // Infallibly rekey one entry if present, and return whether that happened.
+    bool rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const T &new_value) {
+        if (Ptr p = lookup(old_lookup)) {
             impl.rekeyAndMaybeRehash(p, new_lookup, new_value);
+            return true;
+        }
+        return false;
     }
 
     // Infallibly rekey one entry with a new key that is equivalent.
     void rekeyInPlace(Ptr p, const T &new_value)
     {
         MOZ_ASSERT(HashPolicy::match(*p, new_value));
         impl.rekeyInPlace(p, new_value);
     }
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -275,16 +275,17 @@ struct JSCompartment
     js::types::TypeObjectWithNewScriptSet lazyTypeObjects;
     void sweepNewTypeObjectTable(js::types::TypeObjectWithNewScriptSet &table);
 
 #ifdef JSGC_HASH_TABLE_CHECKS
     void checkTypeObjectTablesAfterMovingGC();
     void checkTypeObjectTableAfterMovingGC(js::types::TypeObjectWithNewScriptSet &table);
     void checkInitialShapesTableAfterMovingGC();
     void checkWrapperMapAfterMovingGC();
+    void checkBaseShapeTableAfterMovingGC();
 #endif
 
     /*
      * Hash table of all manually call site-cloned functions from within
      * self-hosted code. Cloning according to call site provides extra
      * sensitivity for type specialization and inlining.
      */
     js::CallsiteCloneTable callsiteClones;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -7036,13 +7036,14 @@ js::gc::CheckHashTablesAfterMovingGC(JSR
     /*
      * Check that internal hash tables no longer have any pointers to things
      * that have been moved.
      */
     for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
         c->checkTypeObjectTablesAfterMovingGC();
         c->checkInitialShapesTableAfterMovingGC();
         c->checkWrapperMapAfterMovingGC();
+        c->checkBaseShapeTableAfterMovingGC();
         if (c->debugScopes)
             c->debugScopes->checkHashTablesAfterMovingGC(rt);
     }
 }
 #endif
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1441,81 +1441,138 @@ Shape::setObjectFlag(ExclusiveContext *c
     StackBaseShape base(last);
     base.flags |= flag;
 
     RootedShape lastRoot(cx, last);
     return replaceLastProperty(cx, base, proto, lastRoot);
 }
 
 /* static */ inline HashNumber
-StackBaseShape::hash(const StackBaseShape *base)
+StackBaseShape::hash(const Lookup& lookup)
 {
-    HashNumber hash = base->flags;
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->clasp) >> 3);
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->parent) >> 3);
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->metadata) >> 3);
+    HashNumber hash = lookup.flags;
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.clasp) >> 3);
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.hashParent) >> 3);
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.hashMetadata) >> 3);
     return hash;
 }
 
 /* static */ inline bool
-StackBaseShape::match(UnownedBaseShape *key, const StackBaseShape *lookup)
+StackBaseShape::match(UnownedBaseShape *key, const Lookup& lookup)
 {
-    return key->flags == lookup->flags
-        && key->clasp_ == lookup->clasp
-        && key->parent == lookup->parent
-        && key->metadata == lookup->metadata;
+    return key->flags == lookup.flags
+        && key->clasp_ == lookup.clasp
+        && key->parent == lookup.matchParent
+        && key->metadata == lookup.matchMetadata;
 }
 
 void
 StackBaseShape::trace(JSTracer *trc)
 {
     if (parent)
         gc::MarkObjectRoot(trc, (JSObject**)&parent, "StackBaseShape parent");
 
     if (metadata)
         gc::MarkObjectRoot(trc, (JSObject**)&metadata, "StackBaseShape metadata");
 }
 
+/*
+ * This class is used to add a post barrier on the baseShapes set, as the key is
+ * calculated based on objects which may be moved by generational GC.
+ */
+class BaseShapeSetRef : public BufferableRef
+{
+    BaseShapeSet *set;
+    UnownedBaseShape *base;
+    JSObject *parentPrior;
+    JSObject *metadataPrior;
+
+  public:
+    BaseShapeSetRef(BaseShapeSet *set, UnownedBaseShape *base)
+      : set(set),
+        base(base),
+        parentPrior(base->getObjectParent()),
+        metadataPrior(base->getObjectMetadata())
+    {
+        MOZ_ASSERT(!base->isOwned());
+    }
+
+    void mark(JSTracer *trc) {
+        JSObject *parent = parentPrior;
+        if (parent)
+            Mark(trc, &parent, "baseShapes set parent");
+        JSObject *metadata = metadataPrior;
+        if (metadata)
+            Mark(trc, &metadata, "baseShapes set metadata");
+        if (parent == parentPrior && metadata == metadataPrior)
+            return;
+
+        StackBaseShape::Lookup lookupPrior(base->getObjectFlags(),
+                                           base->clasp(),
+                                           parentPrior, parent,
+                                           metadataPrior, metadata);
+        ReadBarriered<UnownedBaseShape *> b(base);
+        MOZ_ALWAYS_TRUE(set->rekeyAs(lookupPrior, base, b));
+    }
+};
+
+static void
+BaseShapesTablePostBarrier(ExclusiveContext *cx, BaseShapeSet *table, UnownedBaseShape *base)
+{
+    if (!cx->isJSContext()) {
+        MOZ_ASSERT(!IsInsideNursery(base->getObjectParent()));
+        MOZ_ASSERT(!IsInsideNursery(base->getObjectMetadata()));
+        return;
+    }
+
+    if (IsInsideNursery(base->getObjectParent()) || IsInsideNursery(base->getObjectMetadata())) {
+        StoreBuffer &sb = cx->asJSContext()->runtime()->gc.storeBuffer;
+        sb.putGeneric(BaseShapeSetRef(table, base));
+    }
+}
+
 /* static */ UnownedBaseShape*
 BaseShape::getUnowned(ExclusiveContext *cx, StackBaseShape &base)
 {
     BaseShapeSet &table = cx->compartment()->baseShapes;
 
     if (!table.initialized() && !table.init())
         return nullptr;
 
-    DependentAddPtr<BaseShapeSet> p(cx, table, &base);
+    DependentAddPtr<BaseShapeSet> p(cx, table, base);
     if (p)
         return *p;
 
     RootedGeneric<StackBaseShape*> root(cx, &base);
 
     BaseShape *nbase_ = js_NewGCBaseShape<CanGC>(cx);
     if (!nbase_)
         return nullptr;
 
-    new (nbase_) BaseShape(*root);
+    new (nbase_) BaseShape(base);
 
     UnownedBaseShape *nbase = static_cast<UnownedBaseShape *>(nbase_);
 
-    if (!p.add(cx, table, root, nbase))
+    if (!p.add(cx, table, base, nbase))
         return nullptr;
 
+    BaseShapesTablePostBarrier(cx, &table, nbase);
+
     return nbase;
 }
 
 /* static */ UnownedBaseShape *
 BaseShape::lookupUnowned(ThreadSafeContext *cx, const StackBaseShape &base)
 {
     BaseShapeSet &table = cx->compartment_->baseShapes;
 
     if (!table.initialized())
         return nullptr;
 
-    BaseShapeSet::Ptr p = table.readonlyThreadsafeLookup(&base);
+    BaseShapeSet::Ptr p = table.readonlyThreadsafeLookup(base);
     return *p;
 }
 
 void
 BaseShape::assertConsistency()
 {
 #ifdef DEBUG
     if (isOwned()) {
@@ -1531,24 +1588,46 @@ void
 JSCompartment::sweepBaseShapeTable()
 {
     if (baseShapes.initialized()) {
         for (BaseShapeSet::Enum e(baseShapes); !e.empty(); e.popFront()) {
             UnownedBaseShape *base = e.front().unbarrieredGet();
             if (IsBaseShapeAboutToBeFinalizedFromAnyThread(&base)) {
                 e.removeFront();
             } else if (base != e.front().unbarrieredGet()) {
-                StackBaseShape sbase(base);
                 ReadBarriered<UnownedBaseShape *> b(base);
-                e.rekeyFront(&sbase, b);
+                e.rekeyFront(base, b);
             }
         }
     }
 }
 
+#ifdef JSGC_HASH_TABLE_CHECKS
+
+void
+JSCompartment::checkBaseShapeTableAfterMovingGC()
+{
+    if (!baseShapes.initialized())
+        return;
+
+    for (BaseShapeSet::Enum e(baseShapes); !e.empty(); e.popFront()) {
+        UnownedBaseShape *base = e.front().unbarrieredGet();
+        CheckGCThingAfterMovingGC(base);
+        if (base->getObjectParent())
+            CheckGCThingAfterMovingGC(base->getObjectParent());
+        if (base->getObjectMetadata())
+            CheckGCThingAfterMovingGC(base->getObjectMetadata());
+
+        BaseShapeSet::Ptr ptr = baseShapes.lookup(base);
+        MOZ_ASSERT(ptr.found() && &*ptr == &e.front());
+    }
+}
+
+#endif // JSGC_HASH_TABLE_CHECKS
+
 void
 BaseShape::finalize(FreeOp *fop)
 {
     if (table_) {
         fop->delete_(table_);
         table_ = nullptr;
     }
 }
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -522,18 +522,16 @@ UnownedBaseShape*
 BaseShape::baseUnowned()
 {
     MOZ_ASSERT(isOwned() && unowned_); return unowned_;
 }
 
 /* Entries for the per-compartment baseShapes set of unowned base shapes. */
 struct StackBaseShape : public DefaultHasher<ReadBarrieredUnownedBaseShape>
 {
-    typedef const StackBaseShape *Lookup;
-
     uint32_t flags;
     const Class *clasp;
     JSObject *parent;
     JSObject *metadata;
     JSCompartment *compartment;
 
     explicit StackBaseShape(BaseShape *base)
       : flags(base->flags & BaseShape::OBJECT_FLAG_MASK),
@@ -542,18 +540,60 @@ struct StackBaseShape : public DefaultHa
         metadata(base->metadata),
         compartment(base->compartment())
     {}
 
     inline StackBaseShape(ThreadSafeContext *cx, const Class *clasp,
                           JSObject *parent, JSObject *metadata, uint32_t objectFlags);
     explicit inline StackBaseShape(Shape *shape);
 
-    static inline HashNumber hash(const StackBaseShape *lookup);
-    static inline bool match(UnownedBaseShape *key, const StackBaseShape *lookup);
+    struct Lookup
+    {
+        uint32_t flags;
+        const Class *clasp;
+        JSObject *hashParent;
+        JSObject *matchParent;
+        JSObject *hashMetadata;
+        JSObject *matchMetadata;
+
+        MOZ_IMPLICIT Lookup(const StackBaseShape &base)
+          : flags(base.flags),
+            clasp(base.clasp),
+            hashParent(base.parent),
+            matchParent(base.parent),
+            hashMetadata(base.metadata),
+            matchMetadata(base.metadata)
+        {}
+
+        MOZ_IMPLICIT Lookup(UnownedBaseShape *base)
+          : flags(base->getObjectFlags()),
+            clasp(base->clasp()),
+            hashParent(base->getObjectParent()),
+            matchParent(base->getObjectParent()),
+            hashMetadata(base->getObjectMetadata()),
+            matchMetadata(base->getObjectMetadata())
+        {
+            MOZ_ASSERT(!base->isOwned());
+        }
+
+        // For use by generational GC post barriers.
+        Lookup(uint32_t flags, const Class *clasp,
+               JSObject *hashParent, JSObject *matchParent,
+               JSObject *hashMetadata, JSObject *matchMetadata)
+          : flags(flags),
+            clasp(clasp),
+            hashParent(hashParent),
+            matchParent(matchParent),
+            hashMetadata(hashMetadata),
+            matchMetadata(matchMetadata)
+        {}
+    };
+
+    static inline HashNumber hash(const Lookup& lookup);
+    static inline bool match(UnownedBaseShape *key, const Lookup& lookup);
 
     // For RootedGeneric<StackBaseShape*>
     void trace(JSTracer *trc);
 };
 
 inline
 BaseShape::BaseShape(const StackBaseShape &base)
 {