Skip to content

perf_hooks: fix truncation of uvMetricsInfo counters - #66094

Open
jasnell wants to merge 3 commits into
nodejs:mainfrom
jasnell:jasnell/perf-hooks-uvmetricsinfo-bigint
Open

jasnell wants to merge 3 commits into
nodejs:mainfrom
jasnell:jasnell/perf-hooks-uvmetricsinfo-bigint

Conversation

@jasnell

@jasnell jasnell commented Sep 17, 2026

Copy link
Copy Markdown
Member

libuv reports the event loop metrics exposed through
performance.nodeTiming.uvMetricsInfo as uint64_t counters, but
they were copied into an Int32Array (and, before that, converted
using v8::Integer::New()), so they wrapped around after 2^31 on
long-running processes. While such large numbers aren't typical,
they are possible, making the current implementation flawed.

This PR stores the metrics in a Float64Array instead. Values are
now exact up to Number.MAX_SAFE_INTEGER.

We also introduce a parallel uvMetricsInfoBigInt whose values are
represented as bigint to represent the fully correct value.

Note that the corrected handling using Float64Array vs. Int32Array
adds roughly 1-6ns per call overhead to the fact that v8 must create
a HeapNumber from the Float64Array element while the Int32Array
allows it to use SMI. Given the typical usage pattern here, however,
6ns is unlikely to be noticed but it will show up in the unrealistic
microbenchmark.

libuv reports the event loop metrics exposed through
`performance.nodeTiming.uvMetricsInfo` as `uint64_t` counters, but
they were copied into an `Int32Array` (and, before that, converted
using `v8::Integer::New()`), so they wrapped around after 2^31 on
long-running processes.

Store the metrics in a `Float64Array` instead. Values are now exact
up to `Number.MAX_SAFE_INTEGER`.

Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
`performanceNodeTiming.uvMetricsInfo` is a property, not a method, so
document its value using `Type:` rather than `Returns:`.

Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 17, 2026
@jasnell
jasnell requested a review from mcollina September 17, 2026 21:59
`performance.nodeTiming.uvMetricsInfo` returns the libuv event loop
metrics as numbers, which are only exact up to
`Number.MAX_SAFE_INTEGER`. Add `uvMetricsInfoBigInt`, which returns
the same metrics as bigints backed by `uint64_t` storage, carrying the
full 64-bit range reported by libuv.

A single native call fills both a `Float64Array` and a
`BigUint64Array`, so `uvMetricsInfo` does not pay for bigint
allocation and conversion. The new property is omitted from
`toJSON()`, as `JSON.stringify()` cannot serialize bigints.

Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell
jasnell force-pushed the jasnell/perf-hooks-uvmetricsinfo-bigint branch from f013265 to fbab3f2 Compare September 17, 2026 22:41
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (2dd7e7b) to head (fbab3f2).
⚠️ Report is 28 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66094      +/-   ##
==========================================
- Coverage   92.80%   90.28%   -2.52%     
==========================================
  Files         420      790     +370     
  Lines      190225   271630   +81405     
  Branches    29133    51833   +22700     
==========================================
+ Hits       176540   245249   +68709     
- Misses      13359    16886    +3527     
- Partials      326     9495    +9169     
Files with missing lines Coverage Δ
lib/internal/perf/nodetiming.js 94.89% <100.00%> (+0.57%) ⬆️
src/aliased_buffer.h 100.00% <ø> (ø)
src/node_perf.cc 88.14% <100.00%> (ø)
src/node_snapshotable.cc 73.74% <100.00%> (ø)

... and 501 files with indirect coverage changes

🚀 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.

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 18, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 18, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants