Bug 1319468 - Part 2: Use Rooted<GCVector> for AutoLookupVector. r=sfink
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Wed, 13 Feb 2019 23:27:52 +0100
changeset 519619 ba49b929457a6df36413a33ce0270af65ad472e1
parent 519618 7a0de0939e2fcc6a3abc11f010fbe5d8ca209cf4
child 519620 9685cdc4b7cb67decb4acc603bfed0b371b77e43
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)
reviewerssfink
bugs1319468
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 1319468 - Part 2: Use Rooted<GCVector> for AutoLookupVector. r=sfink
js/src/vm/SavedFrame.h
js/src/vm/SavedStacks.cpp
js/src/vm/SavedStacks.h
--- a/js/src/vm/SavedFrame.h
+++ b/js/src/vm/SavedFrame.h
@@ -105,36 +105,22 @@ class SavedFrame : public NativeObject {
 
   struct Lookup;
   struct HashPolicy;
 
   typedef JS::GCHashSet<ReadBarriered<SavedFrame*>, HashPolicy,
                         SystemAllocPolicy>
       Set;
 
-  class AutoLookupVector;
-
-  class MOZ_STACK_CLASS HandleLookup {
-    friend class AutoLookupVector;
-
-    Lookup& lookup;
-
-    explicit HandleLookup(Lookup& lookup) : lookup(lookup) {}
-
-   public:
-    inline Lookup& get() { return lookup; }
-    inline Lookup* operator->() { return &lookup; }
-  };
-
  private:
   static SavedFrame* create(JSContext* cx);
   static MOZ_MUST_USE bool finishSavedFrameInit(JSContext* cx,
                                                 HandleObject ctor,
                                                 HandleObject proto);
-  void initFromLookup(JSContext* cx, HandleLookup lookup);
+  void initFromLookup(JSContext* cx, Handle<Lookup> lookup);
   void initSource(JSAtom* source);
   void initSourceId(uint32_t id);
   void initLine(uint32_t line);
   void initColumn(uint32_t column);
   void initFunctionDisplayName(JSAtom* maybeName);
   void initAsyncCause(JSAtom* maybeCause);
   void initParent(SavedFrame* maybeParent);
   void initPrincipalsAlreadyHeld(JSPrincipals* principals);
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -237,34 +237,52 @@ struct SavedFrame::Lookup {
                                  "SavedFrame::Lookup::asyncCause");
     }
     if (parent) {
       TraceManuallyBarrieredEdge(trc, &parent, "SavedFrame::Lookup::parent");
     }
   }
 };
 
