perf: add TracingChannel early-exit when no subscribers#31
Closed
bm1549 wants to merge 5 commits into
Closed
Conversation
Match native Node.js >= 22 / >= 20.13 behavior introduced in nodejs/node#51915: when no channel has subscribers, traceSync / tracePromise / traceCallback skip publishing entirely and invoke fn directly. This was a known TODO in checks.js. Applied in two places: - patch-tracing-channel.js (full polyfill, Node < 18.19) - patch-tracing-channel-has-subscribers.js (native + getter wrap, Node 18.19-20.12) Per native semantics, the early-exit intentionally bypasses traceCallback's callback validation and tracePromise's thenable coercion. This matches Node 22+ behavior exactly. Microbenchmark on Node 24 (no subscribers): tracePromise sees a 2.5x speedup on the polyfill path. With subscribers, the slow path is within ~1% of baseline (the early-exit check has no measurable cost when it doesn't fire). Also renames the existing test/test-diagnostics-channel-tracing-channel-sync-early-exit.js to .spec.js so it actually runs (test.sh globs *.spec.js). Adds similar coverage for tracePromise and traceCallback early-exit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous wrapper used `(fn, context, thisArg, ...args)` rest + `[fn, context, thisArg, ...args]` re-spread, which allocated two arrays per call on the slow path (when subscribers exist). On Node 20.12.2 this added ~32 ns to traceSync and ~59 ns to traceCallback. Specialize for argc 3-6 (covers all common callers passing 0-3 user args), avoiding both allocations. Slow-path overhead drops to ~5 ns on traceSync and ~16 ns on traceCallback. Fast path (no subscribers) remains ~5 ns/op = 5-43x speedup vs unwrapped native. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…'s check
Two correctness fixes after Codex adversarial review:
1. Fast-path dispatch must not go through user-controlled fn.call.
`fn.call(...)` is vulnerable when user code does `f.call = null`,
which native trace*() never crashes on (it uses Reflect.apply).
Switch to a captured `Function.prototype.call.bind(Function.prototype.call)`
primordial that survives both `fn.call = null` and
`Function.prototype.call = null` poisoning, with no perf regression
vs the previous arity-specialized `.call` version.
2. Polyfill traceSync's early-exit must only check channels traceSync
would publish to (start/end/error). The previous version touched
asyncStart/asyncEnd, which crashed on partial object-form
TracingChannels (e.g. tracingChannel({start, end, error})) that
the pre-patch polyfill traceSync handled fine.
Adds regression test for shadowed fn.call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Make polyfill TC's hasSubscribers getter null-safe so partial
object-form construction (e.g. tracingChannel({start, end, error}))
can still probe hasSubscribers without crashing on the missing
asyncStart/asyncEnd channels. Add regression test.
- Promote uncurriedCall to primordials.js as FunctionPrototypeCallApply
so other patches can use the tamper-resistant call helper.
- Drop the duplicate copyAll helper; use sliceFrom(args, 0) instead.
- Trim unnecessary preamble comment from shadowed-call test.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On Node <18.19, both patch-tracing-channel.js (full polyfill) and patch-tracing-channel-has-subscribers.js are applied. The latter's getter overrides the polyfill class's null-safe getter, so partial object-form TracingChannels still crash on hasSubscribers. Make this getter null-safe as well. Native TC channels are always defined, so the extra checks are free defensive cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the TracingChannel "early-exit when no subscribers" optimization, matching native Node.js >= 22 / >= 20.13 behavior introduced in nodejs/node#51915. This was a known TODO in
checks.js(now removed).When no channel has subscribers,
traceSync/tracePromise/traceCallbackskip publishing entirely and invokefndirectly. Per native semantics, this intentionally bypassestraceCallback's callback validation andtracePromise's thenable coercion — both match Node 22+ exactly.Applied in two places:
patch-tracing-channel.js— full polyfill, used on Node < 18.19patch-tracing-channel-has-subscribers.js— wraps native TC's three trace methods, used on Node 18.19–20.12Implementation notes
The wrapper for the native path uses arity-specialized branches (3–7 arg fast paths) and dispatches via a tamper-resistant
Function.prototype.call.bind(Function.prototype.call)primordial. This:[fn, context, thisArg, ...args]re-spread allocation on the slow path.f.call = nullorFunction.prototype.call = null(native trace*() doesn't crash on these — neither should the polyfill).The polyfill's
hasSubscribersgetter is null-safe so partial object-formtracingChannel({start, end, error})construction (which the polyfill has always accepted leniently, unlike native) can still probetc.hasSubscriberswithout crashing on missing async channels.Benchmark — Node 20.12.2 (
has-subscriberspatch path, the primary target)This is the path used in production on Node 18.19–20.12, where native TC exists but lacks early-exit. Median of 3 runs.
No subscribers — early-exit fires (the hot fast path)
traceSynctraceCallbacktracePromise(The sub-6 ns/op numbers reflect V8 fully inlining through the early-exit at the benchmark site — the upper bound. Real-world wins are bounded by the baseline column: ~25–290 ns of native trace work eliminated per call.)
With subscribers — slow path runs
traceSynctraceCallbacktracePromiseThe arity-specialized wrapper kept the slow-path regression small (4–29 ns vs ~30–60 ns with a naive
(fn, context, thisArg, ...args)rest-spread wrapper).Benchmark — Node 20.12.2 (full polyfill path, Node < 18.19, legacy)
No subscribers (early-exit fires)
traceSyncpublish()in baseline)traceCallbacktracePromiseThe polyfill
traceSyncregression is a V8 inlining artifact: V8 fully inlines the polyfill's empty JSpublish()no-op into nothing, so the baseline appears very fast. The early-exit check defeats that inlining. This codepath is only reached on Node < 18.19 (legacy), and the absolute overhead is ~100 ns.With subscribers (slow path)
All three methods within ~1–12% of baseline (noise).
Test plan
./test.sh)npm run lint)test-diagnostics-channel-tracing-channel-sync-early-exit.js→.spec.jsso it actually runs (test.sh globs*.spec.js); fixed a tapet.plan(0)issue with explicitt.end()tracePromiseandtraceCallbackearly-exitfn.call = nullpoisoning (Reflect.apply / primordial dispatch)tracingChannel({start, end, error})on the polyfill path🤖 Generated with Claude Code