Bug 1571682 - Make allocationSiteGroup fail cleanly on OOM r=tcampbell,jandem
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 15 Aug 2019 16:18:21 +0000
changeset 488282 36da938c155282e93753e0d0db8c61bcf1ddd04a
parent 488281 d024bd1f20c28bbf183decce5582427eff6f8785
child 488283 6b4a203e3a5567e117999b454cb408f7f5707393
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell, jandem
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,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);
@@ -430,47 +446,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_;
@@ -1321,18 +1314,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 +1349,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
@@ -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;