perf: check for subscribers before building the per-call context in wrappers

[pabloerhard]

What

Generated wrappers built their per-call transport on every call, even with no subscribers: the arguments array plus its .slice(0, arguments.length) copy, the __apm$ctx object, and the __apm$traced closure. They only checked tr_ch_apm_hasSubscribers after that. Callback wrappers also ran Array.prototype.at and created the hoisted __apm$wrappedCb closure first, Auto ran at first, and patched iterator methods copied arguments before checking.

This moves the check to just after the __apm$traced declaration, which now receives the argument list as a parameter. An unsubscribed call goes straight to the original with arguments (or [params] when the wrapper is an arrow, which has no own arguments). The now-duplicate checks are removed from the Sync/Async templates, __apm$wrappedCb becomes a const function expression, and iterator methods check before copying.

The AST shape that idempotency and downstream custom transforms depend on is unchanged: const __apm$traced = <arrow> holding const __apm$wrapped = <fn>, a top-level __apm$ctx object, a top-level if (!tr_ch_apm_hasSubscribers(ch)) return __apm$traced(...) check, and two __apm$traced calls in Sync/Async wrappers.

Why behaviour is unchanged

• No subscribers: nothing can observe or change the arguments, so skipping the transport has no visible effect.
• Subscribed: the callee gets the same __apm$arguments array after start, so in-place mutation (replace, append, callback splice) still reaches it.
• Arguments: the wrapper has a rest parameter, so its arguments lists exactly what [...].slice(0, arguments.length) produced.
• Constructors: before, the fast path got an unsliced array padded with undefined. Parameters, defaults and rest bind the same either way, and arguments inside the moved arrow body is still the constructor's own. new.target is unchanged.
• this is still lexical through the arrow __apm$traced. Generator and async flags stay on the inner function.
• __apm$super setup is still added above everything.
• Check-then-publish: the same check is read once, and nothing user-visible runs between it and runStores.
• Callback/Auto: keep their start-only check after the full one, so they take the fast path only when the old code would have.
• Subscribed calls: the contract is unchanged: a fresh ctx per call, start via runStores, end in finally, ctx.error set before error publishes, and result replacement.

Tests

• fast_path_cjs: every kind (Sync, Async, Callback, Auto, Iterator/AsyncIterator returnKind, class method, base and derived constructor, arrow expression, runtime-patched instance method) with fewer, exact and more arguments. It goes unsubscribed → subscribed → unsubscribed and spies on slice/at to assert no allocation on the fast path. It also pins iterator next with only the main channel subscribed, and Callback/Auto with only asyncEnd subscribed.
• arguments_mutation_kinds_cjs: subscribed in-place mutation for Sync, Async, Callback and constructors.
• fast path ordering: checks the generated code for the ordering above.

The behaviour assertions pass unchanged on main; only the allocation and ordering checks fail there.

Benchmarks

