Skip to content

perf: ArraySet.add returns index — drop trailing indexOf in hot paths#65

Merged
7rulnik merged 1 commit into
mainfrom
perf/arrayset-add-returns-idx
May 10, 2026
Merged

perf: ArraySet.add returns index — drop trailing indexOf in hot paths#65
7rulnik merged 1 commit into
mainfrom
perf/arrayset-add-returns-idx

Conversation

@7rulnik
Copy link
Copy Markdown
Owner

@7rulnik 7rulnik commented May 10, 2026

Summary

ArraySet.add now returns the string's index in the set (existing or new). Three per-mapping hot paths previously paired add() with indexOf() to grab that index — that's two Map operations where one suffices. Letting add return it cuts the redundant Map.get.

Affected hot paths:

  • SourceMapGenerator.addMapping — once per added mapping (source + name)
  • SourceMapGenerator.applySourceMap — once per rebuilt mapping (source + name)
  • IndexedSourceMapConsumer._parseMappings — once per section mapping (source + name)

Bench

SOLO=1 PHASES=adding scripts/bench-diff.sh main generate (two-process, ops/sec via benchmark.js):

fixture cand ops/sec base ops/sec Δ
amp.js.map 559 ±0.45% 464 ±0.31% +20.5%
babel.min.js.map 68.90 ±0.92% 68.73 ±0.42% +0.2%
issue-41.js.map 115 ±0.76% 85.30 ±0.79% +34.8%
preact.js.map 14,650 ±0.19% 11,247 ±0.13% +30.3%
react.js.map 4,969 ±0.19% 4,092 ±0.44% +21.4%
vscode.map 9.66 ±2.89% 7.98 ±3.04% +21.1%

Sample sizes 24–100 runs per cell, error bars ≤3%. Mean +21.4%. babel.min is flat because it has very few unique sources/names — the saved Map.get is amortized away.

API

ArraySet.add gains a return value (the index). Existing callers that ignore the return value continue to work unchanged — no breaking change.

Test plan

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

Three per-mapping hot paths followed `add()` with `indexOf()` to grab the
inserted/existing index. Both call into the same Map: `add` does a
has+set+get-equivalent, `indexOf` does a redundant get. Returning the
index from `add` lets callers get the result for free — one Map op per
source/name instead of two.

Affected:
  * SourceMapGenerator.addMapping
  * SourceMapGenerator.applySourceMap
  * IndexedSourceMapConsumer._parseMappings

Bench (SOLO=1 PHASES=adding generate vs main, two-process):
  addMapping mean +21.4% across jridgewell fixtures
  (preact +30.3%, react +21.4%, amp +20.5%, issue-41 +34.8%,
   vscode +21.1%, babel.min +0.2% — flat, very few unique sources)

API: ArraySet.add gains a return value. Existing callers ignoring it
keep working unchanged.
@7rulnik 7rulnik merged commit 960454d into main May 10, 2026
3 checks passed
@7rulnik 7rulnik deleted the perf/arrayset-add-returns-idx branch May 10, 2026 20:59
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