From 0840bb61c6f5291365bd14a2b8892b2300fdfa37 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 May 2017 09:37:28 +1000 Subject: [PATCH 1/4] Bug 1366650 (part 1) - Move PseudoStack into SpiderMonkey. r=mstange,shu. This includes renaming its fields to match SpiderMonkey naming conventions instead of Gecko naming conventions. This patch is just about moving the code. The next patch will change SpiderMonkey to actually use PseudoStack directly. --HG-- extra : rebase_source : 27e77ddf950201eb6bdba60003218056442cf7ab --- js/public/ProfilingStack.h | 66 +++++++++++++++++++- tools/profiler/core/ThreadInfo.h | 6 +- tools/profiler/core/platform.cpp | 25 ++++---- tools/profiler/moz.build | 1 - tools/profiler/public/GeckoProfiler.h | 1 - tools/profiler/public/PseudoStack.h | 88 --------------------------- xpcom/threads/ThreadStackHelper.cpp | 4 +- 7 files changed, 82 insertions(+), 109 deletions(-) delete mode 100644 tools/profiler/public/PseudoStack.h diff --git a/js/public/ProfilingStack.h b/js/public/ProfilingStack.h index 524d5285c8aa..bb78ae685325 100644 --- a/js/public/ProfilingStack.h +++ b/js/public/ProfilingStack.h @@ -7,10 +7,14 @@ #ifndef js_ProfilingStack_h #define js_ProfilingStack_h +#include "mozilla/ArrayUtils.h" + +#include +#include + #include "jsbytecode.h" #include "jstypes.h" #include "js/TypeDecls.h" - #include "js/Utility.h" struct JSRuntime; @@ -205,4 +209,64 @@ RegisterContextProfilingEventMarker(JSContext* cx, void (*fn)(const char*)); } // namespace js +// The PseudoStack members are accessed in parallel by multiple threads: the +// profiler's sampler thread reads these members while other threads modify +// them. +class PseudoStack +{ + public: + PseudoStack() + : stackPointer(0) + {} + + ~PseudoStack() { + // The label macros keep a reference to the PseudoStack to avoid a TLS + // access. If these are somehow not all cleared we will get a + // use-after-free so better to crash now. + MOZ_RELEASE_ASSERT(stackPointer == 0); + } + + void push(const char* label, js::ProfileEntry::Category category, + void* stackAddress, uint32_t line, const char* dynamicString) { + if (size_t(stackPointer) >= mozilla::ArrayLength(entries)) { + stackPointer++; + return; + } + + volatile js::ProfileEntry& entry = entries[int(stackPointer)]; + + entry.initCppFrame(stackAddress, line); + entry.setLabel(label); + entry.setDynamicString(dynamicString); + MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY); + entry.setCategory(category); + + // This must happen at the end! The compiler will not reorder this + // update because stackPointer is Atomic. + stackPointer++; + } + + void pop() { stackPointer--; } + + uint32_t stackSize() const { + return std::min(uint32_t(stackPointer), uint32_t(mozilla::ArrayLength(entries))); + } + + mozilla::Atomic* AddressOfStackPointer() { return &stackPointer; } + + private: + // No copying. + PseudoStack(const PseudoStack&) = delete; + void operator=(const PseudoStack&) = delete; + + public: + // The stack entries. + js::ProfileEntry volatile entries[1024]; + + protected: + // This may exceed the length of |entries|, so instead use the stackSize() + // method to determine the number of valid samples in |entries|. + mozilla::Atomic stackPointer; +}; + #endif /* js_ProfilingStack_h */ diff --git a/tools/profiler/core/ThreadInfo.h b/tools/profiler/core/ThreadInfo.h index f771ca32c1d7..ea319ada1f7d 100644 --- a/tools/profiler/core/ThreadInfo.h +++ b/tools/profiler/core/ThreadInfo.h @@ -12,7 +12,7 @@ #include "platform.h" #include "ProfileBuffer.h" -#include "PseudoStack.h" +#include "js/ProfilingStack.h" // Stub eventMarker function for js-engine event generation. void ProfilerJSEventMarker(const char* aEvent); @@ -232,9 +232,9 @@ public: js::SetContextProfilingStack( aContext, - (js::ProfileEntry*) RacyInfo()->mStack, + (js::ProfileEntry*) RacyInfo()->entries, RacyInfo()->AddressOfStackPointer(), - (uint32_t) mozilla::ArrayLength(RacyInfo()->mStack)); + (uint32_t) mozilla::ArrayLength(RacyInfo()->entries)); PollJSSampling(); } diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index 300b383c1dc2..9d9c91b544e7 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -24,7 +24,6 @@ #include "mozilla/ThreadLocal.h" #include "mozilla/TimeStamp.h" #include "mozilla/StaticPtr.h" -#include "PseudoStack.h" #include "ThreadInfo.h" #include "nsIHttpProtocolHandler.h" #include "nsIObserverService.h" @@ -719,7 +718,7 @@ AddPseudoEntry(PSLockRef aLock, ProfileBuffer* aBuffer, if (!entry.pc()) { // The JIT only allows the top-most entry to have a nullptr pc. MOZ_ASSERT(&entry == - &aRacyInfo->mStack[aRacyInfo->stackSize() - 1]); + &aRacyInfo->entries[aRacyInfo->stackSize() - 1]); } else { lineno = JS_PCToLineNumber(script, entry.pc()); } @@ -779,7 +778,7 @@ MergeStacksIntoProfile(PSLockRef aLock, ProfileBuffer* aBuffer, const TickSample& aSample, NativeStack& aNativeStack) { NotNull racyInfo = aSample.mRacyInfo; - volatile js::ProfileEntry* pseudoFrames = racyInfo->mStack; + volatile js::ProfileEntry* pseudoEntries = racyInfo->entries; uint32_t pseudoCount = racyInfo->stackSize(); JSContext* context = aSample.mJSContext; @@ -854,10 +853,10 @@ MergeStacksIntoProfile(PSLockRef aLock, ProfileBuffer* aBuffer, uint8_t* nativeStackAddr = nullptr; if (pseudoIndex != pseudoCount) { - volatile js::ProfileEntry& pseudoFrame = pseudoFrames[pseudoIndex]; + volatile js::ProfileEntry& pseudoEntry = pseudoEntries[pseudoIndex]; - if (pseudoFrame.isCpp()) { - lastPseudoCppStackAddr = (uint8_t*) pseudoFrame.stackAddress(); + if (pseudoEntry.isCpp()) { + lastPseudoCppStackAddr = (uint8_t*) pseudoEntry.stackAddress(); } // Skip any pseudo-stack JS frames which are marked isOSR. Pseudostack @@ -866,7 +865,7 @@ MergeStacksIntoProfile(PSLockRef aLock, ProfileBuffer* aBuffer, // pseudoframe and jit frame being recorded (and showing up twice), the // interpreter marks the interpreter pseudostack entry with the OSR flag // to ensure that it doesn't get counted. - if (pseudoFrame.isJs() && pseudoFrame.isOSR()) { + if (pseudoEntry.isJs() && pseudoEntry.isOSR()) { pseudoIndex++; continue; } @@ -905,8 +904,8 @@ MergeStacksIntoProfile(PSLockRef aLock, ProfileBuffer* aBuffer, // Check to see if pseudoStack frame is top-most. if (pseudoStackAddr > jsStackAddr && pseudoStackAddr > nativeStackAddr) { MOZ_ASSERT(pseudoIndex < pseudoCount); - volatile js::ProfileEntry& pseudoFrame = pseudoFrames[pseudoIndex]; - AddPseudoEntry(aLock, aBuffer, pseudoFrame, racyInfo); + volatile js::ProfileEntry& pseudoEntry = pseudoEntries[pseudoIndex]; + AddPseudoEntry(aLock, aBuffer, pseudoEntry, racyInfo); pseudoIndex++; continue; } @@ -1048,7 +1047,7 @@ DoNativeBacktrace(PSLockRef aLock, ProfileBuffer* aBuffer, for (uint32_t i = racyInfo->stackSize(); i > 0; --i) { // The pseudostack grows towards higher indices, so we iterate // backwards (from callee to caller). - volatile js::ProfileEntry& entry = racyInfo->mStack[i - 1]; + volatile js::ProfileEntry& entry = racyInfo->entries[i - 1]; if (!entry.isJs() && strcmp(entry.label(), "EnterJIT") == 0) { // Found JIT entry frame. Unwind up to that point (i.e., force // the stack walk to stop before the block of saved registers; @@ -2815,13 +2814,13 @@ profiler_get_backtrace_noalloc(char *output, size_t outputSize) bool includeDynamicString = !ActivePS::FeaturePrivacy(lock); - volatile js::ProfileEntry* pseudoFrames = pseudoStack->mStack; + volatile js::ProfileEntry* pseudoEntries = pseudoStack->entries; uint32_t pseudoCount = pseudoStack->stackSize(); for (uint32_t i = 0; i < pseudoCount; i++) { - const char* label = pseudoFrames[i].label(); + const char* label = pseudoEntries[i].label(); const char* dynamicString = - includeDynamicString ? pseudoFrames[i].dynamicString() : nullptr; + includeDynamicString ? pseudoEntries[i].dynamicString() : nullptr; size_t labelLength = strlen(label); if (dynamicString) { // Put the label, maybe a space, and the dynamic string into output. diff --git a/tools/profiler/moz.build b/tools/profiler/moz.build index 0969a5b1f883..1e79fbf74b06 100644 --- a/tools/profiler/moz.build +++ b/tools/profiler/moz.build @@ -12,7 +12,6 @@ if CONFIG['MOZ_GECKO_PROFILER']: EXPORTS += [ 'public/CrossProcessProfilerController.h', 'public/ProfilerMarkerPayload.h', - 'public/PseudoStack.h', 'public/shared-libraries.h', ] UNIFIED_SOURCES += [ diff --git a/tools/profiler/public/GeckoProfiler.h b/tools/profiler/public/GeckoProfiler.h index 93b302205c8e..680b8fdaa3dc 100644 --- a/tools/profiler/public/GeckoProfiler.h +++ b/tools/profiler/public/GeckoProfiler.h @@ -391,7 +391,6 @@ PROFILER_FUNC(void* profiler_get_stack_top(), nullptr) #include "js/ProfilingStack.h" #include "mozilla/Sprintf.h" #include "mozilla/ThreadLocal.h" -#include "PseudoStack.h" #include "nscore.h" // Make sure that we can use std::min here without the Windows headers messing with us. diff --git a/tools/profiler/public/PseudoStack.h b/tools/profiler/public/PseudoStack.h deleted file mode 100644 index bb700dad1d96..000000000000 --- a/tools/profiler/public/PseudoStack.h +++ /dev/null @@ -1,88 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef PseudoStack_h -#define PseudoStack_h - -#include "mozilla/ArrayUtils.h" -#include "js/ProfilingStack.h" -#include "nsISupportsImpl.h" // for MOZ_COUNT_{CTOR,DTOR} - -#include -#include - -#include - -// The PseudoStack members are read by signal handlers, so the mutation of them -// needs to be signal-safe. -class PseudoStack -{ -public: - PseudoStack() - : mStackPointer(0) - { - MOZ_COUNT_CTOR(PseudoStack); - } - - ~PseudoStack() - { - MOZ_COUNT_DTOR(PseudoStack); - - // The label macros keep a reference to the PseudoStack to avoid a TLS - // access. If these are somehow not all cleared we will get a - // use-after-free so better to crash now. - MOZ_RELEASE_ASSERT(mStackPointer == 0); - } - - void push(const char* aName, js::ProfileEntry::Category aCategory, - void* aStackAddress, uint32_t line, const char* aDynamicString) - { - if (size_t(mStackPointer) >= mozilla::ArrayLength(mStack)) { - mStackPointer++; - return; - } - - volatile js::ProfileEntry& entry = mStack[int(mStackPointer)]; - - // Make sure we increment the pointer after the name has been written such - // that mStack is always consistent. - entry.initCppFrame(aStackAddress, line); - entry.setLabel(aName); - entry.setDynamicString(aDynamicString); - MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY); - entry.setCategory(aCategory); - - // This must happen at the end! The compiler will not reorder this update - // because mStackPointer is Atomic. - mStackPointer++; - } - - // Pop the stack. - void pop() { mStackPointer--; } - - uint32_t stackSize() const - { - return std::min(uint32_t(mStackPointer), uint32_t(mozilla::ArrayLength(mStack))); - } - - mozilla::Atomic* AddressOfStackPointer() { return &mStackPointer; } - -private: - // No copying. - PseudoStack(const PseudoStack&) = delete; - void operator=(const PseudoStack&) = delete; - -public: - // The list of active checkpoints. - js::ProfileEntry volatile mStack[1024]; - -protected: - // This may exceed the length of mStack, so instead use the stackSize() method - // to determine the number of valid samples in mStack. - mozilla::Atomic mStackPointer; -}; - -#endif // PseudoStack_h diff --git a/xpcom/threads/ThreadStackHelper.cpp b/xpcom/threads/ThreadStackHelper.cpp index 97141724c7c6..c3c6cf9ee770 100644 --- a/xpcom/threads/ThreadStackHelper.cpp +++ b/xpcom/threads/ThreadStackHelper.cpp @@ -13,7 +13,7 @@ #include "shared-libraries.h" #endif #ifdef MOZ_THREADSTACKHELPER_PSEUDO -#include "PseudoStack.h" +#include "js/ProfilingStack.h" #endif #include "mozilla/Assertions.h" @@ -492,7 +492,7 @@ ThreadStackHelper::FillStackBuffer() intptr_t availableBufferSize = intptr_t(reservedBufferSize); // Go from front to back - const volatile js::ProfileEntry* entry = mPseudoStack->mStack; + const volatile js::ProfileEntry* entry = mPseudoStack->entries; const volatile js::ProfileEntry* end = entry + mPseudoStack->stackSize(); // Deduplicate identical, consecutive frames const char* prevLabel = nullptr; From a062b9be51e649a0b150e02d41c107a392c9c9e6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 May 2017 09:51:31 +1000 Subject: [PATCH 2/4] Bug 1366650 (part 2) - In GeckoProfiler, do all pseudo-stack accesses via the PseudoStack class, instead of via raw array manipulation. r=mstange,shu. - The profiler gives the JS engine a reference to the pseudo-stack via SetContextProfiilngStack(). That function now takes a |PseudoStack*| instead of a |ProfileEntry*| and pointer to the stack pointer. - PseudoStack now has a |kMaxEntries| field, which is easier to work with than |mozilla::ArrayLength(entries)|. - AddressOfStackPointer() is no longer needed. - The patch also neatens up the push operations significantly. PseudoStack now has pushCppFrame() and pushJsFrame(), which nicely encapsulate the two main cases. These delegate to the updated initCppFrame() and initJsFrame() functions in ProfileEntry. - Renames max_stck in testProfileStrings.cpp as peakStackPointer, which is a clearer name. - Removes a couple of checks from testProfileStrings.cpp. These checks made sense when the pseudo-stack was accessed via raw manipulation, but are not applicable now because we can't artificially limit the maximum stack size so easily. --- js/public/ProfilingStack.h | 89 +++++----- js/src/jsapi-tests/testProfileStrings.cpp | 76 +++------ js/src/shell/js.cpp | 9 +- js/src/shell/jsshell.h | 4 +- js/src/vm/GeckoProfiler.cpp | 193 +++++++++------------- js/src/vm/GeckoProfiler.h | 48 ++---- tools/profiler/core/ThreadInfo.h | 9 +- tools/profiler/public/GeckoProfiler.h | 4 +- 8 files changed, 182 insertions(+), 250 deletions(-) diff --git a/js/public/ProfilingStack.h b/js/public/ProfilingStack.h index bb78ae685325..b68cd41910a1 100644 --- a/js/public/ProfilingStack.h +++ b/js/public/ProfilingStack.h @@ -7,8 +7,6 @@ #ifndef js_ProfilingStack_h #define js_ProfilingStack_h -#include "mozilla/ArrayUtils.h" - #include #include @@ -20,6 +18,8 @@ struct JSRuntime; class JSTracer; +class PseudoStack; + namespace js { // A call stack can be specified to the JS engine such that all JS entry/exits @@ -58,6 +58,8 @@ class ProfileEntry // General purpose storage describing this frame. uint32_t volatile flags_; + static int32_t pcToOffset(JSScript* aScript, jsbytecode* aPc); + public: // These traits are bit masks. Make sure they're powers of 2. enum Flags : uint32_t { @@ -112,18 +114,27 @@ class ProfileEntry void setLabel(const char* aLabel) volatile { label_ = aLabel; } const char* label() const volatile { return label_; } - void setDynamicString(const char* aDynamicString) volatile { dynamicString_ = aDynamicString; } const char* dynamicString() const volatile { return dynamicString_; } - void initJsFrame(JSScript* aScript, jsbytecode* aPc) volatile { - flags_ = 0; - spOrScript = aScript; - setPC(aPc); - } - void initCppFrame(void* aSp, uint32_t aLine) volatile { - flags_ = IS_CPP_ENTRY; - spOrScript = aSp; + void initCppFrame(const char* aLabel, const char* aDynamicString, void* sp, uint32_t aLine, + js::ProfileEntry::Flags aFlags, js::ProfileEntry::Category aCategory) + volatile + { + label_ = aLabel; + dynamicString_ = aDynamicString; + spOrScript = sp; lineOrPcOffset = static_cast(aLine); + flags_ = aFlags | js::ProfileEntry::IS_CPP_ENTRY | uint32_t(aCategory); + } + + void initJsFrame(const char* aLabel, const char* aDynamicString, JSScript* aScript, + jsbytecode* aPc) volatile + { + label_ = aLabel; + dynamicString_ = aDynamicString; + spOrScript = aScript; + lineOrPcOffset = pcToOffset(aScript, aPc); + flags_ = uint32_t(js::ProfileEntry::Category::JS); // No flags, just the JS category. } void setFlag(uint32_t flag) volatile { @@ -149,7 +160,7 @@ class ProfileEntry MOZ_ASSERT(c >= Category::FIRST); MOZ_ASSERT(c <= Category::LAST); flags_ &= ~CATEGORY_MASK; - setFlag(static_cast(c)); + setFlag(uint32_t(c)); } void setOSR() volatile { @@ -184,7 +195,7 @@ class ProfileEntry JS_FRIEND_API(jsbytecode*) pc() const volatile; JS_FRIEND_API(void) setPC(jsbytecode* pc) volatile; - void trace(JSTracer* trc); + void trace(JSTracer* trc) volatile; // The offset of a pc into a script's code can actually be 0, so to // signify a nullptr pc, use a -1 index. This is checked against in @@ -198,8 +209,7 @@ class ProfileEntry }; JS_FRIEND_API(void) -SetContextProfilingStack(JSContext* cx, ProfileEntry* stack, mozilla::Atomic* size, - uint32_t max); +SetContextProfilingStack(JSContext* cx, PseudoStack* pseudoStack); JS_FRIEND_API(void) EnableContextProfilingStack(JSContext* cx, bool enabled); @@ -226,33 +236,35 @@ class PseudoStack MOZ_RELEASE_ASSERT(stackPointer == 0); } - void push(const char* label, js::ProfileEntry::Category category, - void* stackAddress, uint32_t line, const char* dynamicString) { - if (size_t(stackPointer) >= mozilla::ArrayLength(entries)) { - stackPointer++; - return; + void pushCppFrame(const char* label, const char* dynamicString, void* sp, uint32_t line, + js::ProfileEntry::Category category, + js::ProfileEntry::Flags flags = js::ProfileEntry::Flags(0)) { + if (stackPointer < MaxEntries) { + entries[stackPointer].initCppFrame(label, dynamicString, sp, line, flags, category); } - volatile js::ProfileEntry& entry = entries[int(stackPointer)]; - - entry.initCppFrame(stackAddress, line); - entry.setLabel(label); - entry.setDynamicString(dynamicString); - MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY); - entry.setCategory(category); - // This must happen at the end! The compiler will not reorder this // update because stackPointer is Atomic. stackPointer++; } - void pop() { stackPointer--; } + void pushJsFrame(const char* label, const char* dynamicString, JSScript* script, + jsbytecode* pc) { + if (stackPointer < MaxEntries) { + entries[stackPointer].initJsFrame(label, dynamicString, script, pc); + } - uint32_t stackSize() const { - return std::min(uint32_t(stackPointer), uint32_t(mozilla::ArrayLength(entries))); + // This must happen at the end! The compiler will not reorder this + // update because stackPointer is Atomic. + stackPointer++; } - mozilla::Atomic* AddressOfStackPointer() { return &stackPointer; } + void pop() { + MOZ_ASSERT(stackPointer > 0); + stackPointer--; + } + + uint32_t stackSize() const { return std::min(uint32_t(stackPointer), uint32_t(MaxEntries)); } private: // No copying. @@ -260,12 +272,15 @@ class PseudoStack void operator=(const PseudoStack&) = delete; public: - // The stack entries. - js::ProfileEntry volatile entries[1024]; + static const uint32_t MaxEntries = 1024; - protected: - // This may exceed the length of |entries|, so instead use the stackSize() - // method to determine the number of valid samples in |entries|. + // The stack entries. + js::ProfileEntry volatile entries[MaxEntries]; + + // This may exceed MaxEntries, so instead use the stackSize() method to + // determine the number of valid samples in entries. When this is less + // than MaxEntries, it refers to the first free entry past the top of the + // in-use stack (i.e. entries[stackPointer - 1] is the top stack entry). mozilla::Atomic stackPointer; }; diff --git a/js/src/jsapi-tests/testProfileStrings.cpp b/js/src/jsapi-tests/testProfileStrings.cpp index 542dc3775835..c9928cefda21 100644 --- a/js/src/jsapi-tests/testProfileStrings.cpp +++ b/js/src/jsapi-tests/testProfileStrings.cpp @@ -13,15 +13,13 @@ #include "jsapi-tests/tests.h" -static js::ProfileEntry pstack[10]; -static mozilla::Atomic psize; -static uint32_t max_stack = 0; +static PseudoStack pseudoStack; +static uint32_t peakStackPointer = 0; static void reset(JSContext* cx) { - psize = max_stack = 0; - memset(pstack, 0, sizeof(pstack)); + pseudoStack.stackPointer = 0; cx->runtime()->geckoProfiler().stringsReset(); cx->runtime()->geckoProfiler().enableSlowAssertions(true); js::EnableContextProfilingStack(cx, true); @@ -34,7 +32,7 @@ static const JSClass ptestClass = { static bool test_fn(JSContext* cx, unsigned argc, JS::Value* vp) { - max_stack = psize; + peakStackPointer = pseudoStack.stackPointer; return true; } @@ -82,7 +80,7 @@ static const JSFunctionSpec ptestFunctions[] = { static JSObject* initialize(JSContext* cx) { - js::SetContextProfilingStack(cx, pstack, &psize, 10); + js::SetContextProfilingStack(cx, &pseudoStack); JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx)); return JS_InitClass(cx, global, nullptr, &ptestClass, Prof, 0, nullptr, ptestFunctions, nullptr, nullptr); @@ -108,15 +106,15 @@ BEGIN_TEST(testProfileStrings_isCalledWithInterpreter) /* Make sure the stack resets and we have an entry for each stack */ CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), &rval)); - CHECK(psize == 0); - CHECK(max_stack >= 8); + CHECK(pseudoStack.stackPointer == 0); + CHECK(peakStackPointer >= 8); CHECK(cx->runtime()->geckoProfiler().stringsCount() == 8); /* Make sure the stack resets and we added no new entries */ - max_stack = 0; + peakStackPointer = 0; CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), &rval)); - CHECK(psize == 0); - CHECK(max_stack >= 8); + CHECK(pseudoStack.stackPointer == 0); + CHECK(peakStackPointer >= 8); CHECK(cx->runtime()->geckoProfiler().stringsCount() == 8); } reset(cx); @@ -125,20 +123,8 @@ BEGIN_TEST(testProfileStrings_isCalledWithInterpreter) CHECK(JS_CallFunctionName(cx, global, "check2", JS::HandleValueArray::empty(), &rval)); CHECK(cx->runtime()->geckoProfiler().stringsCount() == 5); - CHECK(max_stack >= 6); - CHECK(psize == 0); - } - js::EnableContextProfilingStack(cx, false); - js::SetContextProfilingStack(cx, pstack, &psize, 3); - reset(cx); - { - JS::RootedValue rval(cx); - pstack[3].setLabel((char*) 1234); - CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), - &rval)); - CHECK((size_t) pstack[3].label() == 1234); - CHECK(max_stack >= 8); - CHECK(psize == 0); + CHECK(peakStackPointer >= 6); + CHECK(pseudoStack.stackPointer == 0); } return true; } @@ -166,32 +152,19 @@ BEGIN_TEST(testProfileStrings_isCalledWithJIT) /* Make sure the stack resets and we have an entry for each stack */ CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), &rval)); - CHECK(psize == 0); - CHECK(max_stack >= 8); + CHECK(pseudoStack.stackPointer == 0); + CHECK(peakStackPointer >= 8); /* Make sure the stack resets and we added no new entries */ uint32_t cnt = cx->runtime()->geckoProfiler().stringsCount(); - max_stack = 0; + peakStackPointer = 0; CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), &rval)); - CHECK(psize == 0); + CHECK(pseudoStack.stackPointer == 0); CHECK(cx->runtime()->geckoProfiler().stringsCount() == cnt); - CHECK(max_stack >= 8); + CHECK(peakStackPointer >= 8); } - js::EnableContextProfilingStack(cx, false); - js::SetContextProfilingStack(cx, pstack, &psize, 3); - reset(cx); - { - /* Limit the size of the stack and make sure we don't overflow */ - JS::RootedValue rval(cx); - pstack[3].setLabel((char*) 1234); - CHECK(JS_CallFunctionName(cx, global, "check", JS::HandleValueArray::empty(), - &rval)); - CHECK(psize == 0); - CHECK(max_stack >= 8); - CHECK((size_t) pstack[3].label() == 1234); - } return true; } END_TEST(testProfileStrings_isCalledWithJIT) @@ -211,11 +184,12 @@ BEGIN_TEST(testProfileStrings_isCalledWhenError) bool ok = JS_CallFunctionName(cx, global, "check2", JS::HandleValueArray::empty(), &rval); CHECK(!ok); - CHECK(psize == 0); + CHECK(pseudoStack.stackPointer == 0); CHECK(cx->runtime()->geckoProfiler().stringsCount() == 1); JS_ClearPendingException(cx); } + return true; } END_TEST(testProfileStrings_isCalledWhenError) @@ -234,8 +208,8 @@ BEGIN_TEST(testProfileStrings_worksWhenEnabledOnTheFly) /* enable it in the middle of JS and make sure things check out */ JS::RootedValue rval(cx); JS_CallFunctionName(cx, global, "a", JS::HandleValueArray::empty(), &rval); - CHECK(psize == 0); - CHECK(max_stack >= 1); + CHECK(pseudoStack.stackPointer == 0); + CHECK(peakStackPointer >= 1); CHECK(cx->runtime()->geckoProfiler().stringsCount() == 1); } @@ -246,7 +220,7 @@ BEGIN_TEST(testProfileStrings_worksWhenEnabledOnTheFly) /* now disable in the middle of js */ JS::RootedValue rval(cx); JS_CallFunctionName(cx, global, "c", JS::HandleValueArray::empty(), &rval); - CHECK(psize == 0); + CHECK(pseudoStack.stackPointer == 0); } EXEC("function e() { var p = new Prof(); d(p); p.enable(); b(p); }"); @@ -255,8 +229,8 @@ BEGIN_TEST(testProfileStrings_worksWhenEnabledOnTheFly) /* now disable in the middle of js, but re-enable before final exit */ JS::RootedValue rval(cx); JS_CallFunctionName(cx, global, "e", JS::HandleValueArray::empty(), &rval); - CHECK(psize == 0); - CHECK(max_stack >= 3); + CHECK(pseudoStack.stackPointer == 0); + CHECK(peakStackPointer >= 3); } EXEC("function h() { }"); @@ -269,7 +243,7 @@ BEGIN_TEST(testProfileStrings_worksWhenEnabledOnTheFly) /* disable, and make sure that if we try to re-enter the JIT the pop * will still happen */ JS_CallFunctionName(cx, global, "f", JS::HandleValueArray::empty(), &rval); - CHECK(psize == 0); + CHECK(pseudoStack.stackPointer == 0); } return true; } diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index a55276d7bc37..4f752ffabe13 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -387,8 +387,7 @@ ShellContext::ShellContext(JSContext* cx) quitting(false), readLineBufPos(0), errFilePtr(nullptr), - outFilePtr(nullptr), - geckoProfilingStackSize(0) + outFilePtr(nullptr) {} ShellContext* @@ -5217,8 +5216,7 @@ EnableGeckoProfiling(JSContext* cx, unsigned argc, Value* vp) if (cx->runtime()->geckoProfiler().installed()) MOZ_ALWAYS_TRUE(cx->runtime()->geckoProfiler().enable(false)); - SetContextProfilingStack(cx, sc->geckoProfilingStack, &sc->geckoProfilingStackSize, - ShellContext::GeckoProfilingMaxStackSize); + SetContextProfilingStack(cx, &sc->geckoProfilingStack); cx->runtime()->geckoProfiler().enableSlowAssertions(false); if (!cx->runtime()->geckoProfiler().enable(true)) JS_ReportErrorASCII(cx, "Cannot ensure single threaded execution in profiler"); @@ -5250,8 +5248,7 @@ EnableGeckoProfilingWithSlowAssertions(JSContext* cx, unsigned argc, Value* vp) if (cx->runtime()->geckoProfiler().installed()) MOZ_ALWAYS_TRUE(cx->runtime()->geckoProfiler().enable(false)); - SetContextProfilingStack(cx, sc->geckoProfilingStack, &sc->geckoProfilingStackSize, - ShellContext::GeckoProfilingMaxStackSize); + SetContextProfilingStack(cx, &sc->geckoProfilingStack); cx->runtime()->geckoProfiler().enableSlowAssertions(true); if (!cx->runtime()->geckoProfiler().enable(true)) JS_ReportErrorASCII(cx, "Cannot ensure single threaded execution in profiler"); diff --git a/js/src/shell/jsshell.h b/js/src/shell/jsshell.h index 53ac44a35ab9..b4ceedd021e9 100644 --- a/js/src/shell/jsshell.h +++ b/js/src/shell/jsshell.h @@ -203,9 +203,7 @@ struct ShellContext js::shell::RCFile** errFilePtr; js::shell::RCFile** outFilePtr; - static const uint32_t GeckoProfilingMaxStackSize = 1000; - js::ProfileEntry geckoProfilingStack[GeckoProfilingMaxStackSize]; - mozilla::Atomic geckoProfilingStackSize; + PseudoStack geckoProfilingStack; OffThreadState offThreadState; diff --git a/js/src/vm/GeckoProfiler.cpp b/js/src/vm/GeckoProfiler.cpp index 043d5d5b0f7d..10ad54dcc4c5 100644 --- a/js/src/vm/GeckoProfiler.cpp +++ b/js/src/vm/GeckoProfiler.cpp @@ -28,9 +28,7 @@ using mozilla::DebugOnly; GeckoProfiler::GeckoProfiler(JSRuntime* rt) : rt(rt), strings(mutexid::GeckoProfilerStrings), - stack_(nullptr), - size_(nullptr), - max_(0), + pseudoStack_(nullptr), slowAssertions(false), enabled_(false), eventMarker_(nullptr) @@ -49,14 +47,12 @@ GeckoProfiler::init() } void -GeckoProfiler::setProfilingStack(ProfileEntry* stack, mozilla::Atomic* size, uint32_t max) +GeckoProfiler::setProfilingStack(PseudoStack* pseudoStack) { - MOZ_ASSERT_IF(size_ && *size_ != 0, !enabled()); + MOZ_ASSERT_IF(pseudoStack_, !enabled()); MOZ_ASSERT(strings.lock()->initialized()); - stack_ = stack; - size_ = size; - max_ = max; + pseudoStack_ = pseudoStack; } void @@ -210,106 +206,55 @@ GeckoProfiler::enter(JSContext* cx, JSScript* script, JSFunction* maybeFun) // In debug builds, assert the JS pseudo frames already on the stack // have a non-null pc. Only look at the top frames to avoid quadratic // behavior. - if (*size_ > 0 && *size_ - 1 < max_) { - size_t start = (*size_ > 4) ? *size_ - 4 : 0; - for (size_t i = start; i < *size_ - 1; i++) - MOZ_ASSERT_IF(stack_[i].isJs(), stack_[i].pc() != nullptr); + uint32_t sp = pseudoStack_->stackPointer; + if (sp > 0 && sp - 1 < PseudoStack::MaxEntries) { + size_t start = (sp > 4) ? sp - 4 : 0; + for (size_t i = start; i < sp - 1; i++) + MOZ_ASSERT_IF(pseudoStack_->entries[i].isJs(), pseudoStack_->entries[i].pc()); } #endif - push("", dynamicString, /* sp = */ nullptr, script, script->code()); + pseudoStack_->pushJsFrame("", dynamicString, script, script->code()); return true; } void GeckoProfiler::exit(JSScript* script, JSFunction* maybeFun) { - pop(); + pseudoStack_->pop(); #ifdef DEBUG /* Sanity check to make sure push/pop balanced */ - if (*size_ < max_) { + uint32_t sp = pseudoStack_->stackPointer; + if (sp < PseudoStack::MaxEntries) { const char* dynamicString = profileString(script, maybeFun); /* Can't fail lookup because we should already be in the set */ - MOZ_ASSERT(dynamicString != nullptr); + MOZ_ASSERT(dynamicString); // Bug 822041 - if (!stack_[*size_].isJs()) { + if (!pseudoStack_->entries[sp].isJs()) { fprintf(stderr, "--- ABOUT TO FAIL ASSERTION ---\n"); - fprintf(stderr, " stack=%p size=%d/%d\n", (void*) stack_, size(), max_); - for (int32_t i = *size_; i >= 0; i--) { - if (stack_[i].isJs()) - fprintf(stderr, " [%d] JS %s\n", i, stack_[i].dynamicString()); + fprintf(stderr, " entries=%p size=%u/%u\n", + (void*) pseudoStack_->entries, + uint32_t(pseudoStack_->stackPointer), + PseudoStack::MaxEntries); + for (int32_t i = sp; i >= 0; i--) { + volatile ProfileEntry& entry = pseudoStack_->entries[i]; + if (entry.isJs()) + fprintf(stderr, " [%d] JS %s\n", i, entry.dynamicString()); else - fprintf(stderr, " [%d] C line %d %s\n", i, stack_[i].line(), stack_[i].dynamicString()); + fprintf(stderr, " [%d] C line %d %s\n", i, entry.line(), entry.dynamicString()); } } - MOZ_ASSERT(stack_[*size_].isJs()); - MOZ_ASSERT(stack_[*size_].script() == script); - MOZ_ASSERT(strcmp((const char*) stack_[*size_].dynamicString(), dynamicString) == 0); - stack_[*size_].setDynamicString(nullptr); - stack_[*size_].setPC(nullptr); + volatile ProfileEntry& entry = pseudoStack_->entries[sp]; + MOZ_ASSERT(entry.isJs()); + MOZ_ASSERT(entry.script() == script); + MOZ_ASSERT(strcmp((const char*) entry.dynamicString(), dynamicString) == 0); } #endif } -void -GeckoProfiler::beginPseudoJS(const char* label, void* sp) -{ - /* these operations cannot be re-ordered, so volatile-ize operations */ - volatile ProfileEntry* stack = stack_; - uint32_t current = *size_; - - MOZ_ASSERT(installed()); - if (current < max_) { - stack[current].setLabel(label); - stack[current].initCppFrame(sp, 0); - stack[current].setFlag(ProfileEntry::BEGIN_PSEUDO_JS); - } - *size_ = current + 1; -} - -void -GeckoProfiler::push(const char* label, const char* dynamicString, void* sp, JSScript* script, - jsbytecode* pc, ProfileEntry::Category category) -{ - MOZ_ASSERT(label[0] == '\0' || !dynamicString); - MOZ_ASSERT_IF(sp != nullptr, script == nullptr && pc == nullptr); - MOZ_ASSERT_IF(sp == nullptr, script != nullptr && pc != nullptr); - - /* these operations cannot be re-ordered, so volatile-ize operations */ - volatile ProfileEntry* stack = stack_; - uint32_t current = *size_; - - MOZ_ASSERT(installed()); - if (current < max_) { - volatile ProfileEntry& entry = stack[current]; - - if (sp != nullptr) { - entry.initCppFrame(sp, 0); - MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY); - } - else { - entry.initJsFrame(script, pc); - MOZ_ASSERT(entry.flags() == 0); - } - - entry.setLabel(label); - entry.setDynamicString(dynamicString); - entry.setCategory(category); - } - *size_ = current + 1; -} - -void -GeckoProfiler::pop() -{ - MOZ_ASSERT(installed()); - MOZ_ASSERT(*size_ > 0); - (*size_)--; -} - /* * Serializes the script/function pair into a "descriptive string" which is * allowed to fail. This function cannot trigger a GC because it could finalize @@ -365,12 +310,12 @@ GeckoProfiler::allocProfileString(JSScript* script, JSFunction* maybeFun) } void -GeckoProfiler::trace(JSTracer* trc) +GeckoProfiler::trace(JSTracer* trc) volatile { - if (stack_) { - size_t limit = Min(uint32_t(*size_), max_); - for (size_t i = 0; i < limit; i++) - stack_[i].trace(trc); + if (pseudoStack_) { + size_t size = pseudoStack_->stackSize(); + for (size_t i = 0; i < size; i++) + pseudoStack_->entries[i].trace(trc); } } @@ -408,7 +353,7 @@ GeckoProfiler::checkStringsMapAfterMovingGC() #endif void -ProfileEntry::trace(JSTracer* trc) +ProfileEntry::trace(JSTracer* trc) volatile { if (isJs()) { JSScript* s = rawScript(); @@ -427,11 +372,15 @@ GeckoProfilerEntryMarker::GeckoProfilerEntryMarker(JSRuntime* rt, profiler = nullptr; return; } - size_before = *profiler->size_; + spBefore_ = profiler->stackPointer(); + // We want to push a CPP frame so the profiler can correctly order JS and native stacks. - profiler->beginPseudoJS("js::RunScript", this); - profiler->push("js::RunScript", /* dynamicString = */ nullptr, /* sp = */ nullptr, script, - script->code()); + profiler->pseudoStack_->pushCppFrame( + "js::RunScript", /* dynamicString = */ nullptr, /* sp = */ this, /* line = */ 0, + js::ProfileEntry::Category::OTHER, js::ProfileEntry::BEGIN_PSEUDO_JS); + + profiler->pseudoStack_->pushJsFrame( + "js::RunScript", /* dynamicString = */ nullptr, script, script->code()); } GeckoProfilerEntryMarker::~GeckoProfilerEntryMarker() @@ -439,9 +388,9 @@ GeckoProfilerEntryMarker::~GeckoProfilerEntryMarker() if (profiler == nullptr) return; - profiler->pop(); - profiler->endPseudoJS(); - MOZ_ASSERT(size_before == *profiler->size_); + profiler->pseudoStack_->pop(); // the JS frame + profiler->pseudoStack_->pop(); // the BEGIN_PSEUDO_JS frame + MOZ_ASSERT(spBefore_ == profiler->stackPointer()); } AutoGeckoProfilerEntry::AutoGeckoProfilerEntry(JSRuntime* rt, const char* label, @@ -454,10 +403,14 @@ AutoGeckoProfilerEntry::AutoGeckoProfilerEntry(JSRuntime* rt, const char* label, profiler_ = nullptr; return; } - sizeBefore_ = *profiler_->size_; - profiler_->beginPseudoJS(label, this); - profiler_->push(label, /* dynamicString = */ nullptr, /* sp = */ this, /* script = */ nullptr, - /* pc = */ nullptr, category); + spBefore_ = profiler_->stackPointer(); + + profiler_->pseudoStack_->pushCppFrame( + label, /* dynamicString = */ nullptr, /* sp = */ this, /* line = */ 0, + js::ProfileEntry::Category::OTHER, js::ProfileEntry::BEGIN_PSEUDO_JS); + + profiler_->pseudoStack_->pushCppFrame( + label, /* dynamicString = */ nullptr, /* sp = */ this, /* line = */ 0, category); } AutoGeckoProfilerEntry::~AutoGeckoProfilerEntry() @@ -465,9 +418,9 @@ AutoGeckoProfilerEntry::~AutoGeckoProfilerEntry() if (!profiler_) return; - profiler_->pop(); - profiler_->endPseudoJS(); - MOZ_ASSERT(sizeBefore_ == *profiler_->size_); + profiler_->pseudoStack_->pop(); // the C++ frame + profiler_->pseudoStack_->pop(); // the BEGIN_PSEUDO_JS frame + MOZ_ASSERT(spBefore_ == profiler_->stackPointer()); } GeckoProfilerBaselineOSRMarker::GeckoProfilerBaselineOSRMarker(JSRuntime* rt, bool hasProfilerFrame @@ -475,18 +428,22 @@ GeckoProfilerBaselineOSRMarker::GeckoProfilerBaselineOSRMarker(JSRuntime* rt, bo : profiler(&rt->geckoProfiler()) { MOZ_GUARD_OBJECT_NOTIFIER_INIT; - if (!hasProfilerFrame || !profiler->enabled() || - profiler->size() >= profiler->maxSize()) - { + if (!hasProfilerFrame || !profiler->enabled()) { profiler = nullptr; return; } - size_before = profiler->size(); - if (profiler->size() == 0) + uint32_t sp = profiler->pseudoStack_->stackPointer; + if (sp >= PseudoStack::MaxEntries) { + profiler = nullptr; + return; + } + + spBefore_ = sp; + if (sp == 0) return; - ProfileEntry& entry = profiler->stack()[profiler->size() - 1]; + volatile ProfileEntry& entry = profiler->pseudoStack_->entries[sp - 1]; MOZ_ASSERT(entry.isJs()); entry.setOSR(); } @@ -496,11 +453,12 @@ GeckoProfilerBaselineOSRMarker::~GeckoProfilerBaselineOSRMarker() if (profiler == nullptr) return; - MOZ_ASSERT(size_before == *profiler->size_); - if (profiler->size() == 0) + uint32_t sp = profiler->stackPointer(); + MOZ_ASSERT(spBefore_ == sp); + if (sp == 0) return; - ProfileEntry& entry = profiler->stack()[profiler->size() - 1]; + volatile ProfileEntry& entry = profiler->stack()[sp - 1]; MOZ_ASSERT(entry.isJs()); entry.unsetOSR(); } @@ -539,19 +497,24 @@ ProfileEntry::pc() const volatile return script ? script->offsetToPC(lineOrPcOffset) : nullptr; } +/* static */ int32_t +ProfileEntry::pcToOffset(JSScript* aScript, jsbytecode* aPc) { + return aPc ? aScript->pcToOffset(aPc) : NullPCOffset; +} + JS_FRIEND_API(void) ProfileEntry::setPC(jsbytecode* pc) volatile { MOZ_ASSERT(isJs()); JSScript* script = this->script(); MOZ_ASSERT(script); // This should not be called while profiling is suppressed. - lineOrPcOffset = pc == nullptr ? NullPCOffset : script->pcToOffset(pc); + lineOrPcOffset = pcToOffset(script, pc); } JS_FRIEND_API(void) -js::SetContextProfilingStack(JSContext* cx, ProfileEntry* stack, mozilla::Atomic* size, uint32_t max) +js::SetContextProfilingStack(JSContext* cx, PseudoStack* pseudoStack) { - cx->runtime()->geckoProfiler().setProfilingStack(stack, size, max); + cx->runtime()->geckoProfiler().setProfilingStack(pseudoStack); } JS_FRIEND_API(void) diff --git a/js/src/vm/GeckoProfiler.h b/js/src/vm/GeckoProfiler.h index 93c31500388b..be9c36d5bc90 100644 --- a/js/src/vm/GeckoProfiler.h +++ b/js/src/vm/GeckoProfiler.h @@ -129,38 +129,24 @@ class GeckoProfiler JSRuntime* rt; ExclusiveData strings; - ProfileEntry* stack_; - mozilla::Atomic* size_; - uint32_t max_; + PseudoStack* pseudoStack_; bool slowAssertions; uint32_t enabled_; void (*eventMarker_)(const char*); UniqueChars allocProfileString(JSScript* script, JSFunction* function); - void push(const char* label, const char* dynamicString, void* sp, JSScript* script, - jsbytecode* pc, ProfileEntry::Category category = ProfileEntry::Category::JS); - void pop(); public: explicit GeckoProfiler(JSRuntime* rt); bool init(); - uint32_t* addressOfMaxSize() { - return &max_; - } - - ProfileEntry** addressOfStack() { - return &stack_; - } - - uint32_t maxSize() { return max_; } - uint32_t size() { MOZ_ASSERT(installed()); return *size_; } - ProfileEntry* stack() { return stack_; } + uint32_t stackPointer() { MOZ_ASSERT(installed()); return pseudoStack_->stackPointer; } + volatile ProfileEntry* stack() { return pseudoStack_->entries; } /* management of whether instrumentation is on or off */ bool enabled() { MOZ_ASSERT_IF(enabled_, installed()); return enabled_; } - bool installed() { return stack_ != nullptr && size_ != nullptr; } + bool installed() { return pseudoStack_ != nullptr; } MOZ_MUST_USE bool enable(bool enabled); void enableSlowAssertions(bool enabled) { slowAssertions = enabled; } bool slowAssertionsEnabled() { return slowAssertions; } @@ -177,18 +163,18 @@ class GeckoProfiler bool enter(JSContext* cx, JSScript* script, JSFunction* maybeFun); void exit(JSScript* script, JSFunction* maybeFun); void updatePC(JSScript* script, jsbytecode* pc) { - if (enabled() && *size_ - 1 < max_) { - MOZ_ASSERT(*size_ > 0); - MOZ_ASSERT(stack_[*size_ - 1].rawScript() == script); - stack_[*size_ - 1].setPC(pc); + if (!enabled()) + return; + + uint32_t sp = pseudoStack_->stackPointer; + if (sp - 1 < PseudoStack::MaxEntries) { + MOZ_ASSERT(sp > 0); + MOZ_ASSERT(pseudoStack_->entries[sp - 1].rawScript() == script); + pseudoStack_->entries[sp - 1].setPC(pc); } } - /* Enter wasm code */ - void beginPseudoJS(const char* label, void* sp); // label must be a static string! - void endPseudoJS() { pop(); } - - void setProfilingStack(ProfileEntry* stack, mozilla::Atomic* size, uint32_t max); + void setProfilingStack(PseudoStack* pseudoStack); void setEventMarker(void (*fn)(const char*)); const char* profileString(JSScript* script, JSFunction* maybeFun); void onScriptFinalized(JSScript* script); @@ -203,7 +189,7 @@ class GeckoProfiler return &enabled_; } - void trace(JSTracer* trc); + void trace(JSTracer* trc) volatile; void fixupStringsMapAfterMovingGC(); #ifdef JSGC_HASH_TABLE_CHECKS void checkStringsMapAfterMovingGC(); @@ -237,7 +223,7 @@ class MOZ_RAII GeckoProfilerEntryMarker private: GeckoProfiler* profiler; - mozilla::DebugOnly size_before; + mozilla::DebugOnly spBefore_; MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER }; @@ -256,7 +242,7 @@ class MOZ_NONHEAP_CLASS AutoGeckoProfilerEntry private: GeckoProfiler* profiler_; - mozilla::DebugOnly sizeBefore_; + mozilla::DebugOnly spBefore_; MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER }; @@ -274,7 +260,7 @@ class MOZ_RAII GeckoProfilerBaselineOSRMarker private: GeckoProfiler* profiler; - mozilla::DebugOnly size_before; + mozilla::DebugOnly spBefore_; MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER }; diff --git a/tools/profiler/core/ThreadInfo.h b/tools/profiler/core/ThreadInfo.h index ea319ada1f7d..7639686dbb08 100644 --- a/tools/profiler/core/ThreadInfo.h +++ b/tools/profiler/core/ThreadInfo.h @@ -230,11 +230,10 @@ public: mContext = aContext; - js::SetContextProfilingStack( - aContext, - (js::ProfileEntry*) RacyInfo()->entries, - RacyInfo()->AddressOfStackPointer(), - (uint32_t) mozilla::ArrayLength(RacyInfo()->entries)); + // We give the JS engine a non-owning reference to the RacyInfo (just the + // PseudoStack, really). It's important that the JS engine doesn't touch + // this once the thread dies. + js::SetContextProfilingStack(aContext, RacyInfo()); PollJSSampling(); } diff --git a/tools/profiler/public/GeckoProfiler.h b/tools/profiler/public/GeckoProfiler.h index 680b8fdaa3dc..ff55010d4aa4 100644 --- a/tools/profiler/public/GeckoProfiler.h +++ b/tools/profiler/public/GeckoProfiler.h @@ -415,7 +415,7 @@ extern MOZ_THREAD_LOCAL(PseudoStack*) sPseudoStack; // necessarily bounded by the lifetime of the thread, which ensures that the // references held can't be used after the PseudoStack is destroyed. inline void* -profiler_call_enter(const char* aInfo, js::ProfileEntry::Category aCategory, +profiler_call_enter(const char* aLabel, js::ProfileEntry::Category aCategory, void* aFrameAddress, uint32_t aLine, const char* aDynamicString = nullptr) { @@ -425,7 +425,7 @@ profiler_call_enter(const char* aInfo, js::ProfileEntry::Category aCategory, if (!pseudoStack) { return pseudoStack; } - pseudoStack->push(aInfo, aCategory, aFrameAddress, aLine, aDynamicString); + pseudoStack->pushCppFrame(aLabel, aDynamicString, aFrameAddress, aLine, aCategory); // The handle is meant to support future changes but for now it is simply // used to avoid having to call TLSInfo::RacyInfo() in profiler_call_exit(). From 9e272b0923966e02ef92d6645e95ce0f033aa1ca Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Fri, 26 May 2017 00:40:12 -0400 Subject: [PATCH 3/4] Bug 1366812 - Account for the fact that the load event may fire even though mIsDocumentLoaded is true in TimeoutManager; r=cpearce a=topcrasher --- dom/base/TimeoutManager.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dom/base/TimeoutManager.cpp b/dom/base/TimeoutManager.cpp index f635f919ad29..191656eefe90 100644 --- a/dom/base/TimeoutManager.cpp +++ b/dom/base/TimeoutManager.cpp @@ -1572,7 +1572,12 @@ TimeoutManager::StartThrottlingTrackingTimeouts() void TimeoutManager::OnDocumentLoaded() { - MaybeStartThrottleTrackingTimout(); + // The load event may be firing again if we're coming back to the page by + // navigating through the session history, so we need to ensure to only call + // this when mThrottleTrackingTimeouts hasn't been set yet. + if (!mThrottleTrackingTimeouts) { + MaybeStartThrottleTrackingTimout(); + } } void From e84601e7a77a168b85afe4bb376b845aead58a9b Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Fri, 26 May 2017 10:59:16 +0300 Subject: [PATCH 4/4] Bug 1367365 - Disable RCWN r=me a=tomcat MozReview-Commit-ID: GuIC5Gv09Z3 --HG-- extra : amend_source : 38de21ab56fc9b05109bfc96511f81c718c136d7 --- modules/libpref/init/all.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 6ddf05451a17..05cd6b93c10d 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1683,7 +1683,7 @@ pref("network.http.keep_empty_response_headers_as_empty_string", true); pref("network.http.max_response_header_size", 393216); // If we should attempt to race the cache and network -pref("network.http.rcwn.enabled", true); +pref("network.http.rcwn.enabled", false); pref("network.http.rcwn.cache_queue_normal_threshold", 8); pref("network.http.rcwn.cache_queue_priority_threshold", 2); // We might attempt to race the cache with the network only if a resource