main (70ea063, which includes #101) vs. this branch. Node v25.0.0, Apple M4 Max, 11 fresh processes per side, median ns/op:

unsubscribed	before	after	Δ	minor GCs / 1M calls
Sync	32.58	12.12	−62.8%	16.6 → 7.6
Async	32.92	12.28	−62.7%	18.6 → 10.4
Callback	49.57	12.33	−75.1%	17.2 → 9.0
Auto	49.75	12.65	−74.6%	17.4 → 9.0
class method	30.05	12.13	−59.6%	15.8 → 7.6
derived constructor	33.15	23.40	−29.4%	14.8 → 12.0
arrow expression	20.72	11.25	−45.7%	11.6 → 6.8
Iterator (call + next())	443.37	408.24	−7.9%	2.8 → 2.4

Arrow expressions gain less than the others. Their wrapper has no own arguments, so the fast path still copies the parameters into a new array ([__apm$arg0, ...__apm$args]) and spreads it into the original. They also started cheaper, because the arrow preamble never had the .slice() copy. Avoiding that copy needs the original hoisted out of __apm$traced (see follow-ups).

Subscribed calls are within noise. With all five events and noop handlers: Sync −1.8%, Async −2.4%, Callback −1.4%, Auto −0.5%, method −2.3%, ctor +0.8%, arrow +1.4%, Iterator −1.0% (spread per side is 3–21%). With an AsyncLocalStorage bound to start: Sync +1.4%, Async −0.9%, Callback −0.6%, Auto −0.6%, method −0.8%, ctor −0.4%, arrow +1.0%.

Follow-ups (not in this PR)

• Remove the per-call allocations still left on the fast path: the __apm$traced closure, the __apm$wrapped function created inside it, the arrow wrappers' copied argument array, and, for methods that use super, the __apm$super object and its assignments. This means hoisting the original out of the wrapper. Measured for plain function declarations at about 12 → 7.6 ns more.
• Decide whether Callback/Auto should use the full five-event check instead of start-only. That would change behaviour.

[isaacs]

This is a good change, I think, and improving performance is worth a lot. Thanks for sending it!

The change in the __apm$traced signature (which affects the custom transforms contract) is the only hazard that could potentially cause problems or need some more consideration.

@jsumners-nr @timfish @bizob2828 @bengl What's your take? I think it's fine for Sentry, but if you think it's worth a more significant version bump (or will cause problems with other platforms) then I wouldn't want to throw any surprises at you by landing it.

[isaacs]

Hm, __apm$traced now has a parameter, which changes an implicit contract with custom transforms.

Custom transforms are public (addTransform, state.transforms.defaults from #99), and they post-process this output. A custom transform that emits or rewrites a call as __apm$traced() with no arguments will not throw. The original then silently gets no arguments (.apply(this, undefined), or ...undefined throws a TypeError for the arrow variant). That is a silent break, not a loud one.

I checked the two downstream users that GitHub code search finds:

• DataDog/dd-trace-js packages/datadog-instrumentations/src/helpers/rewriter/transforms.js, configureGraphqlTraceFastPath. It finds the guard with IfStatement[test.operator="!"][consequent.type="ReturnStatement"]:has(CallExpression[callee.name="__apm$traced"]), asserts two __apm$traced calls, replaces both callees and their arguments, removes the __apm$traced declaration, and moves the guard to the top. All of that still works with the new shape, because it rewrites the arguments itself. (This is the same "hoist the original" idea that the PR lists as a follow-up.)
• getsentry/sentry-javascript packages/cloudflare/src/orchestrion-diagnostics-channel.ts makes hasSubscribers always true, so it never takes the fast path. Only a doc comment there quotes return __apm$traced().

Suggestion: say in the PR body (and make sure to keep in the squash commit and changelog) that __apm$traced now takes the call arguments as its only parameter, and that custom transforms which call it must pass __apm$arguments (or arguments).

The perf: commit type can possibly hide this from people who read the changelog for API changes. Consider adding a test in tests/tests.test.mjs that asserts __apm$traced has exactly one parameter, so that a later change to the signature is a deliberate one.

[isaacs]

Ah, also, the fast path still creates __apm$traced and, inside it, __apm$wrapped on every call

Every unsubscribed call still allocates the __apm$traced arrow, and calling it allocates the __apm$wrapped function. For methods that use super, wrapSuper also puts const __apm$super = {} and one assignment per member above the check. The benchmark numbers include these costs. The PR lists the hoist as a follow-up, which is reasonable.

I don't think there needs to be any change for this. But in the follow-up, include the __apm$super object, which is the other per-call allocation left on the fast path.

[pabloerhard]

Couldn't we add backwards compatibility doing something like the following:

let __apm$arguments; // No array allocation yet.

const __apm$traced = (
  __apm$callArgs = (__apm$arguments ??= buildArgumentArray())
) => {
  // Existing __apm$wrapped declaration and invocation.
};

if (!hasSubscribers(channel)) {
  return __apm$traced(fastArgs);
}

__apm$arguments = buildArgumentArray();

Note: in this case buildArgumentArray is just a shorthand for the existing arguments copying logic and it's just meant for showcasing the above cleaner.

This would keep __apm$traced() working for custom transforms, reuse subscriber-modified arguments, and still skip array construction on the normal fast path.

[isaacs]

Yeah, I think that would work. I think if we don't require any changes in the downstream consumers, then the contract has only expanded, not broken. 👍

[pabloerhard]

I tried implementing the above proposal, but it gave back about 57% of the sync fast-path performance gain (it even made arrows and derived constructors slower for some reason). It looks like adding the default parameter interferes with some V8 optimizations, even when arguments are passed explicitly and the default never runs.

I'll look into other possible ways we can keep both the performance gain and the backwards compatibility. Maybe we could keep the legacy wrapper when custom transforms are present and use the fast path otherwise.

[pabloerhard]

Update: I implemented the above locally, it involves retaining the legacy wrapper shape for files using custom transforms and the fast wrapper otherwise. Benchmarks retained the same gain for the fast path, while custom transforms stayed on par with the current state. What do you think, should we move forward with this as a temporary backward compatible solution? We could also look into version gating it and removing the legacy wrappers in the next major release.

[isaacs]

The arrow fast path still copies the arguments into a new array, then spreads it.

Line 393 sets the fast-path argument to the array literal [${args}], because an arrow has no own arguments. Line 391 makes __apm$traced call an arrow original with __apm$wrapped(...__apm$callArgs). The generated arrow fast path is then:

if (!tr_ch_apm_hasSubscribers(ch)) return __apm$traced([__apm$arg0, ...__apm$args]);
// inside __apm$traced:
return __apm$wrapped(...__apm$callArgs);

So each unsubscribed arrow call reads the rest array __apm$args, builds a new array from it, and spreads that array again into the call. A non-arrow wrapper passes arguments and calls .apply, which does not build a new array. (The constructor case also uses line 391, but its outer function is not an arrow, so it gets arguments from line 393 and only does the one spread.) Arrow expressions are in the tests (observable: false, so allocation is not checked) but not in the benchmark table. The PR body is honest that arrows use [params], but the reader can not tell how much arrows gain.

Suggestion: no code change needed here, but it would be good to add an arrow row to the benchmark table, or say that arrows gain less. A cheaper arrow fast path (for example, calling __apm$wrapped directly) needs __apm$wrapped hoisted out of __apm$traced, so it belongs with the "hoist the original" follow-up, not this PR.

[isaacs]

@pabloerhard I updated your branch to fix the merge conflicts with main. Make sure to pull before pushing back to it 👍