Bug 1367795 - Add barriers to JS::WeakCache for GCHashSet r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 10 Jul 2017 18:27:43 +0100
changeset 368035 3290d214503d307a065b84ba63dd5474d92a6c87
parent 368034 e432f2a10f2ac53e2401bb0bcde5c71a65540550
child 368036 03e4fa4f4cde9dc065dad955139293ca33ad4712
push id92388
push userjcoppeard@mozilla.com
push dateMon, 10 Jul 2017 17:40:12 +0000
treeherdermozilla-inbound@8116597d45f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1367795
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1367795 - Add barriers to JS::WeakCache for GCHashSet r=sfink
js/public/GCHashTable.h
js/public/SweepingAPI.h
js/src/vm/ObjectGroup.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/public/GCHashTable.h
+++ b/js/public/GCHashTable.h
@@ -2,16 +2,18 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef GCHashTable_h
 #define GCHashTable_h
 
+#include "mozilla/Maybe.h"
+
 #include "js/GCPolicyAPI.h"
 #include "js/HashTable.h"
 #include "js/RootingAPI.h"
 #include "js/SweepingAPI.h"
 #include "js/TracingAPI.h"
 
 namespace JS {
 
@@ -383,18 +385,23 @@ class WeakCache<GCHashMap<Key, Value, Ha
     explicit WeakCache(JSRuntime* rt, Args&&... args)
       : WeakCacheBase(rt), map(mozilla::Forward<Args>(args)...)
     {}
 
     bool needsSweep() override {
         return map.needsSweep();
     }
 
-    void sweep() override {
-        return map.sweep();
+    size_t sweep() override {
+        if (!this->initialized())
+            return 0;
+
+        size_t steps = map.count();
+        map.sweep();
+        return steps;
     }
 
     using Lookup = typename MainMap::Lookup;
     using AddPtr = typename MainMap::AddPtr;
     using Ptr = typename MainMap::Ptr;
     using Range = typename MainMap::Range;
     struct Enum : public MainMap::Enum { explicit Enum(Self& self) : MainMap::Enum(self.map) {} };
 
@@ -451,67 +458,191 @@ class WeakCache<GCHashMap<Key, Value, Ha
 template <typename T, typename HashPolicy, typename AllocPolicy>
 class WeakCache<GCHashSet<T, HashPolicy, AllocPolicy>>
     : protected detail::WeakCacheBase
 {
     using Set = GCHashSet<T, HashPolicy, AllocPolicy>;
     using Self = WeakCache<Set>;
 
     Set set;
+    bool needsBarrier;
 
   public:
+    using Entry = typename Set::Entry;
+
     template <typename... Args>
     explicit WeakCache(Zone* zone, Args&&... args)
-      : WeakCacheBase(zone), set(mozilla::Forward<Args>(args)...)
+      : WeakCacheBase(zone), set(mozilla::Forward<Args>(args)...), needsBarrier(false)
     {}
     template <typename... Args>
     explicit WeakCache(JSRuntime* rt, Args&&... args)
-      : WeakCacheBase(rt), set(mozilla::Forward<Args>(args)...)
+      : WeakCacheBase(rt), set(mozilla::Forward<Args>(args)...), needsBarrier(false)
     {}
 
-    void sweep() override {
+    size_t sweep() override {
+        if (!this->initialized())
+            return 0;
+
+        size_t steps = set.count();
         set.sweep();
+        return steps;
     }
 
     bool needsSweep() override {
         return set.needsSweep();
     }
 
-    // Const interface.
+    bool setNeedsIncrementalBarrier(bool needs) override {
+        MOZ_ASSERT(needsBarrier != needs);
+        needsBarrier = needs;
+        return true;
+    }
+
+    bool needsIncrementalBarrier() const override {
+        return needsBarrier;
+    }
 
+  private:
+   static bool entryNeedsSweep(const Entry& prior) {
+        Entry entry(prior);
+        bool result = GCPolicy<T>::needsSweep(&entry);
+        MOZ_ASSERT(prior == entry); // We shouldn't update here.
+        return result;
+    }
+
+  public:
     using Lookup = typename Set::Lookup;
+    using Ptr = typename Set::Ptr;
     using AddPtr = typename Set::AddPtr;
-    using Entry = typename Set::Entry;
-    using Ptr = typename Set::Ptr;
-    using Range = typename Set::Range;
+
+    struct Range
+    {
+        explicit Range(const typename Set::Range& r)
+          : range(r)
+        {
+            settle();
+        }
+        Range() {}
+
+        bool empty() const { return range.empty(); }
+        const Entry& front() const { return range.front(); }
+
+        void popFront() {
+            range.popFront();
+            settle();
+        }
+
+      private:
+        typename Set::Range range;
+
+        void settle() {
+            while (!empty() && entryNeedsSweep(front()))
+                popFront();
+        }
+    };
 
-    bool initialized() const                   { return set.initialized(); }
-    Ptr lookup(const Lookup& l) const          { return set.lookup(l); }
-    AddPtr lookupForAdd(const Lookup& l) const { return set.lookupForAdd(l); }
-    Range all() const                          { return set.all(); }
-    bool empty() const                         { return set.empty(); }
-    uint32_t count() const                     { return set.count(); }
-    size_t capacity() const                    { return set.capacity(); }
-    bool has(const Lookup& l) const            { return set.lookup(l).found(); }
+    struct Enum : public Set::Enum
+    {
+        explicit Enum(Self& cache)
+          : Set::Enum(cache.set)
+        {
+            // This operation is not allowed while barriers are in place as we
+            // may also need to enumerate the set for sweeping.
+            MOZ_ASSERT(!cache.needsBarrier);
+        }
+    };
+
+    bool initialized() const {
+        return set.initialized();
+    }
+
+    Ptr lookup(const Lookup& l) const {
+        Ptr ptr = set.lookup(l);
+        if (needsBarrier && ptr && entryNeedsSweep(*ptr)) {
+            const_cast<Set&>(set).remove(ptr);
+            return Ptr();
+        }
+        return ptr;
+    }
+
+    AddPtr lookupForAdd(const Lookup& l) const {
+        AddPtr ptr = set.lookupForAdd(l);
+        if (needsBarrier && ptr && entryNeedsSweep(*ptr)) {
+            const_cast<Set&>(set).remove(ptr);
+            return set.lookupForAdd(l);
+        }
+        return ptr;
+    }
+
+    Range all() const {
+        return Range(set.all());
+    }
+
+    bool empty() const {
+        // This operation is not currently allowed while barriers are in place
+        // as it would require iterating the set and the caller expects a
+        // constant time operation.
+        MOZ_ASSERT(!needsBarrier);
+        return set.empty();
+    }
+
+    uint32_t count() const {
+        // This operation is not currently allowed while barriers are in place
+        // as it would require iterating the set and the caller expects a
+        // constant time operation.
+        MOZ_ASSERT(!needsBarrier);
+        return set.count();
+    }
+
+    size_t capacity() const {
+        return set.capacity();
+    }
+
+    bool has(const Lookup& l) const {
+        return lookup(l).found();
+    }
+
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
         return set.sizeOfExcludingThis(mallocSizeOf);
     }
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
         return mallocSizeOf(this) + set.sizeOfExcludingThis(mallocSizeOf);
     }
 
-    // Non-const interface.
+    bool init(uint32_t len = 16) {
+        MOZ_ASSERT(!needsBarrier);
+        return set.init(len);
+    }
 
-    struct Enum : public Set::Enum { explicit Enum(Self& o) : Set::Enum(o.set) {} };
+    void clear() {
+        // This operation is not currently allowed while barriers are in place
+        // since it doesn't make sense to clear a cache while it is being swept.
+        MOZ_ASSERT(!needsBarrier);
+        set.clear();
+    }
 
-    bool init(uint32_t len = 16) { return set.init(len); }
-    void clear()                 { set.clear(); }
-    void finish()                { set.finish(); }
-    void remove(Ptr p)           { set.remove(p); }
-    void remove(const Lookup& l) { set.remove(l); }
+    void finish() {
+        // This operation is not currently allowed while barriers are in place
+        // since it doesn't make sense to destroy a cache while it is being swept.
+        MOZ_ASSERT(!needsBarrier);
+        set.finish();
+    }
+
+    void remove(Ptr p) {
+        // This currently supports removing entries during incremental
+        // sweeping. If we allow these tables to be swept incrementally this may
+        // no longer be possible.
+        set.remove(p);
+    }
+
+    void remove(const Lookup& l) {
+        Ptr p = lookup(l);
+        if (p)
+            remove(p);
+    }
 
     template<typename TInput>
     bool add(AddPtr& p, TInput&& t) {
         return set.add(p, mozilla::Forward<TInput>(t));
     }
 
     template<typename TInput>
     bool relookupOrAdd(AddPtr& p, const Lookup& l, TInput&& t) {
--- a/js/public/SweepingAPI.h
+++ b/js/public/SweepingAPI.h
@@ -32,18 +32,27 @@ class WeakCacheBase : public mozilla::Li
         shadow::RegisterWeakCache(zone, this);
     }
     explicit WeakCacheBase(JSRuntime* rt) {
         shadow::RegisterWeakCache(rt, this);
     }
     WeakCacheBase(WeakCacheBase&& other) = default;
     virtual ~WeakCacheBase() {}
 
-    virtual void sweep() = 0;
+    virtual size_t sweep() = 0;
     virtual bool needsSweep() = 0;
+
+    virtual bool setNeedsIncrementalBarrier(bool needs) {
+        // Derived classes do not support incremental barriers by default.
+        return false;
+    }
+    virtual bool needsIncrementalBarrier() const {
+        // Derived classes do not support incremental barriers by default.
+        return false;
+    }
 };
 } // namespace detail
 
 // A WeakCache stores the given Sweepable container and links itself into a
 // list of such caches that are swept during each GC. A WeakCache can be
 // specific to a zone, or across a whole runtime, depending on which
 // constructor is used.
 template <typename T>
