Bug 1436179: Lazily grow the ProfileEntryStorage. r=mstange,jandem

MozReview-Commit-ID: BEGP1ykl4S

Author: emilio

config/check_vanilla_allocations.py

@@ -143,6 +143,11 @@ def main():
         if "Fuzzer" in filename:
             continue
 
+        # Ignore the profiling pseudo-stack, since it needs to run even when
+        # SpiderMonkey's allocator isn't initialized.
+        if "ProfilingStack" in filename:
+            continue
+
         fn = m.group(2)
         if filename == 'jsutil.o':
             jsutil_cpp.add(fn)

js/public/ProfilingStack.h

@@ -149,6 +149,20 @@ class ProfileEntry
     static int32_t pcToOffset(JSScript* aScript, jsbytecode* aPc);
 
   public:
+    ProfileEntry() = default;
+    ProfileEntry& operator=(const ProfileEntry& other)
+    {
+        label_ = other.label();
+        dynamicString_ = other.dynamicString();
+        void* spScript = other.spOrScript;
+        spOrScript = spScript;
+        int32_t offset = other.lineOrPcOffset;
+        lineOrPcOffset = offset;
+        uint32_t kindAndCategory = other.kindAndCategory_;
+        kindAndCategory_ = kindAndCategory;
+        return *this;
+    }
+
     enum class Kind : uint32_t {
         // A normal C++ frame.
         CPP_NORMAL = 0,
@@ -311,18 +325,14 @@ class PseudoStack final
       : 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);
-    }
+    ~PseudoStack();
 
     void pushCppFrame(const char* label, const char* dynamicString, void* sp, uint32_t line,
                       js::ProfileEntry::Kind kind, js::ProfileEntry::Category category) {
-        if (stackPointer < MaxEntries) {
-            entries[stackPointer].initCppFrame(label, dynamicString, sp, line, kind, category);
-        }
+        uint32_t oldStackPointer = stackPointer;
+
+        if (MOZ_LIKELY(entryCapacity > oldStackPointer) || MOZ_LIKELY(ensureCapacitySlow()))
+            entries[oldStackPointer].initCppFrame(label, dynamicString, sp, line, kind, category);
 
         // This must happen at the end! The compiler will not reorder this
         // update because stackPointer is Atomic<..., ReleaseAcquire>, so any
@@ -332,15 +342,15 @@ class PseudoStack final
         // more expensive on x86 than the separate operations done here.
         // This thread is the only one that ever changes the value of
         // stackPointer.
-        uint32_t oldStackPointer = stackPointer;
         stackPointer = oldStackPointer + 1;
     }
 
     void pushJsFrame(const char* label, const char* dynamicString, JSScript* script,
                      jsbytecode* pc) {
-        if (stackPointer < MaxEntries) {
-            entries[stackPointer].initJsFrame(label, dynamicString, script, pc);
-        }
+        uint32_t oldStackPointer = stackPointer;
+
+        if (MOZ_LIKELY(entryCapacity > oldStackPointer) || MOZ_LIKELY(ensureCapacitySlow()))
+            entries[oldStackPointer].initJsFrame(label, dynamicString, script, pc);
 
         // This must happen at the end! The compiler will not reorder this
         // update because stackPointer is Atomic<..., ReleaseAcquire>, which
@@ -351,7 +361,6 @@ class PseudoStack final
         // more expensive on x86 than the separate operations done here.
         // This thread is the only one that ever changes the value of
         // stackPointer.
-        uint32_t oldStackPointer = stackPointer;
         stackPointer = oldStackPointer + 1;
     }
 
@@ -366,20 +375,33 @@ class PseudoStack final
         stackPointer = oldStackPointer - 1;
     }
 
-    uint32_t stackSize() const { return std::min(uint32_t(stackPointer), uint32_t(MaxEntries)); }
+    uint32_t stackSize() const { return std::min(uint32_t(stackPointer), stackCapacity()); }
+    uint32_t stackCapacity() const { return entryCapacity; }
 
   private:
+    // Out of line path for expanding the buffer, since otherwise this would get inlined in every
+    // DOM WebIDL call.
+    MOZ_COLD MOZ_MUST_USE bool ensureCapacitySlow();
+
     // No copying.
     PseudoStack(const PseudoStack&) = delete;
     void operator=(const PseudoStack&) = delete;
 
+    // No moving either.
+    PseudoStack(PseudoStack&&) = delete;
+    void operator=(PseudoStack&&) = delete;
+
+    uint32_t entryCapacity = 0;
+
   public:
-    static const uint32_t MaxEntries = 1024;
 
-    // The stack entries.
-    js::ProfileEntry entries[MaxEntries];
+    // The pointer to the stack entries, this is read from the profiler thread and written from the
+    // current thread.
+    //
+    // This is effectively a unique pointer.
+    mozilla::Atomic<js::ProfileEntry*> entries { nullptr };
 
-    // This may exceed MaxEntries, so instead use the stackSize() method to
+    // This may exceed the entry capacity, 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).

js/src/moz.build

@@ -440,6 +440,9 @@ UNIFIED_SOURCES += [
 # StoreBuffer.cpp cannot be built in unified because its template
 #   instantiations may or may not be needed depending on what it gets bundled
 #   with.
+# perf/ProfilingStack.cpp cannot be built in unified mode because we want to
+#   suppress warnings due to usage of the system allocator, and this allows it
+#   to have a deterministic object name.
 # util/DoubleToString.cpp cannot be built in unified mode because we want to
 #   suppress compiler warnings in third-party dtoa.c.
 # vm/Interpreter.cpp is gigantic and destroys incremental build times for any
@@ -454,6 +457,7 @@ SOURCES += [
     'util/DoubleToString.cpp',
     'vm/Interpreter.cpp',
     'vm/JSAtom.cpp',
+    'vm/ProfilingStack.cpp',
 ]
 
 if CONFIG['JS_POSIX_NSPR']:

js/src/vm/GeckoProfiler-inl.h

@@ -21,7 +21,7 @@ GeckoProfilerThread::updatePC(JSContext* cx, JSScript* script, jsbytecode* pc)
         return;
 
     uint32_t sp = pseudoStack_->stackPointer;
-    if (sp - 1 < PseudoStack::MaxEntries) {
+    if (sp - 1 < pseudoStack_->stackCapacity()) {
         MOZ_ASSERT(sp > 0);
         MOZ_ASSERT(pseudoStack_->entries[sp - 1].rawScript() == script);
         pseudoStack_->entries[sp - 1].setPC(pc);

js/src/vm/GeckoProfiler.cpp

@@ -215,7 +215,7 @@ GeckoProfilerThread::enter(JSContext* cx, JSScript* script, JSFunction* maybeFun
     // have a non-null pc. Only look at the top frames to avoid quadratic
     // behavior.
     uint32_t sp = pseudoStack_->stackPointer;
-    if (sp > 0 && sp - 1 < PseudoStack::MaxEntries) {
+    if (sp > 0 && sp - 1 < pseudoStack_->stackCapacity()) {
         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());
@@ -234,7 +234,7 @@ GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun)
 #ifdef DEBUG
     /* Sanity check to make sure push/pop balanced */
     uint32_t sp = pseudoStack_->stackPointer;
-    if (sp < PseudoStack::MaxEntries) {
+    if (sp < pseudoStack_->stackCapacity()) {
         JSRuntime* rt = script->runtimeFromActiveCooperatingThread();
         const char* dynamicString = rt->geckoProfiler().profileString(script, maybeFun);
         /* Can't fail lookup because we should already be in the set */
@@ -246,7 +246,7 @@ GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun)
             fprintf(stderr, " entries=%p size=%u/%u\n",
                             (void*) pseudoStack_->entries,
                             uint32_t(pseudoStack_->stackPointer),
-                            PseudoStack::MaxEntries);
+                            pseudoStack_->stackCapacity());
             for (int32_t i = sp; i >= 0; i--) {
                 ProfileEntry& entry = pseudoStack_->entries[i];
                 if (entry.isJs())
@@ -382,7 +382,7 @@ GeckoProfilerBaselineOSRMarker::GeckoProfilerBaselineOSRMarker(JSContext* cx, bo
     }
 
     uint32_t sp = profiler->pseudoStack_->stackPointer;
-    if (sp >= PseudoStack::MaxEntries) {
+    if (sp >= profiler->pseudoStack_->stackCapacity()) {
         profiler = nullptr;
         return;
     }

js/src/vm/ProfilingStack.cpp

@@ -0,0 +1,52 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ * 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/. */
+
+#include "js/ProfilingStack.h"
+
+#include "mozilla/IntegerRange.h"
+#include "mozilla/UniquePtr.h"
+#include "mozilla/UniquePtrExtensions.h"
+
+#include <algorithm>
+
+using namespace js;
+
+PseudoStack::~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);
+
+    delete[] entries;
+}
+
+bool
+PseudoStack::ensureCapacitySlow()
+{
+    MOZ_ASSERT(stackPointer >= entryCapacity);
+    const uint32_t kInitialCapacity = 128;
+
+    uint32_t sp = stackPointer;
+    auto newCapacity = std::max(sp + 1,  entryCapacity ? entryCapacity * 2 : kInitialCapacity);
+
+    auto* newEntries =
+        new (mozilla::fallible) js::ProfileEntry[newCapacity];
+    if (MOZ_UNLIKELY(!newEntries))
+        return false;
+
+    // It's important that `entries` / `entryCapacity` / `stackPointer` remain consistent here at
+    // all times.
+    for (auto i : mozilla::IntegerRange(entryCapacity))
+        newEntries[i] = entries[i];
+
+    js::ProfileEntry* oldEntries = entries;
+    entries = newEntries;
+    entryCapacity = newCapacity;
+    delete[] oldEntries;
+
+    return true;
+}