Bug 1529006 - Use Rooted for NewObjectMetadataState. r=jonco
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Wed, 20 Feb 2019 16:09:42 +0100
changeset 519625 584d1d57db0b1474ec36244b5024c1a24876d089
parent 519624 e743e4fa9a4d174b09688eb06b9689f4589d964d
child 519626 18bf7cf021184952bcb7b614734a8c59832ebd15
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1529006
milestone67.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 1529006 - Use Rooted for NewObjectMetadataState. r=jonco Remove the `if (!mozilla::IsPointer<T>::value || thing)` check in GCVariantImplementation::trace, as GCPolicy will dispatch these to GCPointerPolicy and InternalPointerPolicy (for pointers) and StructGCPolicy (for non-pointers). Also use Rooted for prevState_ in AutoSetNewObjectMetadata and remove inherit from CustomAutoRooter.
js/public/GCVariant.h
js/src/vm/Realm.cpp
js/src/vm/Realm.h
--- a/js/public/GCVariant.h
+++ b/js/public/GCVariant.h
@@ -42,19 +42,17 @@ template <typename... Ts>
 struct GCVariantImplementation;
 
 // The base case.
 template <typename T>
 struct GCVariantImplementation<T> {
   template <typename ConcreteVariant>
   static void trace(JSTracer* trc, ConcreteVariant* v, const char* name) {
     T& thing = v->template as<T>();
-    if (!mozilla::IsPointer<T>::value || thing) {
-      GCPolicy<T>::trace(trc, &thing, name);
-    }
+    GCPolicy<T>::trace(trc, &thing, name);
   }
 
   template <typename Matcher, typename ConcreteVariant>
   static typename Matcher::ReturnType match(Matcher& matcher,
                                             Handle<ConcreteVariant> v) {
     const T& thing = v.get().template as<T>();
     return matcher.match(Handle<T>::fromMarkedLocation(&thing));
   }
@@ -71,19 +69,17 @@ struct GCVariantImplementation<T> {
 template <typename T, typename... Ts>
 struct GCVariantImplementation<T, Ts...> {
   using Next = GCVariantImplementation<Ts...>;
 
   template <typename ConcreteVariant>
   static void trace(JSTracer* trc, ConcreteVariant* v, const char* name) {
     if (v->template is<T>()) {
       T& thing = v->template as<T>();
-      if (!mozilla::IsPointer<T>::value || thing) {
-        GCPolicy<T>::trace(trc, &thing, name);
-      }
+      GCPolicy<T>::trace(trc, &thing, name);
     } else {
       Next::trace(trc, v, name);
     }
   }
 
   template <typename Matcher, typename ConcreteVariant>
   static typename Matcher::ReturnType match(Matcher& matcher,
                                             Handle<ConcreteVariant> v) {
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -298,18 +298,18 @@ void ObjectRealm::trace(JSTracer* trc) {
   if (nonSyntacticLexicalEnvironments_) {
     nonSyntacticLexicalEnvironments_->trace(trc);
   }
 }
 
 void Realm::traceRoots(JSTracer* trc,
                        js::gc::GCRuntime::TraceOrMarkRuntime traceOrMark) {
   if (objectMetadataState_.is<PendingMetadata>()) {
-    TraceRoot(trc, &objectMetadataState_.as<PendingMetadata>(),
-              "on-stack object pending metadata");
+    GCPolicy<NewObjectMetadataState>::trace(trc, &objectMetadataState_,
+                                            "on-stack object pending metadata");
   }
 
   if (!JS::RuntimeHeapIsMinorCollecting()) {
     // The global is never nursery allocated, so we don't need to
     // trace it when doing a minor collection.
     //
     // If a compartment is on-stack, we mark its global so that
     // JSContext::global() remains valid.
@@ -919,19 +919,18 @@ void Realm::addSizeOfIncludingThis(
 
 mozilla::HashCodeScrambler Realm::randomHashCodeScrambler() {
   return mozilla::HashCodeScrambler(randomKeyGenerator_.next(),
                                     randomKeyGenerator_.next());
 }
 
 AutoSetNewObjectMetadata::AutoSetNewObjectMetadata(
     JSContext* cx MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
-    : CustomAutoRooter(cx),
-      cx_(cx->helperThread() ? nullptr : cx),
-      prevState_(cx->realm()->objectMetadataState_) {
+    : cx_(cx->helperThread() ? nullptr : cx),
+      prevState_(cx, cx->realm()->objectMetadataState_) {
   MOZ_GUARD_OBJECT_NOTIFIER_INIT;
   if (cx_) {
     cx_->realm()->objectMetadataState_ =
         NewObjectMetadataState(DelayMetadata());
   }
 }
 
 AutoSetNewObjectMetadata::~AutoSetNewObjectMetadata() {
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -15,16 +15,17 @@
 #include "mozilla/Tuple.h"
 #include "mozilla/Variant.h"
 #include "mozilla/XorShift128PlusRNG.h"
 
 #include <stddef.h>
 
 #include "builtin/Array.h"
 #include "gc/Barrier.h"
+#include "js/GCVariant.h"
 #include "js/UniquePtr.h"
 #include "vm/ArrayBufferObject.h"
 #include "vm/Compartment.h"
 #include "vm/ReceiverGuard.h"
 #include "vm/RegExpShared.h"
 #include "vm/SavedStacks.h"
 #include "vm/Time.h"
 #include "wasm/WasmRealm.h"
@@ -169,40 +170,34 @@ class NewProxyCache {
 //           |   |                                  |
 //           V   V                                  |
 //     DelayMetadata -------------------------> PendingMetadata
 //                         via allocation
 //
 // In the presence of internal errors, we do not set the new object's metadata
 // (if it was even allocated) and reset to the previous state on the stack.
 
+// See below in namespace JS for the template specialization for
+// ImmediateMetadata and DelayMetadata.
 struct ImmediateMetadata {};
 struct DelayMetadata {};
 using PendingMetadata = JSObject*;
 
 using NewObjectMetadataState =
     mozilla::Variant<ImmediateMetadata, DelayMetadata, PendingMetadata>;
 
-class MOZ_RAII AutoSetNewObjectMetadata : private JS::CustomAutoRooter {
+class MOZ_RAII AutoSetNewObjectMetadata {
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER;
 
   JSContext* cx_;
-  NewObjectMetadataState prevState_;
+  Rooted<NewObjectMetadataState> prevState_;
 
   AutoSetNewObjectMetadata(const AutoSetNewObjectMetadata& aOther) = delete;
   void operator=(const AutoSetNewObjectMetadata& aOther) = delete;
 
- protected:
-  virtual void trace(JSTracer* trc) override {
-    if (prevState_.is<PendingMetadata>()) {
-      TraceRoot(trc, &prevState_.as<PendingMetadata>(),
-                "Object pending metadata");
-    }
-  }
-
  public:
   explicit AutoSetNewObjectMetadata(
       JSContext* cx MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
   ~AutoSetNewObjectMetadata();
 };
 
 class PropertyIteratorObject;
 
@@ -289,16 +284,23 @@ class ObjectRealm {
       JSContext* cx, js::HandleObject enclosing, js::HandleObject key,
       js::HandleObject thisv);
   js::LexicalEnvironmentObject* getNonSyntacticLexicalEnvironment(
       JSObject* key) const;
 };
 
 }  // namespace js
 
+namespace JS {
+template <> struct GCPolicy<js::ImmediateMetadata>:
+  public IgnoreGCPolicy<js::ImmediateMetadata> {};
+template <> struct GCPolicy<js::DelayMetadata>:
+  public IgnoreGCPolicy<js::DelayMetadata> {};
+} // namespace JS
+
 class JS::Realm : public JS::shadow::Realm {
   JS::Zone* zone_;
   JSRuntime* runtime_;
 
   const JS::RealmCreationOptions creationOptions_;
   JS::RealmBehaviors behaviors_;
 
   friend struct ::JSContext;