@@ -62,18 +71,19 @@ class WeakCache : protected detail::Weak
     template <typename... Args>
     explicit WeakCache(JSRuntime* rt, Args&&... args)
       : WeakCacheBase(rt), cache(mozilla::Forward<Args>(args)...)
     {}
 
     const T& get() const { return cache; }
     T& get() { return cache; }
 
-    void sweep() override {
+    size_t sweep() override {
         GCPolicy<T>::sweep(&cache);
+        return 0;
     }
 
     bool needsSweep() override {
         return cache.needsSweep();
     }
 };
 
 } // namespace JS
--- a/js/src/vm/ObjectGroup.cpp
+++ b/js/src/vm/ObjectGroup.cpp
@@ -379,16 +379,25 @@ struct ObjectGroupCompartment::NewEntry
         const Class* clasp;
         TaggedProto proto;
         JSObject* associated;
 
         Lookup(const Class* clasp, TaggedProto proto, JSObject* associated)
           : clasp(clasp), proto(proto), associated(associated)
         {}
 
+        explicit Lookup(const NewEntry& entry)
+          : clasp(entry.group.unbarrieredGet()->clasp()),
+            proto(entry.group.unbarrieredGet()->proto()),
+            associated(entry.associated)
+        {
+            if (associated && associated->is<JSFunction>())
+                clasp = nullptr;
+        }
+
         bool hasAssocId() const {
             return !associated || associated->zone()->hasUniqueId(associated);
         }
 
         bool ensureAssocId() const {
             uint64_t unusedId;
             return !associated ||
                    associated->zoneFromAnyThread()->getUniqueId(associated, &unusedId);
@@ -432,16 +441,20 @@ struct ObjectGroupCompartment::NewEntry
     }
 
     static void rekey(NewEntry& k, const NewEntry& newKey) { k = newKey; }
 
     bool needsSweep() {
         return (IsAboutToBeFinalized(&group) ||
                 (associated && IsAboutToBeFinalizedUnbarriered(&associated)));
     }
