Skip to content

fix(perf): cache lengths/locals in more hot loops#225

Merged
alexander-akait merged 7 commits intomainfrom
claude/test-perf-improvements-JXkUd
Apr 20, 2026
Merged

fix(perf): cache lengths/locals in more hot loops#225
alexander-akait merged 7 commits intomainfrom
claude/test-perf-improvements-JXkUd

Conversation

@alexander-akait
Copy link
Copy Markdown
Member

more perf improvements

Extends the earlier perf pass (#224) to the remaining hot spots:

- HookCodeFactory.contentWithInterceptors: short-circuit the
  no-interceptor case up front (skip the closure-wrapping and the
  Object.assign on options); hoist the `interceptors` array and its
  length to locals so the inner onError/onResult/onDone closures don't
  re-read them on every iteration.
- HookCodeFactory.callTap: cache `this.options.taps` and
  `this.options.interceptors` once at the top.
- HookCodeFactory.callTapsLooping: inline Array#every to drop the
  per-call callback allocation; cache interceptors length.
- HookCodeFactory.needContext: cache taps length.
- Hook.intercept: cache `taps`, its length, and `interceptor.register`
  so the re-registration loop stops doing three property lookups per
  iteration.
- MultiHook.{tap,tapAsync,tapPromise,isUsed,intercept}: cache
  `hooks.length` so the loop condition doesn't re-read it each pass.

Benchmarks (npm run benchmark, --no-opt --predictable):

  hook-map for(new key) x 10, no interceptors  174k -> 226k ops/s (+30%)
  hook-map for(new key) x 10, 3 interceptors   157k -> 190k ops/s (+21%)
  multi-hook intercept across 3 hooks          162k -> 178k ops/s (+10%)
  multi-hook tap x 10 across 3 hooks           103k -> 109k ops/s (+6%)
  interceptors-sync call interceptor          6.46k -> 6.93k ops/s (+7%)
  interceptors-sync register + 10 taps         255k -> 267k ops/s (+5%)

All 126 jest tests pass.

https://claude.ai/code/session_01863WjdJD1wESVQcdrqgoJL
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 20, 2026

⚠️ No Changeset found

Latest commit: 1729bb4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 20, 2026

CLA Not Signed

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.09%. Comparing base (9e9ae4d) to head (1729bb4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   98.96%   99.09%   +0.12%     
==========================================
  Files          15       15              
  Lines         776      775       -1     
  Branches      130      132       +2     
==========================================
  Hits          768      768              
+ Misses          8        7       -1     
Flag Coverage Δ
integration 99.09% <100.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 20, 2026

Merging this PR will not alter performance

✅ 92 untouched benchmarks


Comparing claude/test-perf-improvements-JXkUd (1729bb4) with main (9e9ae4d)

Open in CodSpeed

claude added 6 commits April 20, 2026 15:48
Three changes in HookCodeFactory that actually reduce allocations on
the compile path (visible on CodSpeed's instruction-count runs, where
the previous length-caching tweaks were not):

- init(): stop calling `options.args.slice()`. `_args` is only read
  (length, join, indexing) by this class and its subclasses - never
  mutated - so sharing the caller's array is safe and saves a copy
  on every compile.
- args(): the `before` / `after` branches used to allocate
  `[before, ...allArgs]` and/or `[...allArgs, after]`, then re-join
  them. The no-customization result is already cached as
  `_joinedArgs`; the customized variants can be built by concatenating
  that cached string, avoiding both the array spread and the second
  join. This runs for every tap (async gets `after: "_callback"`, and
  interceptors with context get `before: "_context"`), so it
  accumulates.
- create(): read `options.type` from the parameter instead of
  re-reading `this.options.type` (set one line earlier by init()).

Generated code output is byte-identical - all 88 HookCodeFactory
snapshots still pass - so no hooks library consumers see any change.

https://claude.ai/code/session_01863WjdJD1wESVQcdrqgoJL
Three independent allocation reductions:

- HookCodeFactory.setup(): swap `Array.from({length})` for
  `new Array(length)`. The former walks the array-like protocol and
  writes `undefined` into every slot before the loop immediately
  overwrites it; the latter just reserves the backing store.

- HookCodeFactory.contentWithInterceptors(): assign directly to
  `options.onError`/`onResult`/`onDone` instead of
  `Object.assign(options, {…})`. Avoids allocating the RHS literal
  (which is thrown away after the merge) on every compile that has
  interceptors.

- Hook._insert(): fast path for single-name `before`. When
  `item.before` is `"name"` (or `["name"]`) we now hold the name in
  a string and do `tap.name === beforeName` instead of building a
  Set just to call `.has()` once per walk step. The existing Set
  path is kept for multi-name before.

Behavior unchanged: all 126 jest tests and 88 snapshots still pass,
generated code is byte-identical.

https://claude.ai/code/session_01863WjdJD1wESVQcdrqgoJL
The previous commit added a single-string `before` fast path to
_insert but routed the stage-only path (no `before`, hit by
`tap({stage})` insertions like the `with stages` bench) through the
same loop, which then had to do `beforeName !== undefined` and
`before !== undefined` checks on every iteration. That showed as a
regression on CodSpeed instruction counts.

Fork the walk into two variants:
- No `before`: pure stage walk with zero extra branches - matches
  pre-fast-path instruction count on that path.
- Has `before`: keeps the beforeName / Set logic.

Also drop the `before: ["x"]` -> beforeName coercion. Single-element
`before` arrays are rare; removing the special case simplifies the
setup block and the generated hidden class.

All 126 jest tests and 88 snapshots still pass, lint clean.

https://claude.ai/code/session_01863WjdJD1wESVQcdrqgoJL
@alexander-akait alexander-akait merged commit 2ba184c into main Apr 20, 2026
33 of 34 checks passed
@alexander-akait alexander-akait deleted the claude/test-perf-improvements-JXkUd branch April 20, 2026 17:50
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.

2 participants