Bug 1540719 - Perform a last ditch GC if symbol allocation fails as we do for most other GC things r=sfink
☠☠ backed out by 7d3896c97b07 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 01 Apr 2019 18:36:41 +0100
changeset 527355 87ae5e3b6962b5db12bcb271a1fd2aef4e20a809
parent 527309 1b7309612d60dc2a2d8e3fcf99416a9fa597dd89
child 527356 0199e2beb6f94dda62383ff2f7b0d401b148d16a
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1540719
milestone68.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 1540719 - Perform a last ditch GC if symbol allocation fails as we do for most other GC things r=sfink
js/src/vm/JSAtom.cpp
js/src/vm/SymbolType.cpp
js/src/vm/SymbolType.h
--- a/js/src/vm/JSAtom.cpp
+++ b/js/src/vm/JSAtom.cpp
@@ -288,18 +288,18 @@ bool JSRuntime::initializeAtoms(JSContex
     return false;
   }
 
   ImmutablePropertyNamePtr* descriptions =
       commonNames->wellKnownSymbolDescriptions();
   ImmutableSymbolPtr* symbols =
       reinterpret_cast<ImmutableSymbolPtr*>(wellKnownSymbols.ref());
   for (size_t i = 0; i < JS::WellKnownSymbolLimit; i++) {
-    JS::Symbol* symbol =
-        JS::Symbol::new_(cx, JS::SymbolCode(i), descriptions[i]);
+    HandlePropertyName description = descriptions[i];
+    JS::Symbol* symbol = JS::Symbol::new_(cx, JS::SymbolCode(i), description);
     if (!symbol) {
       ReportOutOfMemory(cx);
       return false;
     }
     symbols[i].init(symbol);
   }
 
   return true;
@@ -925,18 +925,17 @@ template <typename Chars>
 static MOZ_ALWAYS_INLINE JSAtom* AllocateNewAtom(
     JSContext* cx, Chars chars, size_t length, PinningBehavior pin,
     const Maybe<uint32_t>& indexValue, const AtomHasher::Lookup& lookup) {
   AutoAllocInAtomsZone ac(cx);
 
   JSFlatString* flat = MakeFlatStringForAtomization(cx, chars, length);
   if (!flat) {
     // Grudgingly forgo last-ditch GC. The alternative would be to release
-    // the lock, manually GC here, and retry from the top. If you fix this,
-    // please also fix or comment the similar case in Symbol::new_.
+    // the lock, manually GC here, and retry from the top.
     ReportOutOfMemory(cx);
     return nullptr;
   }
 
   JSAtom* atom = flat->morphAtomizedStringIntoAtom(lookup.hash);
   MOZ_ASSERT(atom->hash() == lookup.hash);
 
   if (pin) {
--- a/js/src/vm/SymbolType.cpp
+++ b/js/src/vm/SymbolType.cpp
@@ -3,83 +3,78 @@
  * 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/. */
 
 #include "vm/SymbolType.h"
 
 #include "builtin/Symbol.h"
 #include "gc/Allocator.h"
+#include "gc/HashUtil.h"
 #include "gc/Rooting.h"
 #include "util/StringBuffer.h"
 #include "vm/JSContext.h"
 #include "vm/Realm.h"
 
 #include "vm/Realm-inl.h"
 
 using JS::Symbol;
 using namespace js;
 
 Symbol* Symbol::newInternal(JSContext* cx, JS::SymbolCode code, uint32_t hash,
-                            JSAtom* description) {
+                            HandleAtom description) {
   MOZ_ASSERT(CurrentThreadCanAccessRuntime(cx->runtime()));
   AutoAllocInAtomsZone az(cx);
 
-  // Following js::AtomizeString, we grudgingly forgo last-ditch GC here.
-  Symbol* p = Allocate<JS::Symbol, NoGC>(cx);
+  Symbol* p = Allocate<JS::Symbol>(cx);
   if (!p) {
-    ReportOutOfMemory(cx);
     return nullptr;
   }
   return new (p) Symbol(code, hash, description);
 }
 
 Symbol* Symbol::new_(JSContext* cx, JS::SymbolCode code,
-                     JSString* description) {
-  JSAtom* atom = nullptr;
+                     HandleString description) {
+  RootedAtom atom(cx);
   if (description) {
     atom = AtomizeString(cx, description);
     if (!atom) {
       return nullptr;
     }
   }
 
   Symbol* sym = newInternal(cx, code, cx->runtime()->randomHashCode(), atom);
   if (sym) {
     cx->markAtom(sym);
   }
   return sym;
 }
 
 Symbol* Symbol::for_(JSContext* cx, HandleString description) {
-  JSAtom* atom = AtomizeString(cx, description);
+  RootedAtom atom(cx, AtomizeString(cx, description));
   if (!atom) {
     return nullptr;
   }
 
   SymbolRegistry& registry = cx->symbolRegistry();
-  SymbolRegistry::AddPtr p = registry.lookupForAdd(atom);
+  DependentAddPtr<SymbolRegistry> p(cx, registry, atom);
   if (p) {
     cx->markAtom(*p);
     return *p;
   }
 
   // Rehash the hash of the atom to give the corresponding symbol a hash
   // that is different than the hash of the corresponding atom.
   HashNumber hash = mozilla::HashGeneric(atom->hash());
   Symbol* sym = newInternal(cx, SymbolCode::InSymbolRegistry, hash, atom);
   if (!sym) {
     return nullptr;
   }
 
-  // p is still valid here because we only access the symbol registry from the
-  // main thread, and newInternal can't GC.
-  if (!registry.add(p, sym)) {
-    // SystemAllocPolicy does not report OOM.
-    ReportOutOfMemory(cx);
+  if (!p.add(cx, registry, atom, sym)) {
     return nullptr;
   }
 
   cx->markAtom(sym);
   return sym;
 }
 
 #if defined(DEBUG) || defined(JS_JITSPEW)
--- a/js/src/vm/SymbolType.h
+++ b/js/src/vm/SymbolType.h
@@ -44,30 +44,31 @@ class Symbol : public js::gc::TenuredCel
 
   Symbol(SymbolCode code, js::HashNumber hash, JSAtom* desc)
       : description_(desc), code_(code), hash_(hash) {}
 
   Symbol(const Symbol&) = delete;
   void operator=(const Symbol&) = delete;
 
   static Symbol* newInternal(JSContext* cx, SymbolCode code,
-                             js::HashNumber hash, JSAtom* description);
+                             js::HashNumber hash, js::HandleAtom description);
 
   static void staticAsserts() {
     static_assert(uint32_t(SymbolCode::WellKnownAPILimit) ==
                       JS::shadow::Symbol::WellKnownAPILimit,
                   "JS::shadow::Symbol::WellKnownAPILimit must match "
                   "SymbolCode::WellKnownAPILimit");
     static_assert(
         offsetof(Symbol, code_) == offsetof(JS::shadow::Symbol, code_),
         "JS::shadow::Symbol::code_ offset must match SymbolCode::code_");
   }
 
  public:
-  static Symbol* new_(JSContext* cx, SymbolCode code, JSString* description);
+  static Symbol* new_(JSContext* cx, SymbolCode code,
+                      js::HandleString description);
   static Symbol* for_(JSContext* cx, js::HandleString description);
 
   JSAtom* description() const { return description_; }
   SymbolCode code() const { return code_; }
   js::HashNumber hash() const { return hash_; }
 
   bool isWellKnownSymbol() const {
     return uint32_t(code_) < WellKnownSymbolLimit;