Bug 1499507 - Fold the 'profiler is active' check into the 'JSContext has a non-null PseudoStack' check. r=sfink
☠☠ backed out by ccf46ccf3519 ☠ ☠
authorMarkus Stange <mstange@themasta.com>
Mon, 05 Nov 2018 20:53:58 +0000
changeset 444469 24a9494155fec56521621c91344f81be9cb2a5e7
parent 444468 6317ff97e5c05cdd435f9218220dfacd1ba6f06a
child 444470 30476b824eb400ec1ad0f3493d97df15f337a87e
push id34996
push userrgurzau@mozilla.com
push dateTue, 06 Nov 2018 09:53:23 +0000
treeherdermozilla-central@e160f0a60e4f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1499507
milestone65.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 1499507 - Fold the 'profiler is active' check into the 'JSContext has a non-null PseudoStack' check. r=sfink This eliminates a few instructions from every profiler label and saves code size. We have around 9000 WebIDL constructors + methods + getters + setters which all have an inlined instance of this code. This change reduces the binary size on Linux x64 by around 160KB. Here's a diff of the impact on the code generated for Attr_Binding::get_specified in the Mac build: movq %rsp, %rbp pushq %r15 pushq %r14 pushq %r12 pushq %rbx subq $0x10, %rsp movq %rcx, %r14 movq %rdx, %r15 - movq __ZN7mozilla8profiler6detail12RacyFeatures18sActiveAndFeaturesE@GOT, %rax ; __ZN7mozilla8profiler6detail12RacyFeatures18sActiveAndFeaturesE@GOT - movl (%rax), %eax - testl %eax, %eax - js loc_xxxxx - - movq $0x0, -40(%rbp) - jmp loc_xxxxx - - movq 0x78(%rdi), %rbx + movq 0x80(%rdi), %rbx movq %rbx, -40(%rbp) testq %rbx, %rbx je loc_xxxxx movl 0x10(%rbx), %r12d cmpl %r12d, (%rbx) jbe loc_xxxxx Differential Revision: https://phabricator.services.mozilla.com/D9192
js/public/ProfilingStack.h
js/public/RootingAPI.h
js/src/vm/GeckoProfiler.cpp
tools/profiler/public/GeckoProfiler.h
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -466,31 +466,36 @@ class GeckoProfilerBaselineOSRMarker;
 class GeckoProfilerThread
 {
     friend class AutoGeckoProfilerEntry;
     friend class GeckoProfilerEntryMarker;
     friend class GeckoProfilerBaselineOSRMarker;
 
     ProfilingStack*         profilingStack_;
 
+    // Same as profilingStack_ if the profiler is currently active, otherwise null.
+    ProfilingStack*         profilingStackIfEnabled_;
+
   public:
     GeckoProfilerThread();
 
     uint32_t stackPointer() { MOZ_ASSERT(infraInstalled()); return profilingStack_->stackPointer; }
     ProfilingStackFrame* stack() { return profilingStack_->frames; }
     ProfilingStack* getProfilingStack() { return profilingStack_; }
+    ProfilingStack* getProfilingStackIfEnabled() { return profilingStackIfEnabled_; }
 
     /*
      * True if the profiler infrastructure is setup.  Should be true in builds
      * that include profiler support except during early startup or late
      * shutdown.  Unrelated to the presence of the Gecko Profiler addon.
      */
     bool infraInstalled() { return profilingStack_ != nullptr; }
 
-    void setProfilingStack(ProfilingStack* profilingStack);
+    void setProfilingStack(ProfilingStack* profilingStack, bool enabled);
+    void enable(bool enable) { profilingStackIfEnabled_ = enable ? profilingStack_ : nullptr; }
     void trace(JSTracer* trc);
 
     /*
      * Functions which are the actual instrumentation to track run information
      *
      *   - enter: a function has started to execute
      *   - updatePC: updates the pc information about where a function
      *               is currently executing
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -861,18 +861,18 @@ class RootingContext
     template <typename T> friend class JS::Rooted;
 
     // Stack GC roots for AutoFooRooter classes.
     JS::AutoGCRooter* autoGCRooters_;
     friend class JS::AutoGCRooter;
 
     // Gecko profiling metadata.
     // This isn't really rooting related. It's only here because we want
-    // GetContextProfilingStack to be inlineable into non-JS code, and we
-    // didn't want to add another superclass of JSContext just for this.
+    // GetContextProfilingStackIfEnabled to be inlineable into non-JS code, and
+    // we didn't want to add another superclass of JSContext just for this.
     js::GeckoProfilerThread geckoProfiler_;
 
   public:
     RootingContext();
 
     void traceStackRoots(JSTracer* trc);
     void checkNoGCRooters();
 
@@ -1088,19 +1088,19 @@ GetContextCompartment(const JSContext* c
 
 inline JS::Zone*
 GetContextZone(const JSContext* cx)
 {
     return JS::RootingContext::get(cx)->zone_;
 }
 
 inline ProfilingStack*
-GetContextProfilingStack(JSContext* cx)
+GetContextProfilingStackIfEnabled(JSContext* cx)
 {
-    return JS::RootingContext::get(cx)->geckoProfiler().getProfilingStack();
+    return JS::RootingContext::get(cx)->geckoProfiler().getProfilingStackIfEnabled();
 }
 
 /**
  * Augment the generic Rooted<T> interface when T = JSObject* with
  * class-querying and downcasting operations.
  *
  * Given a Rooted<JSObject*> obj, one can view
  *   Handle<StringObject*> h = obj.as<StringObject*>();
--- a/js/src/vm/GeckoProfiler.cpp
+++ b/js/src/vm/GeckoProfiler.cpp
@@ -25,33 +25,35 @@
 #include "gc/Marking-inl.h"
 
 using namespace js;
 
 using mozilla::DebugOnly;
 
 GeckoProfilerThread::GeckoProfilerThread()
   : profilingStack_(nullptr)
+  , profilingStackIfEnabled_(nullptr)
 {
 }
 
 GeckoProfilerRuntime::GeckoProfilerRuntime(JSRuntime* rt)
   : rt(rt),
     strings(mutexid::GeckoProfilerStrings),
     slowAssertions(false),
     enabled_(false),
     eventMarker_(nullptr)
 {
     MOZ_ASSERT(rt != nullptr);
 }
 
 void
-GeckoProfilerThread::setProfilingStack(ProfilingStack* profilingStack)
+GeckoProfilerThread::setProfilingStack(ProfilingStack* profilingStack, bool enabled)
 {
     profilingStack_ = profilingStack;
+    profilingStackIfEnabled_ = enabled ? profilingStack : nullptr;
 }
 
 void
 GeckoProfilerRuntime::setEventMarker(void (*fn)(const char*))
 {
     eventMarker_ = fn;
 }
 
@@ -467,22 +469,24 @@ ProfilingStackFrame::setPC(jsbytecode* p
     JSScript* script = this->script();
     MOZ_ASSERT(script); // This should not be called while profiling is suppressed.
     lineOrPcOffset = pcToOffset(script, pc);
 }
 
 JS_FRIEND_API(void)
 js::SetContextProfilingStack(JSContext* cx, ProfilingStack* profilingStack)
 {
-    cx->geckoProfiler().setProfilingStack(profilingStack);
+    cx->geckoProfiler().setProfilingStack(profilingStack,
+        cx->runtime()->geckoProfiler().enabled());
 }
 
 JS_FRIEND_API(void)
 js::EnableContextProfilingStack(JSContext* cx, bool enabled)
 {
+    cx->geckoProfiler().enable(enabled);
     cx->runtime()->geckoProfiler().enable(enabled);
 }
 
 JS_FRIEND_API(void)
 js::RegisterContextProfilingEventMarker(JSContext* cx, void (*fn)(const char*))
 {
     MOZ_ASSERT(cx->runtime()->geckoProfiler().enabled());
     cx->runtime()->geckoProfiler().setEventMarker(fn);
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -723,30 +723,26 @@ public:
                     MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 
     // Get the ProfilingStack from TLS.
     Push(sProfilingStack.get(), aLabel, aDynamicString, aLine, aCategory);
   }
 
-  // This is the AUTO_PROFILER_LABEL_FAST variant. It's guarded on
-  // profiler_is_active() and retrieves the ProfilingStack from the JSContext.
+  // This is the AUTO_PROFILER_LABEL_FAST variant. It retrieves the ProfilingStack
+  // from the JSContext and does nothing if the profiler is inactive.
   AutoProfilerLabel(JSContext* aJSContext,
                     const char* aLabel, const char* aDynamicString,
                     uint32_t aLine, js::ProfilingStackFrame::Category aCategory
                     MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-    if (profiler_is_active()) {
-      Push(js::GetContextProfilingStack(aJSContext),
-           aLabel, aDynamicString, aLine, aCategory);
-    } else {
-      mProfilingStack = nullptr;
-    }
+    Push(js::GetContextProfilingStackIfEnabled(aJSContext),
+         aLabel, aDynamicString, aLine, aCategory);
   }
 
   void Push(ProfilingStack* aProfilingStack,
             const char* aLabel, const char* aDynamicString,
             uint32_t aLine, js::ProfilingStackFrame::Category aCategory)
   {
     // This function runs both on and off the main thread.