From 36e8d76476170de7522862c8efa1665cbd50f60d Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 11:50:13 -0400 Subject: [PATCH 1/8] fix incorrect gc types in newer versions of node --- package.json | 5 ++- src/metrics/GarbageCollection.hpp | 58 ++++++++++++++++++++++--------- test/metrics.spec.js | 3 ++ yarn.lock | 5 --- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/package.json b/package.json index 187c9e5..9abbf35 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "compile_commands": "node-gyp configure --release -- -f gyp.generator.compile_commands_json.py && mv Release/compile_commands.json ./", "rebuild": "node-gyp rebuild -j max", "lint": "node scripts/check_licenses.js && eslint .", - "test": "mocha 'test/**/*.spec.js' && node test/main" + "test": "mocha -n expose-gc 'test/**/*.spec.js' && node test/main" }, "keywords": [ "datadog", @@ -34,7 +34,6 @@ "eslint-plugin-node": "^11.1.0", "eslint-plugin-promise": "^5.1.0", "eslint-plugin-standard": "^5.0.0", - "mocha": "^9.0.3", - "nan": "^2.17.0" + "mocha": "^9.0.3" } } diff --git a/src/metrics/GarbageCollection.hpp b/src/metrics/GarbageCollection.hpp index c3f5cab..e11afd6 100644 --- a/src/metrics/GarbageCollection.hpp +++ b/src/metrics/GarbageCollection.hpp @@ -13,6 +13,7 @@ using Napi::Env; using Napi::Object; using Napi::Value; +using Napi::VersionManagement; namespace datadog { class GarbageCollection { @@ -29,25 +30,11 @@ namespace datadog { std::map pause_; std::map types_; uint64_t start_time_; + + const char* ToType(Env env, v8::GCType); }; GarbageCollection::GarbageCollection() { -#if NODE_MODULE_VERSION >= 108 - types_[1] = "scavenge"; - types_[2] = "minor_mark_compact"; - types_[4] = "mark_sweep_compact"; - types_[8] = "incremental_marking"; - types_[16] = "process_weak_callbacks"; - types_[31] = "all"; -#else - types_[1] = "scavenge"; - types_[2] = "mark_sweep_compact"; - types_[3] = "all"; - types_[4] = "incremental_marking"; - types_[8] = "process_weak_callbacks"; - types_[15] = "all"; -#endif - pause_[v8::GCType::kGCTypeAll] = Histogram(); start_time_ = uv_hrtime(); } @@ -91,10 +78,47 @@ namespace datadog { Value GarbageCollection::ToJSON(Env env) { Object gc = Object::New(env); + for (auto &it : pause_) { - gc.Set(types_[it.first], it.second.ToJSON(env)); + auto type = this->ToType(env, it.first); + gc.Set(type, it.second.ToJSON(env)); it.second.reset(); } return gc; } + + const char* GarbageCollection::ToType(Env env, v8::GCType type) { + auto version = VersionManagement::GetNodeVersion(env); + auto type_bit = static_cast(type); + + if (version->major >= 22) { + switch (type_bit) { + case 1: return "scavenge"; + case 2: return "minor_mark_sweep"; + case 4: return "mark_sweep_compact"; // Deprecated, might be removed soon. + case 8: return "incremental_marking"; + case 16: return "process_weak_callbacks"; + case 31: return "all"; + } + } else if (version->major >= 18) { + switch (type_bit) { + case 1: return "scavenge"; + case 2: return "minor_mark_compact"; + case 4: return "mark_sweep_compact"; + case 8: return "incremental_marking"; + case 16: return "process_weak_callbacks"; + case 31: return "all"; + } + } else { + switch (type_bit) { + case 1: return "scavenge"; + case 2: return "mark_sweep_compact"; + case 4: return "incremental_marking"; + case 8: return "process_weak_callbacks"; + case 15: return "all"; + } + } + + return "unknown"; + } } diff --git a/test/metrics.spec.js b/test/metrics.spec.js index 4463ad7..5ff7ac1 100644 --- a/test/metrics.spec.js +++ b/test/metrics.spec.js @@ -27,6 +27,8 @@ describe('metrics', () => { }) it('should collect stats', () => { + globalThis.gc() + const stats = nativeMetrics.stats() expect(stats).to.have.property('cpu') @@ -53,6 +55,7 @@ describe('metrics', () => { expect(stats).to.have.property('gc') expect(stats.gc).to.have.property('all') + expect(stats.gc).to.not.have.property('unknown') expect(stats.gc.all).to.have.property('min') expect(stats.gc.all.min).to.be.a('number') expect(stats.gc.all).to.have.property('max') diff --git a/yarn.lock b/yarn.lock index 7f2d862..fac24a2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1178,11 +1178,6 @@ ms@2.1.3: resolved "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz" integrity sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA== -nan@^2.17.0: - version "2.17.0" - resolved "https://registry.yarnpkg.com/nan/-/nan-2.17.0.tgz#c0150a2368a182f033e9aa5195ec76ea41a199cb" - integrity sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ== - nanoid@3.3.1: version "3.3.1" resolved "https://registry.npmjs.org/nanoid/-/nanoid-3.3.1.tgz" From 56c07fef480ef1ca9d0983d523237e4e690d848e Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 12:02:28 -0400 Subject: [PATCH 2/8] debug --- test/metrics.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/metrics.spec.js b/test/metrics.spec.js index 5ff7ac1..c373aab 100644 --- a/test/metrics.spec.js +++ b/test/metrics.spec.js @@ -53,6 +53,7 @@ describe('metrics', () => { expect(stats.eventLoop).to.have.property('p95') expect(stats.eventLoop.p95).to.be.a('number') + console.log(stats.gc) expect(stats).to.have.property('gc') expect(stats.gc).to.have.property('all') expect(stats.gc).to.not.have.property('unknown') From c95eb40aa38d1481839d62911adcce10e72cb495 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 12:13:33 -0400 Subject: [PATCH 3/8] debug --- src/metrics/GarbageCollection.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/metrics/GarbageCollection.hpp b/src/metrics/GarbageCollection.hpp index e11afd6..56c36d7 100644 --- a/src/metrics/GarbageCollection.hpp +++ b/src/metrics/GarbageCollection.hpp @@ -80,6 +80,7 @@ namespace datadog { Object gc = Object::New(env); for (auto &it : pause_) { + printf("%u\n", it.first); auto type = this->ToType(env, it.first); gc.Set(type, it.second.ToJSON(env)); it.second.reset(); From aa5ef7c4a7ff3dfab5687bcd59285e988cec6bf2 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 12:31:41 -0400 Subject: [PATCH 4/8] debug --- src/metrics/GarbageCollection.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/metrics/GarbageCollection.hpp b/src/metrics/GarbageCollection.hpp index 56c36d7..04bd5d6 100644 --- a/src/metrics/GarbageCollection.hpp +++ b/src/metrics/GarbageCollection.hpp @@ -92,6 +92,8 @@ namespace datadog { auto version = VersionManagement::GetNodeVersion(env); auto type_bit = static_cast(type); + printf("major: %u\n", version->major); + if (version->major >= 22) { switch (type_bit) { case 1: return "scavenge"; From 62701415f0d18cbe08aa3cd7e5b130d35ba5e413 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 13:10:03 -0400 Subject: [PATCH 5/8] debug --- test/metrics.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/metrics.spec.js b/test/metrics.spec.js index c373aab..cc58814 100644 --- a/test/metrics.spec.js +++ b/test/metrics.spec.js @@ -7,6 +7,7 @@ const nativeMetrics = require('..') describe('metrics', () => { beforeEach(() => { + console.log('js:', process.version) nativeMetrics.start() }) From 0925a74a653ad57df33393636c412925d6a81e82 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 15:10:42 -0400 Subject: [PATCH 6/8] fix all gc type regardless of runtime version --- src/metrics/GarbageCollection.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/metrics/GarbageCollection.hpp b/src/metrics/GarbageCollection.hpp index 04bd5d6..5d8fb97 100644 --- a/src/metrics/GarbageCollection.hpp +++ b/src/metrics/GarbageCollection.hpp @@ -28,6 +28,7 @@ namespace datadog { Value ToJSON(Env env); private: std::map pause_; + Histogram pause_all_; std::map types_; uint64_t start_time_; @@ -35,7 +36,6 @@ namespace datadog { }; GarbageCollection::GarbageCollection() { - pause_[v8::GCType::kGCTypeAll] = Histogram(); start_time_ = uv_hrtime(); } @@ -73,7 +73,7 @@ namespace datadog { } pause_[type].add(usage); - pause_[v8::GCType::kGCTypeAll].add(usage); + pause_all_.add(usage); } Value GarbageCollection::ToJSON(Env env) { @@ -85,6 +85,10 @@ namespace datadog { gc.Set(type, it.second.ToJSON(env)); it.second.reset(); } + + gc.Set("all", pause_all_.ToJSON(env)); + pause_all_.reset(); + return gc; } From bccbf7a7b581422340971cbc2a3ece5f4397f39f Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 24 Oct 2024 19:14:48 -0400 Subject: [PATCH 7/8] code cleanup --- src/metrics/GarbageCollection.hpp | 3 --- test/metrics.spec.js | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/metrics/GarbageCollection.hpp b/src/metrics/GarbageCollection.hpp index 5d8fb97..42d158e 100644 --- a/src/metrics/GarbageCollection.hpp +++ b/src/metrics/GarbageCollection.hpp @@ -80,7 +80,6 @@ namespace datadog { Object gc = Object::New(env); for (auto &it : pause_) { - printf("%u\n", it.first); auto type = this->ToType(env, it.first); gc.Set(type, it.second.ToJSON(env)); it.second.reset(); @@ -96,8 +95,6 @@ namespace datadog { auto version = VersionManagement::GetNodeVersion(env); auto type_bit = static_cast(type); - printf("major: %u\n", version->major); - if (version->major >= 22) { switch (type_bit) { case 1: return "scavenge"; diff --git a/test/metrics.spec.js b/test/metrics.spec.js index cc58814..5ff7ac1 100644 --- a/test/metrics.spec.js +++ b/test/metrics.spec.js @@ -7,7 +7,6 @@ const nativeMetrics = require('..') describe('metrics', () => { beforeEach(() => { - console.log('js:', process.version) nativeMetrics.start() }) @@ -54,7 +53,6 @@ describe('metrics', () => { expect(stats.eventLoop).to.have.property('p95') expect(stats.eventLoop.p95).to.be.a('number') - console.log(stats.gc) expect(stats).to.have.property('gc') expect(stats.gc).to.have.property('all') expect(stats.gc).to.not.have.property('unknown') From 949756d4f4625056f7b70dee5a72011377792411 Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 25 Oct 2024 15:20:30 -0400 Subject: [PATCH 8/8] start more workers in test --- test/main.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/main.js b/test/main.js index 1ff6ea7..b4bfad7 100644 --- a/test/main.js +++ b/test/main.js @@ -3,8 +3,10 @@ const path = require('path') const { Worker } = require('worker_threads') -const worker = new Worker(path.join(__dirname, 'worker.js')) +for (let i = 0; i < 100; i++) { + const worker = new Worker(path.join(__dirname, 'worker.js')) -setTimeout(() => { - worker.terminate() -}, 1000) + setTimeout(() => { + worker.terminate() + }, 1000) +}