+
+    bool operator==(const NewEntry& other) const {
+        return group == other.group && associated == other.associated;
+    }
 };
 
 namespace js {
 template <>
 struct FallibleHashMethods<ObjectGroupCompartment::NewEntry>
 {
     template <typename Lookup> static bool hasHash(Lookup&& l) {
         return ObjectGroupCompartment::NewEntry::hasHash(mozilla::Forward<Lookup>(l));
@@ -1911,19 +1924,14 @@ ObjectGroupCompartment::checkNewTableAft
     for (auto r = table->all(); !r.empty(); r.popFront()) {
         NewEntry entry = r.front();
         CheckGCThingAfterMovingGC(entry.group.unbarrieredGet());
         TaggedProto proto = entry.group.unbarrieredGet()->proto();
         if (proto.isObject())
             CheckGCThingAfterMovingGC(proto.toObject());
         CheckGCThingAfterMovingGC(entry.associated);
 
-        const Class* clasp = entry.group.unbarrieredGet()->clasp();
-        if (entry.associated && entry.associated->is<JSFunction>())
-            clasp = nullptr;
-
-        NewEntry::Lookup lookup(clasp, proto, entry.associated);
-        auto ptr = table->lookup(lookup);
+        auto ptr = table->lookup(NewEntry::Lookup(entry));
         MOZ_RELEASE_ASSERT(ptr.found() && &*ptr == &r.front());
     }
 }
 
 #endif // JSGC_HASH_TABLE_CHECKS
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1115,31 +1115,16 @@ Shape::setObjectFlags(JSContext* cx, Bas
 
     StackBaseShape base(last);
     base.flags |= flags;
 
     RootedShape lastRoot(cx, last);
     return replaceLastProperty(cx, base, proto, lastRoot);
 }
 
-/* static */ inline HashNumber
-StackBaseShape::hash(const Lookup& lookup)
-{
-    HashNumber hash = lookup.flags;
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.clasp) >> 3);
-    return hash;
-}
-
-/* static */ inline bool
-StackBaseShape::match(const ReadBarriered<UnownedBaseShape*>& key, const Lookup& lookup)
-{
-    return key.unbarrieredGet()->flags == lookup.flags &&
-           key.unbarrieredGet()->clasp_ == lookup.clasp;
-}
-
 inline
 BaseShape::BaseShape(const StackBaseShape& base)
   : clasp_(base.clasp),
     flags(base.flags),
     slotSpan_(0),
     unowned_(nullptr),
     table_(nullptr)
 {
@@ -1290,33 +1275,16 @@ InitialShapeEntry::InitialShapeEntry() :
 }
 
 inline
 InitialShapeEntry::InitialShapeEntry(Shape* shape, const Lookup::ShapeProto& proto)
   : shape(shape), proto(proto)
 {
 }
 
