Bug 1436179: Move allocation out of line. r?mstange
Also make it fallible, since I had to do it when I was trying to use js_malloc
anyway.
MozReview-Commit-ID: A800X1pMtWc
--- a/config/check_vanilla_allocations.py
+++ b/config/check_vanilla_allocations.py
@@ -138,16 +138,21 @@ def main():
# we whitelist them.
if "_memory_" in filename:
continue
# Ignore the fuzzing code imported from m-c
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)
else:
# An allocation is present in a non-special file. Fail!
fail("'" + fn + "' present in " + filename)
# Try to give more precise information about the offending code.
emit_line_info = True
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -2,19 +2,16 @@
* 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/. */
#ifndef js_ProfilingStack_h
#define js_ProfilingStack_h
-#include "mozilla/IntegerRange.h"
-#include "mozilla/UniquePtr.h"
-
#include <algorithm>
#include <stdint.h>
#include "jstypes.h"
#include "js/TypeDecls.h"
#include "js/Utility.h"
@@ -313,118 +310,98 @@ RegisterContextProfilingEventMarker(JSCo
//
class PseudoStack final
{
public:
PseudoStack()
: stackPointer(0)
{}
- ~PseudoStack() {
- delete[] entries;
- // 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) {
- ensureCapacityToPush([&] (js::ProfileEntry& entry) {
- entry.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
// the writes above will not be reordered below the stackPointer store.
// Do the read and the write as two separate statements, in order to
// make it clear that we don't need an atomic increment, which would be
// 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) {
- ensureCapacityToPush([&] (js::ProfileEntry& entry) {
- entry.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
// makes this assignment a release-store, so the writes above will not
// be reordered to occur after the stackPointer store.
// Do the read and the write as two separate statements, in order to
// make it clear that we don't need an atomic increment, which would be
// 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 pop() {
MOZ_ASSERT(stackPointer > 0);
// Do the read and the write as two separate statements, in order to
// make it clear that we don't need an atomic decrement, which would be
// 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;
}
- 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;
- // Ensures that there's capacity to push a frame at least up to MaxEntries, and if so executes
- // the functor with the current entry.
- template<typename Callback>
- void ensureCapacityToPush(const Callback& cb)
- {
- const size_t kInitialCapacity = 50;
-
- MOZ_ASSERT(entryCapacity >= stackPointer);
- if (stackPointer >= MaxEntries) {
- return;
- }
+ // No moving either, since we may contain interior pointers to inlineStorage.
+ PseudoStack(PseudoStack&&) = delete;
+ void operator=(PseudoStack&&) = delete;
- // Ensure that entries is always sane here.
- if (entryCapacity == stackPointer) {
- size_t newCapacity = entryCapacity ? entryCapacity * 2 : kInitialCapacity;
- auto* newEntries = new js::ProfileEntry[newCapacity];
- for (auto i : mozilla::IntegerRange(entryCapacity)) {
- newEntries[i] = entries[i];
- }
- js::ProfileEntry* oldEntries = entries;
- entries = newEntries;
- entryCapacity = newCapacity;
- delete[] oldEntries;
- }
- cb(entries[stackPointer]);
- }
+ // Reserve capacity for a few frames inline, because this code will run before the SM allocator
+ // is initialized, and we don't want to mess up with malloc / free at that point.
+ static const uint32_t kInlineCapacity = 4;
- size_t entryCapacity = 0;
+ uint32_t entryCapacity = kInlineCapacity;
+ js::ProfileEntry inlineStorage[kInlineCapacity];
public:
- static const uint32_t MaxEntries = 1024;
// 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;
+ mozilla::Atomic<js::ProfileEntry*> entries { inlineStorage };
- // 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).
//
// WARNING WARNING WARNING
//
// This is an atomic variable that uses ReleaseAcquire memory ordering.
// See the "Concurrency considerations" paragraph at the top of this file
--- a/js/src/moz.build
+++ b/js/src/moz.build
@@ -420,27 +420,31 @@ UNIFIED_SOURCES += [
# template instantiations.
# jsmath.cpp cannot be built in unified mode because it needs to re-#define the
# RtlGenRandom declaration's calling convention in <ntsecapi.h> on Windows.
# jsutil.cpp cannot be built in unified mode because it is needed for
# check-vanilla-allocations.
# 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
# files unlucky enough to be unified with it.
SOURCES += [
'builtin/Array.cpp',
'builtin/RegExp.cpp',
'frontend/Parser.cpp',
'gc/StoreBuffer.cpp',
'jsmath.cpp',
'jsutil.cpp',
+ 'perf/ProfilingStack.cpp',
'util/DoubleToString.cpp',
'vm/Interpreter.cpp',
'vm/JSAtom.cpp',
]
if CONFIG['JS_POSIX_NSPR']:
UNIFIED_SOURCES += [
'vm/PosixNSPR.cpp',
new file mode 100644
--- /dev/null
+++ b/js/src/perf/ProfilingStack.cpp
@@ -0,0 +1,53 @@
+/* -*- 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()
+{
+ if (entries != inlineStorage)
+ delete[] entries;
+ // 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);
+}
+
+bool PseudoStack::ensureCapacitySlow()
+{
+ MOZ_ASSERT(stackPointer >= entryCapacity);
+ MOZ_ASSERT(entryCapacity > 0);
+
+ uint32_t sp = stackPointer;
+ auto newCapacity = std::max(sp + 1, entryCapacity * 2);
+
+ auto* newEntries =
+ new (mozilla::fallible) js::ProfileEntry[newCapacity];
+ if (MOZ_UNLIKELY(!newEntries))
+ return false;
+
+ // It's important that `entries` / `entryCapacity` / `stackPointer` remain in sync here all
+ // the time.
+ for (auto i : mozilla::IntegerRange(entryCapacity))
+ newEntries[i] = entries[i];
+
+ js::ProfileEntry* oldEntries = entries;
+ entries = newEntries;
+ entryCapacity = newCapacity;
+ if (oldEntries != inlineStorage)
+ delete[] oldEntries;
+
+ return true;
+}
--- a/js/src/vm/GeckoProfiler-inl.h
+++ b/js/src/vm/GeckoProfiler-inl.h
@@ -16,17 +16,17 @@ namespace js {
inline void
GeckoProfilerThread::updatePC(JSContext* cx, JSScript* script, jsbytecode* pc)
{
if (!cx->runtime()->geckoProfiler().enabled())
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);
}
}
/*
* This class is used to suppress profiler sampling during
--- a/js/src/vm/GeckoProfiler.cpp
+++ b/js/src/vm/GeckoProfiler.cpp
@@ -219,17 +219,17 @@ GeckoProfilerThread::enter(JSContext* cx
return false;
}
#ifdef DEBUG
// 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.
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());
}
#endif
pseudoStack_->pushJsFrame("", dynamicString, script, script->code());
return true;
@@ -238,29 +238,29 @@ GeckoProfilerThread::enter(JSContext* cx
void
GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun)
{
pseudoStack_->pop();
#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 */
MOZ_ASSERT(dynamicString);
// Bug 822041
if (!pseudoStack_->entries[sp].isJs()) {
fprintf(stderr, "--- ABOUT TO FAIL ASSERTION ---\n");
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())
fprintf(stderr, " [%d] JS %s\n", i, entry.dynamicString());
else
fprintf(stderr, " [%d] C line %d %s\n", i, entry.line(), entry.dynamicString());
}
}
@@ -386,17 +386,17 @@ GeckoProfilerBaselineOSRMarker::GeckoPro
{
MOZ_GUARD_OBJECT_NOTIFIER_INIT;
if (!hasProfilerFrame || !cx->runtime()->geckoProfiler().enabled()) {
profiler = nullptr;
return;
}
uint32_t sp = profiler->pseudoStack_->stackPointer;
- if (sp >= PseudoStack::MaxEntries) {
+ if (sp >= profiler->pseudoStack_->stackCapacity()) {
profiler = nullptr;
return;
}
spBefore_ = sp;
if (sp == 0)
return;