Bug 1499507 - Fold the 'profiler is active' check into the 'JSContext has a non-null PseudoStack' check. r=sfink
authorMarkus Stange <mstange@themasta.com>
Tue, 06 Nov 2018 04:29:35 +0000
changeset 444527 18a8aa4a02c3bfd9b730bf7e338d2db361a01a6a
parent 444526 e63ddbf2a29063f16922c6bd835fe716b8c9c10a
child 444528 1f19af3b43746f48ca2d8351e60470dc1055f2d1
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.