Skip to content

fix(perf_hooks): complete timerify parity - #8276

Merged
proggeramlug merged 2 commits into
mainfrom
fix/8234-timerify-parity
Aug 17, 2026
Merged

fix(perf_hooks): complete timerify parity#8276
proggeramlug merged 2 commits into
mainfrom
fix/8234-timerify-parity

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • defer performance.timerify() entries and histogram samples until returned promises settle
  • preserve constructor behavior, call arguments, result identity, and class-call errors through timerify wrappers
  • expose Node-compatible entry argument aliases and wrapper name/length descriptors
  • split timerify into a topical submodule to keep the runtime source within the repository's file-size limit

Testing

  • cargo fmt --all -- --check
  • cargo check -p perry-runtime
  • cargo build --release -p perry-runtime-static
  • all 14 node-suite/perf_hooks/timerify parity fixtures with PERRY_SKIP_BUILD=1
  • ./scripts/pre-tag-check.sh --quick

Closes #8234

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

Limit details: You’ve used all 8 included reviews currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 921bd613-a32e-47b9-b49c-38c458693618

📥 Commits

Reviewing files that changed from the base of the PR and between 259a225 and d8dc4a5.

📒 Files selected for processing (6)
  • changelog.d/8276-perf-hooks-timerify.md
  • crates/perry-runtime/src/object/descriptors.rs
  • crates/perry-runtime/src/object/native_call_method/common_methods.rs
  • crates/perry-runtime/src/object/object_ops/has_own.rs
  • crates/perry-runtime/src/perf_hooks.rs
  • crates/perry-runtime/src/perf_hooks/timerify.rs

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 marked this pull request as ready for review August 17, 2026 03:16
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Merging. Runtime suite 2567 passed / 0 failed, all six script gates and check_file_size clean, cargo fmt clean, and the node-suite timerify fixtures run cleanly under the pinned oracle.

Extracting timerify.rs is the right call and it fixes a problem I flagged on #8254. That PR left perf_hooks.rs at 1998 lines — two under the 2000-line cap, so the next addition would have tripped check_file_size.sh. This takes it to 1920 by moving 204 lines into a submodule, which is the split I suggested there. More of that file should go the same way.

One observation, not a blocker: the runtime suite contains zero tests matching timerify, so the behaviour added here is covered only by the node-suite fixtures, which are nightly-only and floor-gated. That means a regression in this code has no per-PR signal — the same structural gap #8269 has. Worth a unit test or two on the settle/pending record transitions if this area keeps growing; the oracle output (pending records: 0settled records: 1) suggests there is a clean state machine to assert on.

Unrelated, but discovered while auditing this PR and now fixed on main: gc_runtime_root_holders was failing here, and it was not this PR — #8270 (Windows MapView) added two thread-locals to map_view.rs, pinned MAPS, but missed its #[cfg(target_os = "windows")] sibling HWND_TO_HANDLE, so the census had been red on main since that merge. That is my miss: I ran cargo-audit, the perry tests and fmt on #8270 but not the census gate. Pinned it on main — the value is a 1-based widget handle rather than a NaN-boxed callback, so it cannot park a closure and pinning is the right disposition rather than a scanner.

@proggeramlug
proggeramlug merged commit 1e52858 into main Aug 17, 2026
25 of 33 checks passed
@proggeramlug
proggeramlug deleted the fix/8234-timerify-parity branch August 17, 2026 03:41
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.

[parity] node:perf_hooks timerify — async settlement, construct, entry arguments, and name/length descriptors

1 participant