-class MOZ_STACK_CLASS SavedFrame::AutoLookupVector
-    : public JS::CustomAutoRooter {
- public:
-  explicit AutoLookupVector(JSContext* cx)
-      : JS::CustomAutoRooter(cx), lookups(cx) {}
+using GCLookupVector = GCVector<SavedFrame::Lookup, ASYNC_STACK_MAX_FRAME_COUNT>;
+
+template <class Wrapper>
+class WrappedPtrOperations<SavedFrame::Lookup, Wrapper> {
+  const SavedFrame::Lookup& value() const {
+    return static_cast<const Wrapper*>(this)->get();
+  }
 
-  typedef Vector<Lookup, ASYNC_STACK_MAX_FRAME_COUNT> LookupVector;
-  inline LookupVector* operator->() { return &lookups; }
-  inline HandleLookup operator[](size_t i) { return HandleLookup(lookups[i]); }
-  inline HandleLookup back() { return HandleLookup(lookups.back()); }
+ public:
+  JSAtom* source() { return value().source; }
+  uint32_t sourceId() { return value().sourceId; }
+  uint32_t line() { return value().line; }
+  uint32_t column() { return value().column; }
+  JSAtom* functionDisplayName() { return value().functionDisplayName; }
+  JSAtom* asyncCause() { return value().asyncCause; }
+  SavedFrame* parent() { return value().parent; }
+  JSPrincipals* principals() { return value().principals; }
+  Maybe<LiveSavedFrameCache::FramePtr> framePtr() { return value().framePtr; }
+  jsbytecode* pc() { return value().pc; }
+  Activation* activation() { return value().activation; }
+};
 
- private:
-  LookupVector lookups;
+template <typename Wrapper>
+class MutableWrappedPtrOperations<SavedFrame::Lookup, Wrapper>
+    : public WrappedPtrOperations<SavedFrame::Lookup, Wrapper> {
+  SavedFrame::Lookup& value() {
+    return static_cast<Wrapper*>(this)->get();
+  }
 
-  virtual void trace(JSTracer* trc) override {
-    for (size_t i = 0; i < lookups.length(); i++) {
-      lookups[i].trace(trc);
-    }
+ public:
+  void setParent(SavedFrame* parent) {
+    value().parent = parent;
+  }
+
+  void setAsyncCause(HandleAtom asyncCause) {
+    value().asyncCause = asyncCause;
   }
 };
 
 /* static */ bool SavedFrame::HashPolicy::hasHash(const Lookup& l) {
   return SavedFramePtrHasher::hasHash(l.parent);
 }
 
 /* static */ bool SavedFrame::HashPolicy::ensureHash(const Lookup& l) {
@@ -485,40 +503,40 @@ void SavedFrame::initAsyncCause(JSAtom* 
                    maybeCause ? StringValue(maybeCause) : NullValue());
 }
 
 void SavedFrame::initParent(SavedFrame* maybeParent) {
   initReservedSlot(JSSLOT_PARENT, ObjectOrNullValue(maybeParent));
 }
 
 void SavedFrame::initFromLookup(JSContext* cx,
-                                SavedFrame::HandleLookup lookup) {
+                                Handle<Lookup> lookup) {
   // Make sure any atoms used in the lookup are marked in the current zone.
   // Normally we would try to keep these mark bits up to date around the
   // points where the context moves between compartments, but Lookups live on
   // the stack (where the atoms are kept alive regardless) and this is a
   // more convenient pinchpoint.
-  if (lookup->source) {
-    cx->markAtom(lookup->source);
+  if (lookup.source()) {
+    cx->markAtom(lookup.source());
   }
-  if (lookup->functionDisplayName) {
-    cx->markAtom(lookup->functionDisplayName);
+  if (lookup.functionDisplayName()) {
+    cx->markAtom(lookup.functionDisplayName());
   }
-  if (lookup->asyncCause) {
-    cx->markAtom(lookup->asyncCause);
+  if (lookup.asyncCause()) {
+    cx->markAtom(lookup.asyncCause());
   }
 
-  initSource(lookup->source);
-  initSourceId(lookup->sourceId);
-  initLine(lookup->line);
-  initColumn(lookup->column);
-  initFunctionDisplayName(lookup->functionDisplayName);
-  initAsyncCause(lookup->asyncCause);
-  initParent(lookup->parent);
-  initPrincipals(lookup->principals);
+  initSource(lookup.source());
+  initSourceId(lookup.sourceId());
+  initLine(lookup.line());
+  initColumn(lookup.column());
+  initFunctionDisplayName(lookup.functionDisplayName());
+  initAsyncCause(lookup.asyncCause());
+  initParent(lookup.parent());
+  initPrincipals(lookup.principals());
 }
 
 /* static */ SavedFrame* SavedFrame::create(JSContext* cx) {
   RootedGlobalObject global(cx, cx->global());
   cx->check(global);
 
   // Ensure that we don't try to capture the stack again in the
   // `SavedStacksMetadataBuilder` for this new SavedFrame object, and
@@ -1309,17 +1327,17 @@ bool SavedStacks::insertFrames(JSContext
   // To avoid making many copies of FrameIter (whose copy constructor is
   // relatively slow), we use a vector of `SavedFrame::Lookup` objects, which
   // only contain the FrameIter data we need. The `SavedFrame::Lookup`
   // objects are partially initialized with everything except their parent
   // pointers on the first pass, and then we fill in the parent pointers as we
   // return in the second pass.
 
   // Accumulate the vector of Lookup objects here, youngest to oldest.
-  SavedFrame::AutoLookupVector stackChain(cx);
+  Rooted<js::GCLookupVector> stackChain(cx, js::GCLookupVector(cx));
 
   // If we find an async parent or a cached saved frame, then that supplies
   // the parent of the frames we have placed in stackChain. If we walk the
   // stack all the way to the end, this remains null.
   RootedSavedFrame parent(cx, nullptr);
 
   // Choose the right frame iteration strategy to accomodate both
   // evalInFramePrev links and the LiveSavedFrameCache. For background, see
@@ -1387,17 +1405,17 @@ bool SavedStacks::insertFrames(JSContext
       }
     }
 
     RootedAtom displayAtom(cx, iter.maybeFunctionDisplayAtom());
 
     auto principals = iter.realm()->principals();
     MOZ_ASSERT_IF(framePtr && !iter.isWasm(), iter.pc());
 
-    if (!stackChain->emplaceBack(
+    if (!stackChain.emplaceBack(
             location.source(), location.sourceId(),
             location.line(), location.column(), displayAtom,
             nullptr,  // asyncCause
             nullptr,  // parent (not known yet)
             principals, framePtr, iter.pc(), &activation)) {
       ReportOutOfMemory(cx);
       return false;
     }
@@ -1461,39 +1479,39 @@ bool SavedStacks::insertFrames(JSContext
     if (capture.is<JS::MaxFrames>()) {
       capture.as<JS::MaxFrames>().maxFrames--;
     }
   }
 
   // Iterate through |stackChain| in reverse order and get or create the
   // actual SavedFrame instances.
   frame.set(parent);
-  for (size_t i = stackChain->length(); i != 0; i--) {
-    SavedFrame::HandleLookup lookup = stackChain[i - 1];
-    lookup->parent = frame;
+  for (size_t i = stackChain.length(); i != 0; i--) {
+    MutableHandle<SavedFrame::Lookup> lookup = stackChain[i - 1];
+    lookup.setParent(frame);
 
     // If necessary, adjust the parent of a debugger eval frame to point to
     // the frame in whose scope the eval occurs - if we're using
     // LiveSavedFrameCache. Otherwise, we simply ask the FrameIter to follow
     // evalInFramePrev links, so that the parent is always the last frame we
     // created.
-    if (capture.is<JS::AllFrames>() && lookup->framePtr) {
+    if (capture.is<JS::AllFrames>() && lookup.framePtr()) {
       if (!checkForEvalInFramePrev(cx, lookup)) {
         return false;
       }
     }
 
     frame.set(getOrCreateSavedFrame(cx, lookup));
     if (!frame) {
       return false;
     }
 
-    if (capture.is<JS::AllFrames>() && lookup->framePtr) {
-      auto* cache = lookup->activation->getLiveSavedFrameCache(cx);
-      if (!cache || !cache->insert(cx, *lookup->framePtr, lookup->pc, frame)) {
+    if (capture.is<JS::AllFrames>() && lookup.framePtr()) {
+      auto* cache = lookup.activation()->getLiveSavedFrameCache(cx);
+      if (!cache || !cache->insert(cx, *lookup.framePtr(), lookup.pc(), frame)) {
         return false;
       }
     }
   }
 
   return true;
 }
 
@@ -1507,60 +1525,60 @@ bool SavedStacks::adoptAsyncStack(JSCont
   // If maxFrameCount is Nothing, the caller asked for an unlimited number of
   // stack frames, but async stacks are not limited by the available stack
   // memory, so we need to set an arbitrary limit when collecting them. We
   // still don't enforce an upper limit if the caller requested more frames.
   size_t maxFrames = maxFrameCount.valueOr(ASYNC_STACK_MAX_FRAME_COUNT);
 
   // Turn the chain of frames starting with asyncStack into a vector of Lookup
   // objects in |stackChain|, youngest to oldest.
-  SavedFrame::AutoLookupVector stackChain(cx);
+  Rooted<js::GCLookupVector> stackChain(cx, js::GCLookupVector(cx));
   SavedFrame* currentSavedFrame = asyncStack;
-  while (currentSavedFrame && stackChain->length() < maxFrames) {
-    if (!stackChain->emplaceBack(*currentSavedFrame)) {
+  while (currentSavedFrame && stackChain.length() < maxFrames) {
+    if (!stackChain.emplaceBack(*currentSavedFrame)) {
       ReportOutOfMemory(cx);
       return false;
     }
 
     currentSavedFrame = currentSavedFrame->getParent();
   }
 
   // Attach the asyncCause to the youngest frame.
-  stackChain[0]->asyncCause = asyncCause;
+  stackChain[0].setAsyncCause(asyncCause);
 
   // If we walked the entire stack, and it's in cx's realm, we don't
   // need to rebuild the full chain again using the lookup objects - we can
   // just use the existing chain. Only the asyncCause on the youngest frame
   // needs to be changed.
   if (currentSavedFrame == nullptr && asyncStack->realm() == cx->realm()) {
-    SavedFrame::HandleLookup lookup = stackChain[0];
-    lookup->parent = asyncStack->getParent();
+    MutableHandle<SavedFrame::Lookup> lookup = stackChain[0];
+    lookup.setParent(asyncStack->getParent());
     asyncStack.set(getOrCreateSavedFrame(cx, lookup));
     return !!asyncStack;
   }
 
   // If we captured the maximum number of frames and the caller requested no
   // specific limit, we only return half of them. This means that if we do
   // many subsequent captures with the same async stack, it's likely we can
   // use the optimization above.
   if (maxFrameCount.isNothing() && currentSavedFrame) {
-    stackChain->shrinkBy(ASYNC_STACK_MAX_FRAME_COUNT / 2);
+    stackChain.shrinkBy(ASYNC_STACK_MAX_FRAME_COUNT / 2);
   }
 
   // Iterate through |stackChain| in reverse order and get or create the
   // actual SavedFrame instances.
   asyncStack.set(nullptr);
-  while (!stackChain->empty()) {
-    SavedFrame::HandleLookup lookup = stackChain.back();
-    lookup->parent = asyncStack;
+  while (!stackChain.empty()) {
+    Rooted<SavedFrame::Lookup> lookup(cx, stackChain.back());
+    lookup.setParent(asyncStack);
     asyncStack.set(getOrCreateSavedFrame(cx, lookup));
     if (!asyncStack) {
       return false;
     }
-    stackChain->popBack();
+    stackChain.popBack();
   }
 
   return true;
 }
 
 // Given a |lookup| for which we're about to construct a SavedFrame, if it
 // refers to a Debugger eval frame, adjust |lookup|'s parent to be the frame's
 // evalInFramePrev target.
@@ -1569,38 +1587,38 @@ bool SavedStacks::adoptAsyncStack(JSCont
 // stack (the 'target' frame). It is our custom to report the target as the
 // immediate parent of the eval frame. The LiveSavedFrameCache requires us not
 // to skip frames, so instead we walk the entire stack, and just give Debugger
 // eval frames the right parents as we encounter them.
 //
 // Call this function only if we are using the LiveSavedFrameCache; otherwise,
 // FrameIter has already taken care of getting us the right parent.
 bool SavedStacks::checkForEvalInFramePrev(JSContext* cx,
-                                          SavedFrame::HandleLookup lookup) {
-  MOZ_ASSERT(lookup->framePtr);
-  if (!lookup->framePtr->isInterpreterFrame()) {
+                                          MutableHandle<SavedFrame::Lookup> lookup) {
+  MOZ_ASSERT(lookup.framePtr());
+  if (!lookup.framePtr()->isInterpreterFrame()) {
     return true;
   }
 
-  InterpreterFrame& interpreterFrame = lookup->framePtr->asInterpreterFrame();
+  InterpreterFrame& interpreterFrame = lookup.framePtr()->asInterpreterFrame();
   if (!interpreterFrame.isDebuggerEvalFrame()) {
     return true;
   }
 
   LiveSavedFrameCache::FramePtr target =
       LiveSavedFrameCache::FramePtr::create(interpreterFrame.evalInFramePrev());
 
   // If we're caching the frame to which |lookup| refers, then we should
   // definitely have the target frame in the cache as well.
   MOZ_ASSERT(target.hasCachedSavedFrame());
 
   // Search the chain of activations for a LiveSavedFrameCache that has an
   // entry for target.
   RootedSavedFrame saved(cx, nullptr);
-  for (Activation* act = lookup->activation; act; act = act->prev()) {
+  for (Activation* act = lookup.activation(); act; act = act->prev()) {
     // It's okay to force allocation of a cache here; we're about to put
     // something in the top cache, and all the lower ones should exist
     // already.
     auto* cache = act->getLiveSavedFrameCache(cx);
     if (!cache) {
       return false;
     }
 
@@ -1608,22 +1626,22 @@ bool SavedStacks::checkForEvalInFramePre
     if (saved) {
       break;
     }
   }
 
   // Since |target| has its cached bit set, we should have found it.
   MOZ_ALWAYS_TRUE(saved);
 
-  lookup->parent = saved;
+  lookup.setParent(saved);
   return true;
 }
 
 SavedFrame* SavedStacks::getOrCreateSavedFrame(
-    JSContext* cx, SavedFrame::HandleLookup lookup) {
+    JSContext* cx, Handle<SavedFrame::Lookup> lookup) {
   const SavedFrame::Lookup& lookupInstance = lookup.get();
   DependentAddPtr<SavedFrame::Set> p(cx, frames, lookupInstance);
   if (p) {
     MOZ_ASSERT(*p);
     return *p;
   }
 
   RootedSavedFrame frame(cx, createFrameFromLookup(cx, lookup));
@@ -1634,17 +1652,17 @@ SavedFrame* SavedStacks::getOrCreateSave
   if (!p.add(cx, frames, lookupInstance, frame)) {
     return nullptr;
   }
 
   return frame;
 }
 
 SavedFrame* SavedStacks::createFrameFromLookup(
-    JSContext* cx, SavedFrame::HandleLookup lookup) {
+    JSContext* cx, Handle<SavedFrame::Lookup> lookup) {
   RootedSavedFrame frame(cx, SavedFrame::create(cx));
   if (!frame) {
     return nullptr;
   }
   frame->initFromLookup(cx, lookup);
 
   if (!FreezeObject(cx, frame)) {
     return nullptr;
@@ -1859,17 +1877,17 @@ struct MOZ_STACK_CLASS AtomizingMatcher 
     MOZ_ASSERT(chars);
     return AtomizeChars(cx, chars, length);
   }
 };
 
 JS_PUBLIC_API bool ConstructSavedFrameStackSlow(
     JSContext* cx, JS::ubi::StackFrame& frame,
     MutableHandleObject outSavedFrameStack) {
-  SavedFrame::AutoLookupVector stackChain(cx);
+  Rooted<js::GCLookupVector> stackChain(cx, js::GCLookupVector(cx));
   Rooted<JS::ubi::StackFrame> ubiFrame(cx, frame);
 
   while (ubiFrame.get()) {
     // Convert the source and functionDisplayName strings to atoms.
 
     js::RootedAtom source(cx);
     AtomizingMatcher atomizer(cx, ubiFrame.get().sourceLength());
     source = ubiFrame.get().source().match(atomizer);
@@ -1886,32 +1904,32 @@ JS_PUBLIC_API bool ConstructSavedFrameSt
       if (!functionDisplayName) {
         return false;
       }
     }
 
     auto principals =
         js::ReconstructedSavedFramePrincipals::getSingleton(ubiFrame.get());
 
-    if (!stackChain->emplaceBack(source, ubiFrame.get().sourceId(),
+    if (!stackChain.emplaceBack(source, ubiFrame.get().sourceId(),
                                  ubiFrame.get().line(),
                                  ubiFrame.get().column(), functionDisplayName,
                                  /* asyncCause */ nullptr,
                                  /* parent */ nullptr, principals)) {
       ReportOutOfMemory(cx);
       return false;
     }
 
     ubiFrame = ubiFrame.get().parent();
   }
 
   js::RootedSavedFrame parentFrame(cx);
-  for (size_t i = stackChain->length(); i != 0; i--) {
-    SavedFrame::HandleLookup lookup = stackChain[i - 1];
-    lookup->parent = parentFrame;
+  for (size_t i = stackChain.length(); i != 0; i--) {
+    MutableHandle<SavedFrame::Lookup> lookup = stackChain[i - 1];
+    lookup.setParent(parentFrame);
     parentFrame = cx->realm()->savedStacks().getOrCreateSavedFrame(cx, lookup);
     if (!parentFrame) {
       return false;
     }
   }
 
   outSavedFrameStack.set(parentFrame);
   return true;
--- a/js/src/vm/SavedStacks.h
+++ b/js/src/vm/SavedStacks.h
@@ -217,21 +217,21 @@ class SavedStacks {
   };
 
   MOZ_MUST_USE bool insertFrames(JSContext* cx, MutableHandleSavedFrame frame,
                                  JS::StackCapture&& capture);
   MOZ_MUST_USE bool adoptAsyncStack(
       JSContext* cx, MutableHandleSavedFrame asyncStack, HandleAtom asyncCause,
       const mozilla::Maybe<size_t>& maxFrameCount);
   MOZ_MUST_USE bool checkForEvalInFramePrev(JSContext* cx,
-                                            SavedFrame::HandleLookup lookup);
+                                            MutableHandle<SavedFrame::Lookup> lookup);
   SavedFrame* getOrCreateSavedFrame(JSContext* cx,
-                                    SavedFrame::HandleLookup lookup);
+                                    Handle<SavedFrame::Lookup> lookup);
   SavedFrame* createFrameFromLookup(JSContext* cx,
-                                    SavedFrame::HandleLookup lookup);
+                                    Handle<SavedFrame::Lookup> lookup);
 
   // Cache for memoizing PCToLineNumber lookups.
 
   struct PCKey {
     PCKey(JSScript* script, jsbytecode* pc) : script(script), pc(pc) {}
 
     HeapPtr<JSScript*> script;
     jsbytecode* pc;