From 464a796d36ec7475fb50428f3386cf40f7078e79 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Fri, 2 Oct 2015 09:42:38 +0100 Subject: [PATCH 1/2] Bug 1177508 - Truncate the stack more aggressively in adoptAsyncStack. r=fitzgen --HG-- extra : commitid : 4uOCJQqPLmI extra : rebase_source : 51d5ce04ff1953fc1e800a5a34faf446da2e3ef6 --- .../saved-stacks/async-max-frame-count.js | 14 +++++-- js/src/vm/SavedStacks.cpp | 41 ++++++++++++++----- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/js/src/jit-test/tests/saved-stacks/async-max-frame-count.js b/js/src/jit-test/tests/saved-stacks/async-max-frame-count.js index 4717897ac3e0..db531f818eec 100644 --- a/js/src/jit-test/tests/saved-stacks/async-max-frame-count.js +++ b/js/src/jit-test/tests/saved-stacks/async-max-frame-count.js @@ -23,10 +23,16 @@ function checkRecursion(n, limit) { } // Async stacks are limited even if we didn't ask for a limit. There is a - // default limit on frames attached on top of any synchronous frames. In this - // case the synchronous frame is the last call to `recur`. + // default limit on frames attached on top of any synchronous frames, and + // every time the limit is reached when capturing, half of the frames are + // truncated from the old end of the async stack. if (limit == 0) { - limit = defaultAsyncStackLimit + 1; + // Always add one synchronous frame that is the last call to `recur`. + if (n + 1 < defaultAsyncStackLimit) { + limit = defaultAsyncStackLimit + 1; + } else { + limit = n + 2 - (defaultAsyncStackLimit / 2); + } } // The first `n` or `limit` frames should have `recur` as their `asyncParent`. @@ -68,6 +74,8 @@ function checkRecursion(n, limit) { checkRecursion(0, 0); checkRecursion(1, 0); checkRecursion(2, 0); +checkRecursion(defaultAsyncStackLimit - 10, 0); +checkRecursion(defaultAsyncStackLimit, 0); checkRecursion(defaultAsyncStackLimit + 10, 0); // Limit of 1 frame. diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp index ad22cdf0be64..1e30548bede5 100644 --- a/js/src/vm/SavedStacks.cpp +++ b/js/src/vm/SavedStacks.cpp @@ -206,7 +206,7 @@ class MOZ_STACK_CLASS SavedFrame::AutoLookupVector : public JS::CustomAutoRooter lookups(cx) { } - typedef Vector LookupVector; + typedef Vector LookupVector; inline LookupVector* operator->() { return &lookups; } inline HandleLookup operator[](size_t i) { return HandleLookup(lookups[i]); } @@ -1141,29 +1141,50 @@ SavedStacks::adoptAsyncStack(JSContext* cx, HandleSavedFrame asyncStack, // 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. - if (maxFrameCount == 0) - maxFrameCount = ASYNC_STACK_MAX_FRAME_COUNT; + unsigned maxFrames = maxFrameCount > 0 ? maxFrameCount : ASYNC_STACK_MAX_FRAME_COUNT; // Accumulate the vector of Lookup objects in |stackChain|. SavedFrame::AutoLookupVector stackChain(cx); SavedFrame* currentSavedFrame = asyncStack; - for (unsigned i = 0; i < maxFrameCount && currentSavedFrame; i++) { + SavedFrame* firstSavedFrameParent = nullptr; + for (unsigned i = 0; i < maxFrames && currentSavedFrame; i++) { if (!stackChain->emplaceBack(*currentSavedFrame)) { ReportOutOfMemory(cx); return false; } - // Attach the asyncCause to the youngest frame. - if (i == 0) - stackChain->back().asyncCause = asyncCauseAtom; - currentSavedFrame = currentSavedFrame->getParent(); + + // Attach the asyncCause to the youngest frame. + if (i == 0) { + stackChain->back().asyncCause = asyncCauseAtom; + firstSavedFrameParent = currentSavedFrame; + } + } + + // This is the 1-based index of the oldest frame we care about. + size_t oldestFramePosition = stackChain->length(); + RootedSavedFrame parentFrame(cx, nullptr); + + if (currentSavedFrame == nullptr && + asyncStack->compartment() == cx->compartment()) { + // If we consumed the full async stack, and the stack is in the same + // compartment as the one requested, we don't need to rebuild the full + // chain again using the lookup objects, we can just reference the + // existing chain and change the asyncCause on the younger frame. + oldestFramePosition = 1; + parentFrame = firstSavedFrameParent; + } else if (maxFrameCount == 0 && + oldestFramePosition == ASYNC_STACK_MAX_FRAME_COUNT) { + // If we captured the maximum number of frames and the caller requested + // no specific limit, we only return half of them. This means that for + // the next iterations, it's likely we can use the optimization above. + oldestFramePosition = ASYNC_STACK_MAX_FRAME_COUNT / 2; } // Iterate through |stackChain| in reverse order and get or create the // actual SavedFrame instances. - RootedSavedFrame parentFrame(cx, nullptr); - for (size_t i = stackChain->length(); i != 0; i--) { + for (size_t i = oldestFramePosition; i != 0; i--) { SavedFrame::HandleLookup lookup = stackChain[i-1]; lookup->parent = parentFrame; parentFrame.set(getOrCreateSavedFrame(cx, lookup)); From 3265dfcbb646dd94d593efff6b4e0a3411203e6e Mon Sep 17 00:00:00 2001 From: Maxim Zhilyaev Date: Sat, 3 Oct 2015 20:32:56 -0700 Subject: [PATCH 2/2] Bug 1202040 - Intermittent browser_newtab_bug1194895.js | This test exceeded the timeout threshold. [r=marcosc] --- browser/base/content/test/newtab/browser_newtab_bug1194895.js | 1 + 1 file changed, 1 insertion(+) diff --git a/browser/base/content/test/newtab/browser_newtab_bug1194895.js b/browser/base/content/test/newtab/browser_newtab_bug1194895.js index d7c2901c76dc..798a28839352 100644 --- a/browser/base/content/test/newtab/browser_newtab_bug1194895.js +++ b/browser/base/content/test/newtab/browser_newtab_bug1194895.js @@ -26,6 +26,7 @@ gDirectorySource = "data:application/json," + JSON.stringify({ function runTests() { + requestLongerTimeout(4); let origEnhanced = NewTabUtils.allPages.enhanced; let origCompareLinks = NewTabUtils.links.compareLinks; registerCleanupFunction(() => {