Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kGCTypeMinorMarkCompact mapping is missing on pref_hooks "kind" #48239

Open
artur-ma opened this issue May 29, 2023 · 1 comment
Open

kGCTypeMinorMarkCompact mapping is missing on pref_hooks "kind" #48239

artur-ma opened this issue May 29, 2023 · 1 comment
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.

Comments

@artur-ma
Copy link

artur-ma commented May 29, 2023

Version

18

Platform

No response

Subsystem

pref_hooks

What steps will reproduce the bug?

Looks like GC operation with type "kGCTypeMinorMarkCompact" will never be observed
according to the v8 docs
https://v8docs.nodesource.com/node-18.2/d2/dc3/namespacev8.html#a83be2143eef5fa2b17127fd69686496b
there are 5 GCTypes

kGCTypeScavenge |  
kGCTypeMinorMarkCompact |  
kGCTypeMarkSweepCompact |  
kGCTypeIncrementalMarking |  
kGCTypeProcessWeakCallbacks |  
~~kGCTypeAll~~

But pref hooks is mapping only 4 of them

node/src/node_perf.h

Lines 63 to 68 in 191dce8

enum PerformanceGCKind {
NODE_PERFORMANCE_GC_MAJOR = v8::GCType::kGCTypeMarkSweepCompact,
NODE_PERFORMANCE_GC_MINOR = v8::GCType::kGCTypeScavenge,
NODE_PERFORMANCE_GC_INCREMENTAL = v8::GCType::kGCTypeIncrementalMarking,
NODE_PERFORMANCE_GC_WEAKCB = v8::GCType::kGCTypeProcessWeakCallbacks
};

same here

node/src/node_v8.cc

Lines 241 to 252 in 191dce8

static const char* GetGCTypeName(v8::GCType gc_type) {
switch (gc_type) {
case v8::GCType::kGCTypeScavenge:
return "Scavenge";
case v8::GCType::kGCTypeMarkSweepCompact:
return "MarkSweepCompact";
case v8::GCType::kGCTypeIncrementalMarking:
return "IncrementalMarking";
case v8::GCType::kGCTypeProcessWeakCallbacks:
return "ProcessWeakCallbacks";
default:
return "Unknown";

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

No response

What do you see instead?

I would expect this type to be reported also
same as other third-party monitoring tools have

https://github.com/DataDog/dd-native-metrics-js/blob/main/src/metrics/GarbageCollection.hpp#L37

Additional information

No response

@kvakil kvakil added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label May 30, 2023
@kvakil
Copy link
Contributor

kvakil commented May 30, 2023

Note that this is only emitted if the V8 flag --minor-mc is set, which is pretty far from finished. But would probably be good to get ahead of this in case V8 enables it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

No branches or pull requests

2 participants