Bug 1257982 - root some hashtables r=jonco
☠☠ backed out by eff691bb6cf8 ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Mon, 18 Mar 2019 21:37:05 +0000
changeset 464939 e73fdb7ff05d85aa0a9fb6bba46f27d25ad66041
parent 464938 13425f1eca221fe81b2a7c98fa8d2fab9f254233
child 464940 115db59b20193a54d75520896656baf1e79e560d
push id112486
push useropoprus@mozilla.com
push dateTue, 19 Mar 2019 16:41:04 +0000
treeherdermozilla-inbound@ee866fb50236 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1257982
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 1257982 - root some hashtables r=jonco Differential Revision: https://phabricator.services.mozilla.com/D23620
js/src/frontend/ParseContext.h
js/src/vm/Debugger.cpp
js/src/vm/EnvironmentObject.cpp
--- a/js/src/frontend/ParseContext.h
+++ b/js/src/frontend/ParseContext.h
@@ -73,16 +73,21 @@ class UsedNameTracker {
 
     void resetToScope(uint32_t scriptId, uint32_t scopeId);
 
    public:
     explicit UsedNameInfo(JSContext* cx) : uses_(cx) {}
 
     UsedNameInfo(UsedNameInfo&& other) : uses_(std::move(other.uses_)) {}
 
+    UsedNameInfo& operator=(UsedNameInfo&& other) {
+      uses_ = std::move(other.uses_);
+      return *this;
+    }
+
     bool noteUsedInScope(uint32_t scriptId, uint32_t scopeId) {
       if (uses_.empty() || uses_.back().scopeId < scopeId) {
         return uses_.append(Use{scriptId, scopeId});
       }
       return true;
     }
 
     void noteBoundInScope(uint32_t scriptId, uint32_t scopeId,
@@ -100,31 +105,33 @@ class UsedNameTracker {
       }
     }
 
     bool isUsedInScript(uint32_t scriptId) const {
       return !uses_.empty() && uses_.back().scriptId >= scriptId;
     }
   };
 
-  using UsedNameMap = HashMap<JSAtom*, UsedNameInfo, DefaultHasher<JSAtom*>>;
+  using UsedNameMap = GCHashMap<JSAtom*, UsedNameInfo, DefaultHasher<JSAtom*>>;
 
  private:
-  // The map of names to chains of uses.
-  UsedNameMap map_;
+  // The map of names to chains of uses. UsedNameTracker is not used in LIFO
+  // order with Rooteds, so must use PersistentRooted (for the arbitrary
+  // ordering capability, not for persistence.)
+  PersistentRooted<UsedNameMap> map_;
 
   // Monotonically increasing id for all nested scripts.
   uint32_t scriptCounter_;
 
   // Monotonically increasing id for all nested scopes.
   uint32_t scopeCounter_;
 
  public:
   explicit UsedNameTracker(JSContext* cx)
-      : map_(cx), scriptCounter_(0), scopeCounter_(0) {}
+      : map_(cx, cx), scriptCounter_(0), scopeCounter_(0) {}
 
   uint32_t nextScriptId() {
     MOZ_ASSERT(scriptCounter_ != UINT32_MAX,
                "ParseContext::Scope::init should have prevented wraparound");
     return scriptCounter_++;
   }
 
   uint32_t nextScopeId() {
@@ -660,9 +667,17 @@ class ParseContext : public Nestable<Par
                            mozilla::Maybe<DeclarationKind>* redeclaredKind,
                            uint32_t* prevPos);
 };
 
 }  // namespace frontend
 
 }  // namespace js
 
+namespace JS {
+
+template <>
+struct GCPolicy<js::frontend::UsedNameTracker::UsedNameInfo>
+    : public IgnoreGCPolicy {};
+
+}  // namespace JS
+
 #endif  // frontend_ParseContext_h
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -4533,17 +4533,17 @@ class MOZ_STACK_CLASS Debugger::ScriptQu
       : QueryBase(cx, dbg),
         url(cx),
         displayURLString(cx),
         hasSource(false),
         source(cx, AsVariant(static_cast<ScriptSourceObject*>(nullptr))),
         hasLine(false),
         line(0),
         innermost(false),
-        innermostForRealm(cx->zone()),
+        innermostForRealm(cx, cx->zone()),
         scriptVector(cx, ScriptVector(cx)),
         lazyScriptVector(cx, LazyScriptVector(cx)),
         wasmInstanceVector(cx, WasmInstanceObjectVector(cx)) {}
 
   /*
    * Parse the query object |query|, and prepare to match only the scripts
    * it specifies.
    */
@@ -4801,24 +4801,24 @@ class MOZ_STACK_CLASS Debugger::ScriptQu
 
   /* The line matching scripts must cover. */
   unsigned int line;
 
   /* True if the query has an 'innermost' property whose value is true. */
   bool innermost;
 
   using RealmToScriptMap =
-      HashMap<Realm*, JSScript*, DefaultHasher<Realm*>, ZoneAllocPolicy>;
+      GCHashMap<Realm*, JSScript*, DefaultHasher<Realm*>, ZoneAllocPolicy>;
 
   /*
    * For 'innermost' queries, a map from realms to the innermost script
    * we've seen so far in that realm. (Template instantiation code size
    * explosion ho!)
    */
-  RealmToScriptMap innermostForRealm;
+  Rooted<RealmToScriptMap> innermostForRealm;
 
   /*
    * Accumulate the scripts in an Rooted<ScriptVector> and
    * Rooted<LazyScriptVector>, instead of creating the JS array as we go,
    * because we mustn't allocate JS objects or GC while we use the CellIter.
    */
   Rooted<ScriptVector> scriptVector;
   Rooted<LazyScriptVector> lazyScriptVector;
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -3714,17 +3714,17 @@ bool js::GetFrameEnvironmentAndScope(JSC
   } else {
     scope.set(frame.script()->innermostScope(pc));
   }
   return true;
 }
 
 #ifdef DEBUG
 
-typedef HashSet<PropertyName*> PropertyNameSet;
+using PropertyNameSet = GCHashSet<PropertyName*>;
 
 static bool RemoveReferencedNames(JSContext* cx, HandleScript script,
                                   PropertyNameSet& remainingNames) {
   // Remove from remainingNames --- the closure variables in some outer
   // script --- any free variables in this script. This analysis isn't perfect:
   //
   // - It will not account for free variables in an inner script which are
   //   actually accessing some name in an intermediate script between the
@@ -3792,29 +3792,29 @@ static bool RemoveReferencedNames(JSCont
   }
 
   return true;
 }
 
 static bool AnalyzeEntrainedVariablesInScript(JSContext* cx,
                                               HandleScript script,
                                               HandleScript innerScript) {
-  PropertyNameSet remainingNames(cx);
+  Rooted<PropertyNameSet> remainingNames(cx, cx);
 
   for (BindingIter bi(script); bi; bi++) {
     if (bi.closedOver()) {
       PropertyName* name = bi.name()->asPropertyName();
       PropertyNameSet::AddPtr p = remainingNames.lookupForAdd(name);
       if (!p && !remainingNames.add(p, name)) {
         return false;
       }
     }
   }
 
-  if (!RemoveReferencedNames(cx, innerScript, remainingNames)) {
+  if (!RemoveReferencedNames(cx, innerScript, remainingNames.get())) {
     return false;
   }
 
   if (!remainingNames.empty()) {
     Sprinter buf(cx);
     if (!buf.init()) {
       return false;
     }