Bug 1499507 - Fold the 'profiler is active' check into the 'JSContext has a non-null PseudoStack' check. r=sfink
☠☠ backed out by 848152c22f8b ☠ ☠
authorMarkus Stange <mstange@themasta.com>
Mon, 05 Nov 2018 19:06:08 +0000
changeset 444447 4a9c9a91182cbf3055d94055d728cc0888459092
parent 444446 634d9ca93c9491c01d901c53418288bc284567d2
child 444448 bc134fe1722a5fee0ba7f3e663ef26c7c629ad65
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.