Bug 1559385 - Implement a way of tracing a GCCellPtr as a root r=jandem
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 13 Dec 2019 13:29:21 +0000
changeset 506913 dc97cf74084d7ae872e278535895684e94b48bf7
parent 506912 d6c02f97fda99a7e722e6b825ff868e41d74aeda
child 506914 2e490fe1a73ec9bd8ce775ce092dda9ebcbf8894
push id36915
push userrgurzau@mozilla.com
push dateFri, 13 Dec 2019 21:43:22 +0000
treeherdermozilla-central@f09f24f2b545 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1559385
milestone73.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 1559385 - Implement a way of tracing a GCCellPtr as a root r=jandem This removes StackGCCellPtr and a bunch of indirection. Differential Revision: https://phabricator.services.mozilla.com/D56775
js/src/frontend/BytecodeSection.cpp
js/src/frontend/BytecodeSection.h
js/src/gc/Barrier.h
js/src/gc/Marking.cpp
js/src/gc/Policy.h
js/src/gc/Tracer.h
js/src/vm/JSScript.cpp
--- a/js/src/frontend/BytecodeSection.cpp
+++ b/js/src/frontend/BytecodeSection.cpp
@@ -24,18 +24,17 @@ bool GCThingList::append(ObjectBox* objb
   // the ObjectBox to the |lastbox| linked list for finishInnerFunctions below.
 
   MOZ_ASSERT(objbox->isObjectBox());
   MOZ_ASSERT(!objbox->emitLink);
   objbox->emitLink = lastbox;
   lastbox = objbox;
 
   *index = vector.length();
-  return vector.append(
-      mozilla::AsVariant(StackGCCellPtr(JS::GCCellPtr(objbox->object()))));
+  return vector.append(mozilla::AsVariant(JS::GCCellPtr(objbox->object())));
 }
 
 void GCThingList::finishInnerFunctions() {
   ObjectBox* objbox = lastbox;
   while (objbox) {
     if (objbox->isFunctionBox()) {
       objbox->asFunctionBox()->finish();
     }
@@ -47,18 +46,18 @@ bool GCThingList::finish(JSContext* cx, 
   MOZ_ASSERT(length() <= INDEX_LIMIT);
   MOZ_ASSERT(length() == array.size());
 
   struct Matcher {
     JSContext* cx;
     uint32_t i;
     mozilla::Span<JS::GCCellPtr>& array;
 
-    bool operator()(StackGCCellPtr& value) {
-      array[i] = value.get();
+    bool operator()(const JS::GCCellPtr& value) {
+      array[i] = value;
       return true;
     }
 
     bool operator()(BigIntCreationData& data) {
       BigInt* bi = data.createBigInt(cx);
       if (!bi) {
         return false;
       }
--- a/js/src/frontend/BytecodeSection.h
+++ b/js/src/frontend/BytecodeSection.h
@@ -41,69 +41,67 @@ class Scope;
 using BigIntVector = JS::GCVector<js::BigInt*>;
 
 namespace frontend {
 
 class BigIntLiteral;
 class ObjectBox;
 
 struct MOZ_STACK_CLASS GCThingList {
-  using ListType = mozilla::Variant<StackGCCellPtr, BigIntCreationData,
+  using ListType = mozilla::Variant<JS::GCCellPtr, BigIntCreationData,
                                     ObjLiteralCreationData, RegExpCreationData>;
   JS::RootedVector<ListType> vector;
 
   // Last emitted object.
   ObjectBox* lastbox = nullptr;
 
   // Index of the first scope in the vector.
   mozilla::Maybe<uint32_t> firstScopeIndex;
 
   explicit GCThingList(JSContext* cx) : vector(cx) {}
 
   MOZ_MUST_USE bool append(Scope* scope, uint32_t* index) {
     *index = vector.length();
-    if (!vector.append(
-            mozilla::AsVariant(StackGCCellPtr(JS::GCCellPtr(scope))))) {
+    if (!vector.append(mozilla::AsVariant(JS::GCCellPtr(scope)))) {
       return false;
     }
     if (!firstScopeIndex) {
       firstScopeIndex.emplace(*index);
     }
     return true;
   }
   MOZ_MUST_USE bool append(BigIntLiteral* literal, uint32_t* index) {
     *index = vector.length();
     if (literal->isDeferred()) {
       return vector.append(mozilla::AsVariant(literal->creationData()));
     }
-    return vector.append(
-        mozilla::AsVariant(StackGCCellPtr(JS::GCCellPtr(literal->value()))));
+    return vector.append(mozilla::AsVariant(JS::GCCellPtr(literal->value())));
   }
   MOZ_MUST_USE bool append(RegExpLiteral* literal, uint32_t* index) {
     *index = vector.length();
     if (literal->isDeferred()) {
       return vector.append(
           mozilla::AsVariant(std::move(literal->creationData())));
     }
-    return vector.append(mozilla::AsVariant(
-        StackGCCellPtr(JS::GCCellPtr(literal->objbox()->object()))));
+    return vector.append(
+        mozilla::AsVariant(JS::GCCellPtr(literal->objbox()->object())));
   }
   MOZ_MUST_USE bool append(ObjLiteralCreationData&& objlit, uint32_t* index) {
     *index = vector.length();
     return vector.append(mozilla::AsVariant(std::move(objlit)));
   }
   MOZ_MUST_USE bool append(ObjectBox* obj, uint32_t* index);
 
   uint32_t length() const { return vector.length(); }
   MOZ_MUST_USE bool finish(JSContext* cx, mozilla::Span<JS::GCCellPtr> array);
   void finishInnerFunctions();
 
   AbstractScope getScope(size_t index) const {
     auto& elem = vector[index].get();
-    return AbstractScope(&elem.as<StackGCCellPtr>().get().as<Scope>());
+    return AbstractScope(&elem.as<JS::GCCellPtr>().as<Scope>());
   }
 
   AbstractScope firstScope() const {
     MOZ_ASSERT(firstScopeIndex.isSome());
     return getScope(*firstScopeIndex);
   }
 };
 
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -1005,31 +1005,16 @@ struct WeakHeapPtrHasher {
 
   static HashNumber hash(Lookup obj) { return DefaultHasher<T>::hash(obj); }
   static bool match(const Key& k, Lookup l) { return k.unbarrieredGet() == l; }
   static void rekey(Key& k, const Key& newKey) {
     k.set(newKey.unbarrieredGet());
   }
 };
 
-// Wrapper around GCCellPtr for use with RootedVector<StackGCCellPtr>.
-class MOZ_STACK_CLASS StackGCCellPtr {
-  JS::GCCellPtr ptr_;
-
- public:
-  MOZ_IMPLICIT StackGCCellPtr(JS::GCCellPtr ptr) : ptr_(ptr) {}
-  StackGCCellPtr() = default;
-
-  void operator=(const StackGCCellPtr& other) { ptr_ = other.ptr_; }
-
-  void trace(JSTracer* trc);
-
-  JS::GCCellPtr get() const { return ptr_; }
-};
-
 }  // namespace js
 
 namespace mozilla {
 
 template <class T>
 struct DefaultHasher<js::HeapPtr<T>> : js::HeapPtrHasher<T> {};
 
 template <class T>
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -539,29 +539,35 @@ void js::TraceProcessGlobalRoot(JSTracer
     DoCallback(trc->asCallbackTracer(), ConvertToBase(&thing), name);
   }
 }
 template void js::TraceProcessGlobalRoot<JSAtom>(JSTracer*, JSAtom*,
                                                  const char*);
 template void js::TraceProcessGlobalRoot<JS::Symbol>(JSTracer*, JS::Symbol*,
                                                      const char*);
 
+static Cell* TraceGenericPointerRootAndType(JSTracer* trc, Cell* thing,
+                                            JS::TraceKind kind,
+                                            const char* name) {
+  return MapGCThingTyped(thing, kind, [trc, name](auto t) -> Cell* {
+    TraceRoot(trc, &t, name);
+    return t;
+  });
+}
+
 void js::TraceGenericPointerRoot(JSTracer* trc, Cell** thingp,
                                  const char* name) {
   MOZ_ASSERT(thingp);
   Cell* thing = *thingp;
   if (!thing) {
     return;
   }
 
-  auto traced = MapGCThingTyped(thing, thing->getTraceKind(),
-                                [trc, name](auto t) -> Cell* {
-                                  TraceRoot(trc, &t, name);
-                                  return t;
-                                });
+  Cell* traced =
+      TraceGenericPointerRootAndType(trc, thing, thing->getTraceKind(), name);
   if (traced != thing) {
     *thingp = traced;
   }
 }
 
 void js::TraceManuallyBarrieredGenericPointerEdge(JSTracer* trc, Cell** thingp,
                                                   const char* name) {
   MOZ_ASSERT(thingp);
@@ -575,21 +581,30 @@ void js::TraceManuallyBarrieredGenericPo
                                   TraceManuallyBarrieredEdge(trc, &t, name);
                                   return t;
                                 });
   if (traced != thing) {
     *thingp = traced;
   }
 }
 
-void StackGCCellPtr::trace(JSTracer* trc) {
-  Cell* thing = ptr_.asCell();
-  TraceGenericPointerRoot(trc, &thing, "stack-gc-cell-ptr");
-  if (thing != ptr_.asCell()) {
-    ptr_ = JS::GCCellPtr(thing, ptr_.kind());
+void js::TraceGCCellPtrRoot(JSTracer* trc, JS::GCCellPtr* thingp,
+                            const char* name) {
+  Cell* thing = thingp->asCell();
+  if (!thing) {
+    return;
+  }
+
+  Cell* traced =
+      TraceGenericPointerRootAndType(trc, thing, thingp->kind(), name);
+
+  if (!traced) {
+    *thingp = JS::GCCellPtr();
+  } else if (traced != thingp->asCell()) {
+    *thingp = JS::GCCellPtr(traced, thingp->kind());
   }
 }
 
 // This method is responsible for dynamic dispatch to the real tracer
 // implementation. Consider replacing this choke point with virtual dispatch:
 // a sufficiently smart C++ compiler may be able to devirtualize some paths.
 template <typename T>
 bool js::gc::TraceEdgeInternal(JSTracer* trc, T* thingp, const char* name) {
--- a/js/src/gc/Policy.h
+++ b/js/src/gc/Policy.h
@@ -95,11 +95,20 @@ struct GCPolicy<js::WeakHeapPtr<T>> {
   static bool needsSweep(js::WeakHeapPtr<T>* thingp) {
     return js::gc::IsAboutToBeFinalized(thingp);
   }
   static bool traceWeak(JSTracer* trc, js::WeakHeapPtr<T>* thingp) {
     return js::TraceWeakEdge(trc, thingp, "traceWeak");
   }
 };
 
+template <>
+struct GCPolicy<JS::GCCellPtr> {
+  static void trace(JSTracer* trc, JS::GCCellPtr* thingp, const char* name) {
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking.
+    js::TraceGCCellPtrRoot(trc, thingp, name);
+  }
+};
+
 }  // namespace JS
 
 #endif  // gc_Policy_h
--- a/js/src/gc/Tracer.h
+++ b/js/src/gc/Tracer.h
@@ -249,16 +249,18 @@ void TraceProcessGlobalRoot(JSTracer* tr
 void TraceGenericPointerRoot(JSTracer* trc, gc::Cell** thingp,
                              const char* name);
 
 // Trace a non-root edge that uses the base GC thing type, instead of a more
 // specific type.
 void TraceManuallyBarrieredGenericPointerEdge(JSTracer* trc, gc::Cell** thingp,
                                               const char* name);
 
+void TraceGCCellPtrRoot(JSTracer* trc, JS::GCCellPtr* thingp, const char* name);
+
 // Deprecated. Please use one of the strongly typed variants above.
 void TraceChildren(JSTracer* trc, void* thing, JS::TraceKind kind);
 
 namespace gc {
 
 // Trace through a shape or group iteratively during cycle collection to avoid
 // deep or infinite recursion.
 void TraceCycleCollectorChildren(JS::CallbackTracer* trc, Shape* shape);
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -4962,17 +4962,17 @@ static JSObject* CloneInnerInterpretedFu
   }
 
   return clone;
 }
 
 static JSObject* CloneScriptObject(JSContext* cx, PrivateScriptData* srcData,
                                    HandleObject obj,
                                    Handle<ScriptSourceObject*> sourceObject,
-                                   JS::HandleVector<StackGCCellPtr> gcThings) {
+                                   JS::HandleVector<JS::GCCellPtr> gcThings) {
   if (obj->is<RegExpObject>()) {
     return CloneScriptRegExpObject(cx, obj->as<RegExpObject>());
   }
 
   if (obj->is<JSFunction>()) {
     HandleFunction innerFun = obj.as<JSFunction>();
     if (innerFun->isNative()) {
       if (cx->realm() != innerFun->realm()) {
@@ -4987,33 +4987,32 @@ static JSObject* CloneScriptObject(JSCon
       AutoRealm ar(cx, innerFun);
       if (!JSFunction::getOrCreateScript(cx, innerFun)) {
         return nullptr;
       }
     }
 
     Scope* enclosing = innerFun->nonLazyScript()->enclosingScope();
     uint32_t scopeIndex = FindScopeIndex(srcData->gcthings(), *enclosing);
-    RootedScope enclosingClone(cx,
-                               &gcThings[scopeIndex].get().get().as<Scope>());
+    RootedScope enclosingClone(cx, &gcThings[scopeIndex].get().as<Scope>());
     return CloneInnerInterpretedFunction(cx, enclosingClone, innerFun,
                                          sourceObject);
   }
 
   return DeepCloneObjectLiteral(cx, obj, TenuredObject);
 }
 
 /* static */
 bool PrivateScriptData::Clone(JSContext* cx, HandleScript src, HandleScript dst,
                               MutableHandle<GCVector<Scope*>> scopes) {
   PrivateScriptData* srcData = src->data_;
   uint32_t ngcthings = srcData->gcthings().size();
 
   // Clone GC things.
-  JS::RootedVector<StackGCCellPtr> gcThings(cx);
+  JS::RootedVector<JS::GCCellPtr> gcThings(cx);
   size_t scopeIndex = 0;
   Rooted<ScriptSourceObject*> sourceObject(cx, dst->sourceObject());
   RootedObject obj(cx);
   RootedScope scope(cx);
   RootedScope enclosingScope(cx);
   RootedBigInt bigint(cx);
   for (JS::GCCellPtr gcThing : srcData->gcthings()) {
     if (gcThing.is<JSObject>()) {
@@ -5030,17 +5029,17 @@ bool PrivateScriptData::Clone(JSContext*
       if (scopeIndex < scopes.length()) {
         if (!gcThings.append(JS::GCCellPtr(scopes[scopeIndex].get()))) {
           return false;
         }
       } else {
         scope = &gcThing.as<Scope>();
         uint32_t enclosingScopeIndex =
             FindScopeIndex(srcData->gcthings(), *scope->enclosing());
-        enclosingScope = &gcThings[enclosingScopeIndex].get().get().as<Scope>();
+        enclosingScope = &gcThings[enclosingScopeIndex].get().as<Scope>();
         Scope* clone = Scope::clone(cx, scope, enclosingScope);
         if (!clone || !gcThings.append(JS::GCCellPtr(clone))) {
           return false;
         }
       }
       scopeIndex++;
     } else {
       bigint = &gcThing.as<BigInt>();
@@ -5061,17 +5060,17 @@ bool PrivateScriptData::Clone(JSContext*
   if (!JSScript::createPrivateScriptData(cx, dst, ngcthings)) {
     return false;
   }
 
   PrivateScriptData* dstData = dst->data_;
   {
     auto array = dstData->gcthings();
     for (uint32_t i = 0; i < ngcthings; ++i) {
-      array[i] = gcThings[i].get().get();
+      array[i] = gcThings[i].get();
     }
   }
 
   return true;
 }
 
 JSScript* js::detail::CopyScript(JSContext* cx, HandleScript src,
                                  HandleObject functionOrGlobal,