perf: skip null-context bind in eachMapping; direct names array read#67
Merged
Conversation
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.
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
Three structural changes to the per-mapping
eachMappingloop inSourceMapConsumer:Skip
aCallback.bind(context)whencontext === null.Function.prototype.bindproduces 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),bindis pure overhead and the unboundaCallbackinlines normally.Read
this._names._arraydirectly instead ofthis._names.at(idx)per named mapping. Saves the bounds-check + function-call pair — matches the slab-direct pattern already used in_serializeMappings/fromSourceMap.Hoist
mapping.source/mapping.nameinto 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):Mean +243% across 12 rows.
Caveat on the headline number
The bench passes
NOOPas 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