Skip to content

perf_hooks: add NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP constant#63877

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
szegedi:feat/perf-hooks-gc-minor-mark-sweep
Jun 23, 2026
Merged

perf_hooks: add NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP constant#63877
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
szegedi:feat/perf-hooks-gc-minor-mark-sweep

Conversation

@szegedi

@szegedi szegedi commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

V8's Minor Mark-Sweep collector (enabled with --minor-ms, the default young-generation collector since V8 12.x / Node 22) reports garbage collection performance entries with kind kGCTypeMinorMarkSweep (value 2). perf_hooks exposed constants for every other GC kind (major, minor, incremental, weakcb) but not this one, so consumers inspecting performanceEntry.detail.kind had no constant to compare against and saw an unmapped value.

With this PR I expose it as perf_hooks.constants.NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP, mirroring the existing v8::GCType mappings, and document it alongside the other GC kinds.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 12, 2026
@BridgeAR BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 12, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 12, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

obs.observe({ entryTypes: ['gc'] });

globalThis.gc({ type: 'minor' });
// Keep the event loop alive to witness the GC async callback happen.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind converting this to gcUntil from test/common/gc.js, reducing the flakiness on the gc timing?

This can be like:

let observed = fasel;
const obs = new PerformanceObserver(common.mustCallAtLeast((list) => {
   ...
   observed = true;
}));

gcUntil('minor gc event', () => observed, 10, { type: 'minor' });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion – done; thanks!

@szegedi szegedi force-pushed the feat/perf-hooks-gc-minor-mark-sweep branch from 35dac1a to 5caa3d0 Compare June 15, 2026 08:43
@szegedi

szegedi commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

force push merely amended the commit message to include the Signed-off-by trailer

V8's Minor Mark-Sweep collector, enabled with the --minor-ms flag
(available since Node.js 22), reports garbage collection performance
entries with kind kGCTypeMinorMarkSweep (value 2). perf_hooks exposed
constants for every other GC kind (major, minor, incremental, weakcb)
but not this one, so consumers inspecting performanceEntry.detail.kind
had no constant to compare against and saw an unmapped value.

Expose it as perf_hooks.constants.NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP,
mirroring the existing v8::GCType mappings, and document it alongside
the other GC kinds.

Signed-off-by: Attila Szegedi <attila.szegedi@datadoghq.com>
@szegedi szegedi force-pushed the feat/perf-hooks-gc-minor-mark-sweep branch from 16d1ebc to 9880e42 Compare June 15, 2026 09:02
@szegedi

szegedi commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

There's one failing CI job (x86_64-linux: with shared libraries.) It looks unrelated to this change. It aborted in test/addons/worker-addon-exit/test.js with:

  Assertion failed: (env) != nullptr
    at ../../src/api/hooks.cc:130  node::RemoveEnvironmentCleanupHook(...)
    ← MyObject::~MyObject()  (during worker/addon teardown)

Looks like a flake/teardown issue. Could someone with the necessary permissions re-run the failed job (and/or kick off CI)? I don't have access to re-run it myself. Thanks!

@BridgeAR BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 15, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 15, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

assert(entry);
assert.strictEqual(entry.name, 'gc');
assert.strictEqual(entry.entryType, 'gc');
assert.strictEqual(entry.detail.kind, NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP);

@legendecas legendecas Jun 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this callback could also be triggered by other minor GC types. This should be branched like:

  if (entry.detail.kind === NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP) {
    observed = true;
  }

The CI failure seems to be that a scheduled GC fired before the forced NODE_PERFORMANCE_GC_FLAGS_FORCED.

Signed-off-by: Attila Szegedi <attila.szegedi@datadoghq.com>
@szegedi szegedi force-pushed the feat/perf-hooks-gc-minor-mark-sweep branch from 9880e42 to 4a6995c Compare June 22, 2026 15:08
@legendecas legendecas added perf_hooks Issues and PRs related to the implementation of the Performance Timing API. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 22, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@legendecas legendecas added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 23, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 23, 2026
@nodejs-github-bot nodejs-github-bot merged commit 5075bb4 into nodejs:main Jun 23, 2026
90 of 91 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 5075bb4

luanmuniz pushed a commit to luanmuniz/node that referenced this pull request Jun 25, 2026
V8's Minor Mark-Sweep collector, enabled with the --minor-ms flag
(available since Node.js 22), reports garbage collection performance
entries with kind kGCTypeMinorMarkSweep (value 2). perf_hooks exposed
constants for every other GC kind (major, minor, incremental, weakcb)
but not this one, so consumers inspecting performanceEntry.detail.kind
had no constant to compare against and saw an unmapped value.

Expose it as perf_hooks.constants.NODE_PERFORMANCE_GC_MINOR_MARK_SWEEP,
mirroring the existing v8::GCType mappings, and document it alongside
the other GC kinds.

Signed-off-by: Attila Szegedi <attila.szegedi@datadoghq.com>
PR-URL: nodejs#63877
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Ilyas Shabi <ilyasshabi94@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
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 pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. perf_hooks Issues and PRs related to the implementation of the Performance Timing API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants