Bug 1438121: Final Part 3: Make LiveSavedFrameCache::find pop invalid entries as part of the search. r=fitzgen
authorJim Blandy <jimb@mozilla.com>
Thu, 01 Mar 2018 23:14:45 -0800
changeset 462466 449f9d9d664bb47a8f318bba7543c34e2363a4a3
parent 462465 f54675bcf897379cd4f81cad3b80f812b9f2a903
child 462467 bfc321ae89810ce708b0e15d6848a9a822b253f4
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1438121
milestone60.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 1438121: Final Part 3: Make LiveSavedFrameCache::find pop invalid entries as part of the search. r=fitzgen Rather than searching from the beginning (old end) of the cache, LiveSameFrameCache::find can search from the young end of the cache, popping invalid stack entries as it goes. This means that the number of entries searched is related to the number of cached frames popped since the last stack capture, not the total number of entries in the cache. This also removes the need for iterators, iterator arithmetic, or any random access to the stack; the function simply uses the vector's 'back', 'popBack', 'empty', and 'clear' methods. MozReview-Commit-ID: DCFt0uhiqql
js/src/vm/SavedStacks.cpp
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -96,58 +96,69 @@ LiveSavedFrameCache::insert(JSContext* c
 
 void
 LiveSavedFrameCache::find(JSContext* cx, FramePtr& framePtr, const jsbytecode* pc,
                           MutableHandleSavedFrame frame) const
 {
     MOZ_ASSERT(initialized());
     MOZ_ASSERT(framePtr.hasCachedSavedFrame());
 
-    if (frames->empty())
-        // This early return supports the assertion below.
+    // If we flushed the cache due to a compartment mismatch, then we shouldn't
+    // expect to find any frames in the cache.
+    if (frames->empty()) {
+        frame.set(nullptr);
         return;
-
-    Key key(framePtr);
-
-    auto *p = frames->begin();
-    for (; p < frames->end(); p++) {
-        if (key == p->key) {
-            frame.set(p->savedFrame);
-            break;
-        }
     }
 
-    // If the frame's bit was set, the frame should always have an entry in the
-    // cache. (If we purged the entire cache because its SavedFrames had been
-    // captured for a different compartment, then we would have returned early
-    // above.)
-    MOZ_ASSERT(frame);
+    // All our SavedFrames should be in the same compartment. If the last
+    // entry's SavedFrame's compartment doesn't match cx's, flush the cache.
+    if (frames->back().savedFrame->compartment() != cx->compartment()) {
+#ifdef DEBUG
+        // Check that they are, indeed, all in the same compartment.
+        auto compartment = frames->back().savedFrame->compartment();
+        for (const auto& f : (*frames))
+            MOZ_ASSERT(compartment == f.savedFrame->compartment());
+#endif
+        frames->clear();
+        frame.set(nullptr);
+        return;
+    }
 
-    // Now that we have a SavedFrame to look at, check whether its compartment
-    // matches cx's. If our SavedFrames were captured for a different
-    // compartment, purge the whole cache.
-    if (frame->compartment() != cx->compartment()) {
-        frame.set(nullptr);
-        frames->clear();
-        return;
+    Key key(framePtr);
+    while (key != frames->back().key) {
+        MOZ_ASSERT(frames->back().savedFrame->compartment() == cx->compartment());
+
+        // We know that the cache does contain an entry for frameIter's frame,
+        // since its bit is set. That entry must be below this one in the stack,
+        // so frames->back() must correspond to a frame younger than
+        // frameIter's. If frameIter is the youngest frame with its bit set,
+        // then its entry is the youngest that is valid, and we can pop this
+        // entry. Even if frameIter is not the youngest frame with its bit set,
+        // since we're going to push new cache entries for all frames younger
+        // than frameIter, we must pop it anyway.
+        frames->popBack();
+
+        // If the frame's bit was set, the frame should always have an entry in
+        // the cache. (If we purged the entire cache because its SavedFrames had
+        // been captured for a different compartment, then we would have
+        // returned early above.)
+        MOZ_ASSERT(!frames->empty());
     }
 
     // The youngest valid frame may have run some code, so its current pc may
     // not match its cache entry's pc. In this case, just treat it as a miss. No
     // older frame has executed any code; it would have been necessary to pop
     // this frame for that to happen, but this frame's bit is set.
-    if (pc != p->pc) {
+    if (pc != frames->back().pc) {
+        frames->popBack();
         frame.set(nullptr);
-        p--;
+        return;
     }
 
-    // All entries after the one we just matched are stale younger frames that
-    // have since been popped.
-    p++;
-    frames->shrinkBy(frames->end() - p);
+    frame.set(frames->back().savedFrame);
 }
 
 struct SavedFrame::Lookup {
     Lookup(JSAtom* source, uint32_t line, uint32_t column,
            JSAtom* functionDisplayName, JSAtom* asyncCause, SavedFrame* parent,
            JSPrincipals* principals,
            const Maybe<LiveSavedFrameCache::FramePtr>& framePtr = Nothing(),
            jsbytecode* pc = nullptr, Activation* activation = nullptr)