Skip to content

perf_hooks: add binary export/import to Histogram - #65434

Open
jasnell wants to merge 2 commits into
nodejs:mainfrom
jasnell:jasnell/more-histogram-fun-2
Open

perf_hooks: add binary export/import to Histogram#65434
jasnell wants to merge 2 commits into
nodejs:mainfrom
jasnell:jasnell/more-histogram-fun-2

Conversation

@jasnell

@jasnell jasnell commented Aug 20, 2026

Copy link
Copy Markdown
Member

Stacks on: #65416

Pay attention only to the last two commits.

@jasnell
jasnell requested review from Qard and mcollina August 20, 2026 14:32
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/tsc

@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 related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 20, 2026
@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch from 00936d8 to fbcb489 Compare August 20, 2026 15:51
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.02825% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (00c80a8) to head (b784466).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/histogram.cc 83.90% 9 Missing and 43 partials ⚠️
lib/internal/histogram.js 96.55% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #65434    +/-   ##
========================================
  Coverage   90.13%   90.14%            
========================================
  Files         751      751            
  Lines      253439   253789   +350     
  Branches    47730    47808    +78     
========================================
+ Hits       228448   228775   +327     
+ Misses      16256    16239    -17     
- Partials     8735     8775    +40     
Files with missing lines Coverage Δ
lib/perf_hooks.js 100.00% <100.00%> (ø)
src/histogram.h 72.72% <ø> (+4.30%) ⬆️
lib/internal/histogram.js 96.11% <96.55%> (+0.01%) ⬆️
src/histogram.cc 84.22% <83.90%> (-0.06%) ⬇️

... and 36 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 force-pushed the jasnell/more-histogram-fun-2 branch from fbcb489 to 5d9f7ce Compare August 20, 2026 19:20
@jasnell
jasnell requested a review from legendecas August 20, 2026 20:30
@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch from 5d9f7ce to dfaacaf Compare August 20, 2026 20:36
@jasnell jasnell added notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. labels Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @jasnell.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@jasnell

jasnell commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Notes for the notable change:

This PR gives Histogram a new binary interchange format

@jasnell jasnell added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Aug 20, 2026
Comment thread src/histogram.cc
Comment thread src/histogram.cc Outdated
@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch from dfaacaf to 28eb138 Compare August 20, 2026 22:54
@legendecas

Copy link
Copy Markdown
Member
require('node:perf_hooks').importHistogram(new Uint8Array([
  0xa3,                                            // map(3)
  0x01, 0x1b, 0,0,0x20,0,0,0,0,0,                  // 1 (lowest)  = 2**45
  0x02, 0x1b, 0,0,0x40,0,0,0,0,0,                  // 2 (highest) = 2**46
  0x03, 0x05,                                      // 3 (figures) = 5
]));

This still bypasses the checks and triggers assert on hdr_init retcode. I wonder if this check on hdr_init can be converted to be a JS error.

@jasnell

jasnell commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Ugh. yeah, will have to make the validation more thorough.. will update shortly

@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch from 28eb138 to 9b57e4f Compare August 21, 2026 14:10
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch 2 times, most recently from af64f27 to cc71fac Compare August 22, 2026 23:09
Provide a binary export/import mechanism for histograms.

Since there is no standard interchange format for histograms,
using CBOR is meant to make the format as platform/runtime
agnostic as possible while producing a compact/efficient
result. No new dependency is introduced, we just encode
the CBOR directly.

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode/Opus
Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell
jasnell force-pushed the jasnell/more-histogram-fun-2 branch from cc71fac to b784466 Compare August 23, 2026 20:31
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@jasnell jasnell added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants