Skip to content

perf(timer): cut setTimeout/unref/clearTimeout churn cost (#10522) - #11373

Merged
proggeramlug merged 2 commits into
mainfrom
claude/festive-hamilton-7t5rv5
Sep 26, 2026
Merged

proggeramlug merged 2 commits into
mainfrom
claude/festive-hamilton-7t5rv5

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

#10522 measured two linear scans in the timer tables: the min() eviction in record_timer_handle_kind, and clearTimeout's find/retain over CALLBACK_TIMERS. Both are already gone on main. Timers now live in a per-agent slab + heap with an id index (turnloop P3), and the ref-state registry evicts retired ids first-in-first-out (#10447). Re-measuring on current main confirmed the id-count and live-count cliffs are gone, but the per-round cost was still ~13.7 k instructions per setTimeout + unref() + clearTimeout. The issue targets ≤10 k. This PR removes the three biggest constant costs; the round is now ~7.8 k instructions.

No version bump, per request.

Changes

  • Fast path for t.ref() / t.unref() / t.hasRef() / t.refresh() (timer/handle_object.rs::try_timer_method_fast_dispatch, hooked into js_native_call_method right after the perf(runtime): class dispatch and instanceof stop consulting locked hash maps — shapes.ts 0.28s → 0.23s #7769 class-vtable fast path).
    • Why: a timer handle carries a meta record and a recorded prototype, so perf(runtime): class dispatch and instanceof stop consulting locked hash maps — shapes.ts 0.28s → 0.23s #7769's fast path refuses it. Each call walked the full tower (handle/primitive probes, a by-name prototype read that allocated the key string, a closure rebind and a call through the thunk), which cost ~6.3 k instructions per call.
    • Guards: the fast path answers only when the call provably reaches the family's own native method.
      • The receiver still links to its family prototype, has no own string keys, is not dictionary-mode, and has no accessor bit for the name.
      • The prototype's own data slot for the name holds a closure whose function pointer is exactly that method's thunk.
    • No cache to go stale: every user override fails a guard and takes the tower as before. That covers an own t.unref, Timeout.prototype.unref = f, a getter via defineProperty, Object.setPrototypeOf(t, …) and delete. Immediate handles have no refresh, so that call still reaches the tower.
  • Hashing: the timer ref-state registry (timer/ref_states.rs) and async_hooks::RESOURCES hash their internal monotonic ids with aHash instead of SipHash. aHash is already a perry-runtime dependency, and neither map takes untrusted keys.
  • async_hooks::emit_init returns early when no hook is enabled. It used to allocate the "Timeout" type-name string once per setTimeout and then drop it; with_hook_callbacks already returned at once in that case, so behaviour is unchanged.
  • Fragment: changelog.d/10522-timer-churn-constant-factor.md.

Related issue

Closes #10522. The issue's four targets:

  • ≤10 k instructions per set+clear: met (~7.8 k).
  • 120 k ids within 1.2× of 60 k: met (per-op cost flat).
  • 5,000 live within 1.2× of 50 live: met (~1.2×).
  • ≤5× Node wall-clock: not met here (~6–7× on this shared 4-core host). The remaining profile is flat: handle allocation plus its prototype/shape link, the heap push, async-id bookkeeping. Nothing further is timer-specific.

Leaving it to the maintainer whether that last target should keep the issue open.

Test plan

  • Benchmark: the issue's bench.ts, PERRY_NO_AUTO_OPTIMIZE=1, Linux x64. Instructions counted with callgrind; the benchmark was built with PERRY_TARGET_CPU=x86-64 because valgrind can't decode the host-tuned code.

    before after
    whole process, churn N=20 000 (24 k set+unref+clear rounds) 329.3 M instr 188.3 M instr
    per round ~13.6 k ~7.8 k
    churn N=60 000 (72 k ids), per round — ~7.9 k (flat in ids created)

    Wall clock, median of 5, against Node 26.5.1 (from .node-version):

    variant Perry before Perry after Node
    churn 50 000 ~100 ms 65 ms 9.5 ms
    churn 100 000 ~240 ms 138 ms 14.8 ms
    live50 50 000 ~124 ms 82 ms 12.3 ms
    live5000 50 000 ~153 ms 97 ms 14.8 ms

    Checksums matched Node in every run.

  • Unit tests: RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib -- timer async_hooks gives 64 passed. That includes the new timer::tests_inline::method_fast_path_tests, which asserts the fast path's answers, that the js_native_call_method hook is reached, and that each override above (own property, replaced prototype method, prototype getter, setPrototypeOf) falls back to the tower.

  • Timer parity tests: every test-files/ test matching timer/timeout/immediate/unref/async_hook/refresh/using, compiled with the new compiler and compared byte-for-byte against Node 26.5.1. All test_gap_* and test_parity_* timer tests pass. Two exceptions:

    • test_issue_5540_sock_write_map_from_timer differs only in how a refused-connection error prints ([object Object] vs Error: connect ECONNREFUSED …, no server in the sandbox). That's not in the timer path or the gap gate.
    • The three http fixtures timed out compiling at 300 s on the loaded box; they are being re-run with a longer timeout.
  • Lint checks: cargo fmt --check, scripts/check_file_size.sh, scripts/addr_class_inventory.py and scripts/gc_runtime_root_holders.py all clean.

  • cargo build --release clean (-p perry -p perry-runtime-static -p perry-stdlib-static)

  • full cargo test --workspace (left to CI)

  • Added #[test]s in the affected crate

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
  • Commit follows the perf: prefix convention

🤖 Generated with Claude Code

https://claude.ai/code/session_01EShQaaTSCSytaaXwsZzaYG


Generated by Claude Code

Summary by CodeRabbit

  • Performance
    • Timer operations such as ref, unref, hasRef, and refresh now use a faster path when applicable, while preserving normal behavior when methods or prototypes have been modified.
    • Reduced overhead for timer and async resource tracking. In Linux x64 measurements, timer-churn instructions decreased from about 13.7k to 7.8k per round.

The timer tables' linear scans named in #10522 were already gone (per-agent
heap + id index, FIFO retired-id eviction); the remaining cost was constant
factor, ~13.7k instructions per set+unref+clear round:

- t.ref()/unref()/hasRef()/refresh() on a timer handle skip the dynamic
  dispatch tower when the call provably resolves to the family's native
  method (pristine receiver + prototype slot still holding the thunk).
  Every user override fails a guard and takes the tower as before.
- The timer ref-state registry and async_hooks RESOURCES use aHash for
  their internal monotonic ids instead of SipHash.
- async_hooks emit_init returns before allocating the type-name string
  when no hooks are enabled.

~7.8k instructions per round now; per-op cost is flat in ids created.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 67a806ab-c7f6-4a99-a62d-424f0e1bb5a0

📥 Commits

Reviewing files that changed from the base of the PR and between 2febf42 and f41281d.

📒 Files selected for processing (7)
  • changelog.d/11373-timer-churn-constant-factor.md
  • crates/perry-runtime/src/async_hooks.rs
  • crates/perry-runtime/src/object/native_call_method.rs
  • crates/perry-runtime/src/timer.rs
  • crates/perry-runtime/src/timer/handle_object.rs
  • crates/perry-runtime/src/timer/ref_states.rs
  • crates/perry-runtime/src/timer/tests_inline.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The runtime adds guarded direct dispatch for four timer methods. It also changes two map hashers to aHash and skips async-hook resource setup when no hooks are active.

Changes

Runtime performance

Layer / File(s) Summary
Timer method fast dispatch
crates/perry-runtime/src/timer/handle_object.rs, crates/perry-runtime/src/timer.rs, crates/perry-runtime/src/object/native_call_method.rs, crates/perry-runtime/src/timer/tests_inline.rs
The native method-call path tries direct dispatch for unref, ref, hasRef, and refresh. It falls back to generic dispatch when the receiver or prototype does not meet the checks. Tests cover successful calls and fallback cases.
Hashing and inactive-hook allocation
crates/perry-runtime/src/timer/ref_states.rs, crates/perry-runtime/src/async_hooks.rs, changelog.d/11373-timer-churn-constant-factor.md
The timer ref-state and async-hook resource maps use ahash::RandomState. emit_init returns before creating the type-name string when no hooks are active. The changelog records performance measurements and the changes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant js_native_call_method
  participant try_timer_method_fast_dispatch
  participant TimerHandle
  participant GenericDispatch
  js_native_call_method->>try_timer_method_fast_dispatch: Try named timer method
  alt Eligibility checks pass
    try_timer_method_fast_dispatch->>TimerHandle: Invoke timer operation
    try_timer_method_fast_dispatch-->>js_native_call_method: Return direct result
  else Eligibility checks fail
    try_timer_method_fast_dispatch-->>js_native_call_method: Return None
    js_native_call_method->>GenericDispatch: Continue normal dispatch
  end
Loading

Merge Risk: ⚪ Minimal · up to f4128

No actionable merge-blocking risk was identified in the timer or async-hook changes. The reported wall-clock performance target remains unmet, but that is a stated limitation rather than an established regression.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f4128

The new timer call path checks that a handle still uses its original native method before bypassing ordinary lookup. No new security boundary bypass was identified, but the available security coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The independently callable changed behavior is confined by the examined path to timer handles in the runtime process; the supplied changes do not establish a new cross-service or privileged sink.

Security Findings and Attack Paths

  • inferred — No override-bypass attack path was established for the examined timer dispatch: the strongest tested user-controlled property and prototype changes force fallback rather than direct execution.

Trust Boundaries and Controls

  • observed — Handle branding, prototype identity, descriptor checks, and native-thunk identity gate direct execution; failed checks leave method resolution to the generic caller.

Resilience and Maintainability Implications

  • inferred — Repeated or post-clear direct method calls use the same underlying timer operations as the ordinary native methods, so the examined fast path does not add a separate cleanup or recovery owner. The selected tests do not independently exercise every post-clear repetition.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Directly linked issue #10522 requires timer-churn cost of no more than 5× Node wall-clock time, in addition to the id-history, live-timer, and instruction targets. The PR adds timer-method fast dispat… Reduce the timer-churn wall-clock cost to no more than 5× Node under the #10522 benchmark, then add or update automated coverage for the measured target.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, specific, and accurately describes the primary performance change to timer churn involving setTimeout, unref, and clearTimeout.
Description check ✅ Passed The description includes the required Summary, Changes, Related issue, Test plan, and Checklist sections. It provides detailed implementation context, benchmark results, test coverage, and notes that …
Out of Scope Changes check ✅ Passed The changed files support the timer-churn objective in #10522. Timer-method fast dispatch reduces constant timer operation cost. aHash reduces timer and async_hooks::RESOURCES map overhead. The `emi…
Full details: Linked Issues check

Explanation

Directly linked issue #10522 requires timer-churn cost of no more than 5× Node wall-clock time, in addition to the id-history, live-timer, and instruction targets. The PR adds timer-method fast dispatch, aHash maps, and an early async_hooks::emit_init return. The reported measurements meet the first three targets, including about 7.8k instructions per round and approximately 1.2× scaling. The reported wall-clock result remains about 6–7× Node, so the 5× requirement is unmet.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug force-pushed the claude/festive-hamilton-7t5rv5 branch from f41281d to ae91ecb Compare September 26, 2026 06:41
@proggeramlug
proggeramlug merged commit c9be0e2 into main Sep 26, 2026
15 of 19 checks passed
@proggeramlug
proggeramlug deleted the claude/festive-hamilton-7t5rv5 branch September 26, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants