Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 3be6c84

Browse files
committed
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 --HG-- extra : moz-landing-system : lando
1 parent d10e0ca commit 3be6c84

4 files changed

Lines changed: 20 additions & 15 deletions

File tree

js/public/ProfilingStack.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,12 +471,16 @@ class GeckoProfilerThread
471471

472472
ProfilingStack* profilingStack_;
473473

474+
// Same as profilingStack_ if the profiler is currently active, otherwise null.
475+
ProfilingStack* profilingStackIfEnabled_;
476+
474477
public:
475478
GeckoProfilerThread();
476479

477480
uint32_t stackPointer() { MOZ_ASSERT(infraInstalled()); return profilingStack_->stackPointer; }
478481
ProfilingStackFrame* stack() { return profilingStack_->frames; }
479482
ProfilingStack* getProfilingStack() { return profilingStack_; }
483+
ProfilingStack* getProfilingStackIfEnabled() { return profilingStackIfEnabled_; }
480484

481485
/*
482486
* True if the profiler infrastructure is setup. Should be true in builds
@@ -485,7 +489,8 @@ class GeckoProfilerThread
485489
*/
486490
bool infraInstalled() { return profilingStack_ != nullptr; }
487491

488-
void setProfilingStack(ProfilingStack* profilingStack);
492+
void setProfilingStack(ProfilingStack* profilingStack, bool enabled);
493+
void enable(bool enable) { profilingStackIfEnabled_ = enable ? profilingStack_ : nullptr; }
489494
void trace(JSTracer* trc);
490495

491496
/*

js/public/RootingAPI.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,8 @@ class RootingContext
866866

867867
// Gecko profiling metadata.
868868
// This isn't really rooting related. It's only here because we want
869-
// GetContextProfilingStack to be inlineable into non-JS code, and we
870-
// didn't want to add another superclass of JSContext just for this.
869+
// GetContextProfilingStackIfEnabled to be inlineable into non-JS code, and
870+
// we didn't want to add another superclass of JSContext just for this.
871871
js::GeckoProfilerThread geckoProfiler_;
872872

873873
public:
@@ -1093,9 +1093,9 @@ GetContextZone(const JSContext* cx)
10931093
}
10941094

10951095
inline ProfilingStack*
1096-
GetContextProfilingStack(JSContext* cx)
1096+
GetContextProfilingStackIfEnabled(JSContext* cx)
10971097
{
1098-
return JS::RootingContext::get(cx)->geckoProfiler().getProfilingStack();
1098+
return JS::RootingContext::get(cx)->geckoProfiler().getProfilingStackIfEnabled();
10991099
}
11001100

11011101
/**

js/src/vm/GeckoProfiler.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ using mozilla::DebugOnly;
3030

3131
GeckoProfilerThread::GeckoProfilerThread()
3232
: profilingStack_(nullptr)
33+
, profilingStackIfEnabled_(nullptr)
3334
{
3435
}
3536

@@ -44,9 +45,10 @@ GeckoProfilerRuntime::GeckoProfilerRuntime(JSRuntime* rt)
4445
}
4546

4647
void
47-
GeckoProfilerThread::setProfilingStack(ProfilingStack* profilingStack)
48+
GeckoProfilerThread::setProfilingStack(ProfilingStack* profilingStack, bool enabled)
4849
{
4950
profilingStack_ = profilingStack;
51+
profilingStackIfEnabled_ = enabled ? profilingStack : nullptr;
5052
}
5153

5254
void
@@ -472,12 +474,14 @@ ProfilingStackFrame::setPC(jsbytecode* pc)
472474
JS_FRIEND_API(void)
473475
js::SetContextProfilingStack(JSContext* cx, ProfilingStack* profilingStack)
474476
{
475-
cx->geckoProfiler().setProfilingStack(profilingStack);
477+
cx->geckoProfiler().setProfilingStack(profilingStack,
478+
cx->runtime()->geckoProfiler().enabled());
476479
}
477480

478481
JS_FRIEND_API(void)
479482
js::EnableContextProfilingStack(JSContext* cx, bool enabled)
480483
{
484+
cx->geckoProfiler().enable(enabled);
481485
cx->runtime()->geckoProfiler().enable(enabled);
482486
}
483487

tools/profiler/public/GeckoProfiler.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -728,20 +728,16 @@ class MOZ_RAII AutoProfilerLabel
728728
Push(sProfilingStack.get(), aLabel, aDynamicString, aLine, aCategory);
729729
}
730730

731-
// This is the AUTO_PROFILER_LABEL_FAST variant. It's guarded on
732-
// profiler_is_active() and retrieves the ProfilingStack from the JSContext.
731+
// This is the AUTO_PROFILER_LABEL_FAST variant. It retrieves the ProfilingStack
732+
// from the JSContext and does nothing if the profiler is inactive.
733733
AutoProfilerLabel(JSContext* aJSContext,
734734
const char* aLabel, const char* aDynamicString,
735735
uint32_t aLine, js::ProfilingStackFrame::Category aCategory
736736
MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
737737
{
738738
MOZ_GUARD_OBJECT_NOTIFIER_INIT;
739-
if (profiler_is_active()) {
740-
Push(js::GetContextProfilingStack(aJSContext),
741-
aLabel, aDynamicString, aLine, aCategory);
742-
} else {
743-
mProfilingStack = nullptr;
744-
}
739+
Push(js::GetContextProfilingStackIfEnabled(aJSContext),
740+
aLabel, aDynamicString, aLine, aCategory);
745741
}
746742

747743
void Push(ProfilingStack* aProfilingStack,

0 commit comments

Comments
 (0)