-/* static */ inline HashNumber
-InitialShapeEntry::hash(const Lookup& lookup)
-{
-    return (RotateLeft(uintptr_t(lookup.clasp) >> 3, 4) ^ lookup.proto.hashCode()) +
-           lookup.nfixed;
-}
-
-/* static */ inline bool
-InitialShapeEntry::match(const InitialShapeEntry& key, const Lookup& lookup)
-{
-    const Shape* shape = key.shape.unbarrieredGet();
-    return lookup.clasp == shape->getObjectClass()
-        && lookup.nfixed == shape->numFixedSlots()
-        && lookup.baseFlags == shape->getObjectFlags()
-        && lookup.proto.match(key.proto);
-}
-
 #ifdef JSGC_HASH_TABLE_CHECKS
 
 void
 Zone::checkInitialShapesTableAfterMovingGC()
 {
     if (!initialShapes().initialized())
         return;
 
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -654,18 +654,25 @@ struct StackBaseShape : public DefaultHa
 
         explicit Lookup(const ReadBarriered<UnownedBaseShape*>& base)
           : flags(base.unbarrieredGet()->getObjectFlags()), clasp(base.unbarrieredGet()->clasp())
         {
             MOZ_ASSERT(!base.unbarrieredGet()->isOwned());
         }
     };
 
-    static inline HashNumber hash(const Lookup& lookup);
-    static inline bool match(const ReadBarriered<UnownedBaseShape*>& key, const Lookup& lookup);
+    static HashNumber hash(const Lookup& lookup) {
+        HashNumber hash = lookup.flags;
+        hash = mozilla::RotateLeft(hash, 4) ^ (uintptr_t(lookup.clasp) >> 3);
+        return hash;
+    }
+    static inline bool match(const ReadBarriered<UnownedBaseShape*>& key, const Lookup& lookup) {
+        return key.unbarrieredGet()->flags == lookup.flags &&
+               key.unbarrieredGet()->clasp_ == lookup.clasp;
+    }
 };
 
 static MOZ_ALWAYS_INLINE js::HashNumber
 HashId(jsid id)
 {
     // HashGeneric alone would work, but bits of atom and symbol addresses
     // could then be recovered from the hash code. See bug 1330769.
     if (MOZ_LIKELY(JSID_IS_ATOM(id)))
@@ -1317,16 +1324,20 @@ class InitialShapeProto
         return key_;
     }
     const PtrType& proto() const {
         return proto_;
     }
     void setProto(TaggedProto proto) {
         proto_ = proto;
     }
+
+    bool operator==(const InitialShapeProto& other) const {
+        return key_ == other.key_ && proto_ == other.proto_;
+    }
 };
 
 /*
  * Entries for the per-zone initialShapes set indexing initial shapes for
  * objects in the zone and the associated types.
  */
 struct InitialShapeEntry
 {
@@ -1365,27 +1376,42 @@ struct InitialShapeEntry
             nfixed = shape->numFixedSlots();
             baseFlags = shape->getObjectFlags();
         }
     };
 
     inline InitialShapeEntry();
     inline InitialShapeEntry(Shape* shape, const Lookup::ShapeProto& proto);
 
-    static inline HashNumber hash(const Lookup& lookup);
-    static inline bool match(const InitialShapeEntry& key, const Lookup& lookup);
-    static void rekey(InitialShapeEntry& k, const InitialShapeEntry& newKey) { k = newKey; }
+    static HashNumber hash(const Lookup& lookup) {
+        return (mozilla::RotateLeft(uintptr_t(lookup.clasp) >> 3, 4) ^ lookup.proto.hashCode()) +
+               lookup.nfixed;
+    }
+    static inline bool match(const InitialShapeEntry& key, const Lookup& lookup) {
+        const Shape* shape = key.shape.unbarrieredGet();
+        return lookup.clasp == shape->getObjectClass()
+            && lookup.nfixed == shape->numFixedSlots()
+            && lookup.baseFlags == shape->getObjectFlags()
+            && lookup.proto.match(key.proto);
+    }
+    static void rekey(InitialShapeEntry& k, const InitialShapeEntry& newKey) {
+        k = newKey;
+    }
 
     bool needsSweep() {
         Shape* ushape = shape.unbarrieredGet();
         TaggedProto uproto = proto.proto().unbarrieredGet();
         JSObject* protoObj = uproto.raw();
         return (gc::IsAboutToBeFinalizedUnbarriered(&ushape) ||
                 (uproto.isObject() && gc::IsAboutToBeFinalizedUnbarriered(&protoObj)));
     }
+
+    bool operator==(const InitialShapeEntry& other) const {
+        return shape == other.shape && proto == other.proto;
+    }
 };
 
 using InitialShapeSet = JS::WeakCache<JS::GCHashSet<InitialShapeEntry,
                                                     InitialShapeEntry,
                                                     SystemAllocPolicy>>;
 
 struct StackShape
 {