Bug 1571682 - Make allocationSiteGroup fail cleanly on OOM. r=tcampbell,jandem, a=RyanVM
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 15 Aug 2019 16:18:21 +0000
changeset 545172 e1341fad0145dddfa2056caad74136b73b89e163
parent 545171 d047e1dbad3b2f30eab8154c76150e0f7e18c580
child 545173 4c1107c95830b5f1714cd408c261c015af099596
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell, jandem, RyanVM
bugs1571682
milestone69.0
Bug 1571682 - Make allocationSiteGroup fail cleanly on OOM. r=tcampbell,jandem, a=RyanVM This should make this fail cleanly on OOM rather than crashing, which should make this crash go away (without reducing memory usage obviously). The problem was the lack of hasHash/ensureHash methods that we use to handle OOM when generating unique IDs for GC things. I also tidied the equivalent code for ObjectGroupRealm::NewEntry (FallibleHashMethods is already implemented for MovableCellHasher). We could further improve this by giving each script an immutable hash code on creation if you think it's worth the tradeoff of storing this for every script. Differential Revision: https://phabricator.services.mozilla.com/D41233
js/src/vm/ObjectGroup.cpp
js/src/vm/ObjectGroup.h
--- a/js/src/vm/ObjectGroup.cpp
+++ b/js/src/vm/ObjectGroup.cpp
@@ -404,16 +404,32 @@ struct ObjectGroupRealm::NewEntry {
           proto(entry.group.unbarrieredGet()->proto()),
           associated(entry.associated) {
       if (associated && associated->is<JSFunction>()) {
         clasp = nullptr;
       }
     }
   };
 
+  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 MovableCellHasher<ObjectGroupRealm::NewEntry> {
+  using Key = ObjectGroupRealm::NewEntry;
+  using Lookup = ObjectGroupRealm::NewEntry::Lookup;
+
   static bool hasHash(const Lookup& l) {
     return MovableCellHasher<TaggedProto>::hasHash(l.proto) &&
            MovableCellHasher<JSObject*>::hasHash(l.associated);
   }
 
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<TaggedProto>::ensureHash(l.proto) &&
            MovableCellHasher<JSObject*>::ensureHash(l.associated);
@@ -435,47 +451,24 @@ struct ObjectGroupRealm::NewEntry {
     TaggedProto proto = key.group.unbarrieredGet()->proto();
     if (!MovableCellHasher<TaggedProto>::match(proto, lookup.proto)) {
       return false;
     }
 
     return MovableCellHasher<JSObject*>::match(key.associated,
                                                lookup.associated);
   }
-
-  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 mozilla {
-template <>
-struct FallibleHashMethods<ObjectGroupRealm::NewEntry> {
-  template <typename Lookup>
-  static bool hasHash(Lookup&& l) {
-    return ObjectGroupRealm::NewEntry::hasHash(std::forward<Lookup>(l));
-  }
-  template <typename Lookup>
-  static bool ensureHash(Lookup&& l) {
-    return ObjectGroupRealm::NewEntry::ensureHash(std::forward<Lookup>(l));
-  }
-};
-}  // namespace mozilla
+} // namespace js
 
 class ObjectGroupRealm::NewTable
