Bug 1571682 - Make allocationSiteGroup fail cleanly on OOM r=tcampbell,jandem?
☠☠ backed out by 17ef275f8a79 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 15 Aug 2019 14:21:35 +0000
changeset 488246 d8d687ca13f8a8f4b5820e31aa6a6c02413f608a
parent 488245 5104e61fa2b99ffd00c676a2af080763f2e319c6
child 488247 2c738f42b334a678393e359780c82e3ce079dbba
push id113904
push userncsoregi@mozilla.com
push dateThu, 15 Aug 2019 19:41:00 +0000
treeherdermozilla-inbound@b283a7ef186c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1571682
milestone70.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 1571682 - Make allocationSiteGroup fail cleanly on OOM r=tcampbell,jandem? 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
@@ -399,16 +399,31 @@ 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;
+  }
+};
+
+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);
@@ -430,47 +445,23 @@ 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
-
 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_;
@@ -1321,18 +1312,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);
@@ -1357,55 +1347,71 @@ 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;
   }
 };
 
+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());
+  }
+};
+
 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
@@ -601,16 +601,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;