Skip to content

lib,src: improve histogram implementation - #65024

Open
jasnell wants to merge 4 commits into
nodejs:mainfrom
jasnell:jasnell/histogram-improvements
Open

lib,src: improve histogram implementation#65024
jasnell wants to merge 4 commits into
nodejs:mainfrom
jasnell:jasnell/histogram-improvements

Conversation

@jasnell

Copy link
Copy Markdown
Member

Several improvements:

  1. In histogram-inl, Add previous locked only this->mutex while
    reading the other's fields unsafely.
  2. In histogram.cc, PrepareCB now uses ContainerOf
  3. In histogram.cc, BigInt value range is checked
  4. In histogram.js, simplified impl and reduced duplication
  5. In event_loop_delay.js, use a more consistent constructor
  6. Add additional common histogram analysis operations

There is one potentially/technically breaking change. In Record, we were previously silently truncating when Bigint conversion was lossy. Now it throws. This is technically a bug fix. It's exceedingly unlikely that anyone is depending on the former behavior so I'd rather land this as a minor.

Signed-off-by: James M Snell jasnell@gmail.com
Assisted-by: Opencode/Opus

Several improvements:
1. In histogram-inl, Add previous locked only this->mutex while
reading the other's fields unsafely.
2. In histogram.cc, PrepareCB now uses ContainerOf
3. In histogram.cc, BigInt value range is checked
4. In histogram.js, simplified impl and reduced duplication
5. In event_loop_delay.js, use a more consistent constructor
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode/Opus
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode/Opus
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode/Opus
Adds new analytical APIs to Histogram
* histogram.ccdf(value)
* histogram.cdf(value)
* histogram.countAt(value)
* histogram.ksTest(other)
* histogram.kurtosis
* histogram.linearBuckets(stepSize)
* histogram.logBuckets(first, base)
* histogram.percentilesAt(percentiles)
* histogram.shewness
On RecordableHistogram
* histogram.recordCorrected(val, expectedInterval)
* histogram.subtract(other)
Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnelljasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 4, 2026
@nodejs-github-botnodejs-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 4, 2026
@codecov

codecovBot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.45578% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (1576cb8) to head (4c5ff48).
⚠️ Report is 10 commits behind head on main.

Files with missing linesPatch %Lines
src/histogram.cc86.03%22 Missing and 21 partials ⚠️
src/histogram-inl.h76.27%8 Missing and 6 partials ⚠️
lib/internal/perf/event_loop_delay.js72.72%3 Missing ⚠️
lib/internal/histogram.js98.97%2 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65024 +/- ##
========================================
Coverage 90.30% 90.31% ========================================
Files 759 759 Lines 247634 248121 +487 Branches 46684 46792 +108 ========================================
+ Hits 223630 224081 +451 - Misses 15467 15480 +13 - Partials 8537 8560 +23 
Files with missing linesCoverage Δ
src/histogram.h68.42% <100.00%> (+18.42%)⬆️
lib/internal/histogram.js94.77% <98.97%> (+4.62%)⬆️
lib/internal/perf/event_loop_delay.js94.89% <72.72%> (-1.07%)⬇️
src/histogram-inl.h81.30% <76.27%> (-1.13%)⬇️
src/histogram.cc82.30% <86.03%> (+1.06%)⬆️

... and 31 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.

Sign up for freeto 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 / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.semver-minorPRs 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.

2 participants

@jasnell@nodejs-github-bot