-    : public JS::WeakCache<
-          js::GCHashSet<NewEntry, NewEntry, SystemAllocPolicy>> {
-  using Table = js::GCHashSet<NewEntry, NewEntry, SystemAllocPolicy>;
+    : public JS::WeakCache<js::GCHashSet<NewEntry, MovableCellHasher<NewEntry>,
+                                         SystemAllocPolicy>> {
+  using Table =
+      js::GCHashSet<NewEntry, MovableCellHasher<NewEntry>, SystemAllocPolicy>;
   using Base = JS::WeakCache<Table>;
 
  public:
   explicit NewTable(Zone* zone) : Base(zone) {}
 };
 
 /* static*/ ObjectGroupRealm& ObjectGroupRealm::get(const ObjectGroup* group) {
   return group->realm()->objectGroups_;
@@ -1326,18 +1319,17 @@ JSObject* ObjectGroup::newPlainObject(JS
 
   return obj;
 }
 
 /////////////////////////////////////////////////////////////////////
 // ObjectGroupRealm AllocationSiteTable
 /////////////////////////////////////////////////////////////////////
 
-struct ObjectGroupRealm::AllocationSiteKey
-    : public DefaultHasher<AllocationSiteKey> {
+struct ObjectGroupRealm::AllocationSiteKey {
   WeakHeapPtrScript script;
 
   uint32_t offset : 24;
   JSProtoKey kind : 8;
 
   WeakHeapPtrObject proto;
 
   static const uint32_t OFFSET_LIMIT = (1 << 23);
@@ -1362,55 +1354,73 @@ struct ObjectGroupRealm::AllocationSiteK
 
   void operator=(AllocationSiteKey&& key) {
     script = std::move(key.script);
     offset = key.offset;
     kind = key.kind;
     proto = std::move(key.proto);
   }
 
-  static inline HashNumber hash(const AllocationSiteKey& key) {
-    JSScript* script = key.script.unbarrieredGet();
-    JSObject* proto = key.proto.unbarrieredGet();
-    HashNumber hash = mozilla::HashGeneric(key.offset, key.kind);
-    hash = mozilla::AddToHash(hash, MovableCellHasher<JSScript*>::hash(script));
-    hash = mozilla::AddToHash(hash, MovableCellHasher<JSObject*>::hash(proto));
-    return hash;
-  }
-
-  static inline bool match(const AllocationSiteKey& a,
-                           const AllocationSiteKey& b) {
-    return a.offset == b.offset && a.kind == b.kind &&
-           MovableCellHasher<JSScript*>::match(a.script.unbarrieredGet(),
-                                               b.script.unbarrieredGet()) &&
-           MovableCellHasher<JSObject*>::match(a.proto, b.proto);
-  }
-
   void trace(JSTracer* trc) {
     TraceRoot(trc, &script, "AllocationSiteKey script");
     TraceNullableRoot(trc, &proto, "AllocationSiteKey proto");
   }
 
   bool needsSweep() {
     return IsAboutToBeFinalizedUnbarriered(script.unsafeGet()) ||
            (proto && IsAboutToBeFinalizedUnbarriered(proto.unsafeGet()));
   }
 
   bool operator==(const AllocationSiteKey& other) const {
     return script == other.script && offset == other.offset &&
            kind == other.kind && proto == other.proto;
   }
 };
 
+namespace js {
+template <>
+struct MovableCellHasher<ObjectGroupRealm::AllocationSiteKey> {
+  using Key = ObjectGroupRealm::AllocationSiteKey;
+  using Lookup = ObjectGroupRealm::AllocationSiteKey;
+
+  static bool hasHash(const Lookup& l) {
+    return MovableCellHasher<JSScript*>::hasHash(l.script.unbarrieredGet()) &&
+           MovableCellHasher<JSObject*>::hasHash(l.proto.unbarrieredGet());
+  }
+  static bool ensureHash(const Lookup& l) {
+    return MovableCellHasher<JSScript*>::ensureHash(
+               l.script.unbarrieredGet()) &&
+           MovableCellHasher<JSObject*>::ensureHash(l.proto.unbarrieredGet());
+  }
+  static inline HashNumber hash(const Key& key) {
+    HashNumber hash = mozilla::HashGeneric(key.offset, key.kind);
+    hash = mozilla::AddToHash(
+        hash, MovableCellHasher<JSScript*>::hash(key.script.unbarrieredGet()));
+    hash = mozilla::AddToHash(
+        hash, MovableCellHasher<JSObject*>::hash(key.proto.unbarrieredGet()));
+    return hash;
+  }
+
+  static inline bool match(const Key& a, const Lookup& b) {
+    return a.offset == b.offset && a.kind == b.kind &&
+           MovableCellHasher<JSScript*>::match(a.script.unbarrieredGet(),
+                                               b.script.unbarrieredGet()) &&
+           MovableCellHasher<JSObject*>::match(a.proto.unbarrieredGet(),
+                                               b.proto.unbarrieredGet());
+  }
+};
+} // namespace js
+
 class ObjectGroupRealm::AllocationSiteTable
-    : public JS::WeakCache<
-          js::GCHashMap<AllocationSiteKey, WeakHeapPtrObjectGroup,
-                        AllocationSiteKey, SystemAllocPolicy>> {
-  using Table = js::GCHashMap<AllocationSiteKey, WeakHeapPtrObjectGroup,
-                              AllocationSiteKey, SystemAllocPolicy>;
+    : public JS::WeakCache<js::GCHashMap<
+          AllocationSiteKey, WeakHeapPtrObjectGroup,
+          MovableCellHasher<AllocationSiteKey>, SystemAllocPolicy>> {
+  using Table =
+      js::GCHashMap<AllocationSiteKey, WeakHeapPtrObjectGroup,
+                    MovableCellHasher<AllocationSiteKey>, SystemAllocPolicy>;
   using Base = JS::WeakCache<Table>;
 
  public:
   explicit AllocationSiteTable(Zone* zone) : Base(zone) {}
 };
 
 /* static */
 ObjectGroup* ObjectGroup::allocationSiteGroup(
--- a/js/src/vm/ObjectGroup.h
+++ b/js/src/vm/ObjectGroup.h
@@ -599,16 +599,17 @@ class ObjectGroupRealm {
   WeakHeapPtrObjectGroup stringSplitStringGroup = {};
 
   // END OF PROPERTIES
 
  private:
   friend class ObjectGroup;
 
   struct AllocationSiteKey;
+  friend struct MovableCellHasher<AllocationSiteKey>;
 
  public:
   struct NewEntry;
 
   ObjectGroupRealm() = default;
   ~ObjectGroupRealm();
 
   ObjectGroupRealm(ObjectGroupRealm&) = delete;