Skip to content

perf: skip null-context bind in eachMapping; direct names array read#67

Merged
7rulnik merged 1 commit into
mainfrom
perf/eachmapping-direct-names
May 10, 2026
Merged

perf: skip null-context bind in eachMapping; direct names array read#67
7rulnik merged 1 commit into
mainfrom
perf/eachmapping-direct-names

Conversation

@7rulnik
Copy link
Copy Markdown
Owner

@7rulnik 7rulnik commented May 10, 2026

Summary

Three structural changes to the per-mapping eachMapping loop in SourceMapConsumer:

  1. Skip aCallback.bind(context) when context === null. Function.prototype.bind produces a bound function V8 can't inline, so every per-mapping call dispatches through the internal [[Call]] path. With no context (the common case — both bench callsites and most consumers omit it), bind is pure overhead and the unbound aCallback inlines normally.

  2. Read this._names._array directly instead of this._names.at(idx) per named mapping. Saves the bounds-check + function-call pair — matches the slab-direct pattern already used in _serializeMappings / fromSourceMap.

  3. Hoist mapping.source / mapping.name into locals before building the result object. V8 should CSE this, but explicit locals are slightly more inline-friendly.

Bench

SOLO=1 PHASES=eachmapping-generated,eachmapping-original scripts/bench-diff.sh main trace (two-process, ops/sec via benchmark.js):

fixture order cand ops/sec base ops/sec Δ
amp.js.map generated 11,648 ±0.52% 3,505 ±8.05% +232.3%
amp.js.map original 11,783 ±0.70% 3,489 ±8.07% +237.7%
babel.min.js.map generated 1,191 ±0.80% 379 ±1.02% +214.2%
babel.min.js.map original 1,143 ±0.52% 383 ±1.07% +198.4%
issue-41.js.map generated 1,359 ±0.59% 428 ±1.03% +217.5%
issue-41.js.map original 1,366 ±0.70% 429 ±1.14% +218.4%
preact.js.map generated 301,487 ±0.30% 68,015 ±0.19% +343.3%
preact.js.map original 295,655 ±0.49% 66,782 ±0.29% +342.7%
react.js.map generated 106,632 ±0.40% 25,623 ±0.31% +316.2%
react.js.map original 89,972 ±0.28% 26,096 ±0.48% +244.8%
vscode.map generated 153 ±1.45% 56 ±? +173.4%
vscode.map original 150 ±0.76% 54 ±? +177.2%

Mean +243% across 12 rows.

Caveat on the headline number

The bench passes NOOP as the callback. With NOOP, the per-call cost is dominated by whatever happens outside the callback body, so the bound-vs-direct dispatch gap is fully exposed. With a real callback that does work, the proportional win shrinks (the callback body's cost is unchanged); the absolute time saved per call is the same. Baseline error bars on amp are ±8% vs candidate's ±0.5%, consistent with the bound-dispatch path being in a deopt/reopt cycle that the unbound path avoids.

Test plan

  • All 205 tests pass
  • Coverage: 98.14 / 97.37 / 96.75 — above thresholds (98 / 97 / 96)

Three small structural changes to the per-mapping eachMapping loop:

1. Skip `aCallback.bind(context)` when `context === null`. The bind
   produces a bound function V8 can't inline, so every per-mapping call
   dispatches through Function.prototype's internal [[Call]] path. With
   no context (the common case — both bench callsites and most consumers
   omit it), `bind` is unnecessary and the unbound `aCallback` inlines
   normally.

2. Read `this._names._array` directly instead of `this._names.at(idx)`
   per named mapping. Saves the bounds-check + function-call pair —
   matches the slab-direct pattern used in `_serializeMappings` /
   `fromSourceMap`.

3. Hoist `mapping.source` / `mapping.name` into locals before building
   the result object — V8 should CSE this, but explicit locals are
   slightly more inline-friendly.

Bench (`SOLO=1 PHASES=eachmapping-generated,eachmapping-original
scripts/bench-diff.sh main trace`, two-process):

  preact +343%, react +316%, amp +233%, issue-41 +218%, babel.min +214%,
  vscode +173% — mean +243% across 12 rows.

The headline number reflects the bench's NOOP callback amplifying the
bind-vs-inline gap. With a real callback that does work the proportional
win shrinks, but the structural fix (no bound dispatch when context is
null) stands. Baseline error bars on amp are ±8% vs candidate's ±0.5%,
consistent with the bound-dispatch path being in a deopt/reopt cycle.
@7rulnik 7rulnik merged commit bc7a1cd into main May 10, 2026
3 checks passed
@7rulnik 7rulnik deleted the perf/eachmapping-direct-names branch May 10, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant