From 5324666115cecef52b5fdca26cd934e4e0024dc8 Mon Sep 17 00:00:00 2001 From: theanarkh Date: Thu, 8 Sep 2022 17:33:20 +0800 Subject: [PATCH] v8: add setHeapSnapshotNearHeapLimit PR-URL: https://github.com/nodejs/node/pull/44420 Refs: https://github.com/nodejs/node/pull/33010 Reviewed-By: Joyee Cheung Reviewed-By: Chengzhong Wu --- doc/api/v8.md | 15 ++ lib/v8.js | 19 ++- src/env-inl.h | 18 +++ src/env.cc | 19 ++- src/env.h | 10 +- src/node.cc | 5 +- src/node_v8.cc | 12 ++ .../workload/grow-and-set-near-heap-limit.js | 9 ++ .../grow-worker-and-set-near-heap-limit.js | 15 ++ ...apshot-near-heap-limit-by-api-in-worker.js | 41 +++++ ...est-heapsnapshot-near-heap-limit-by-api.js | 144 ++++++++++++++++++ .../test-heapsnapshot-near-heap-limit.js | 2 +- 12 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 test/fixtures/workload/grow-and-set-near-heap-limit.js create mode 100644 test/fixtures/workload/grow-worker-and-set-near-heap-limit.js create mode 100644 test/parallel/test-heapsnapshot-near-heap-limit-by-api-in-worker.js create mode 100644 test/pummel/test-heapsnapshot-near-heap-limit-by-api.js diff --git a/doc/api/v8.md b/doc/api/v8.md index 32139ab4518f21..ce52c81398c7c0 100644 --- a/doc/api/v8.md +++ b/doc/api/v8.md @@ -343,6 +343,20 @@ if (isMainThread) { } ``` +## `v8.setHeapSnapshotNearHeapLimit(limit)` + + + +> Stability: 1 - Experimental + +* `limit` {integer} + +The API is a no-op if `--heapsnapshot-near-heap-limit` is already set from the +command line or the API is called more than once. `limit` must be a positive +integer. See [`--heapsnapshot-near-heap-limit`][] for more information. + ## Serialization API The serialization API provides means of serializing JavaScript values in a way @@ -987,6 +1001,7 @@ Returns true if the Node.js instance is run to build a snapshot. [HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm [Hook Callbacks]: #hook-callbacks [V8]: https://developers.google.com/v8/ +[`--heapsnapshot-near-heap-limit`]: cli.md#--heapsnapshot-near-heap-limitmax_count [`AsyncLocalStorage`]: async_context.md#class-asynclocalstorage [`Buffer`]: buffer.md [`DefaultDeserializer`]: #class-v8defaultdeserializer diff --git a/lib/v8.js b/lib/v8.js index 113c4f74f323dc..be969406da211d 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -33,7 +33,7 @@ const { } = primordials; const { Buffer } = require('buffer'); -const { validateString } = require('internal/validators'); +const { validateString, validateUint32 } = require('internal/validators'); const { Serializer, Deserializer @@ -59,6 +59,7 @@ const { } = internalBinding('heap_utils'); const { HeapSnapshotStream } = require('internal/heap_utils'); const promiseHooks = require('internal/promise_hooks'); +const { getOptionValue } = require('internal/options'); /** * Generates a snapshot of the current V8 heap @@ -95,6 +96,7 @@ const { updateHeapStatisticsBuffer, updateHeapSpaceStatisticsBuffer, updateHeapCodeStatisticsBuffer, + setHeapSnapshotNearHeapLimit: _setHeapSnapshotNearHeapLimit, // Properties for heap statistics buffer extraction. kTotalHeapSizeIndex, @@ -223,6 +225,18 @@ function getHeapCodeStatistics() { }; } +let heapSnapshotNearHeapLimitCallbackAdded = false; +function setHeapSnapshotNearHeapLimit(limit) { + validateUint32(limit, 'limit', 1); + if (heapSnapshotNearHeapLimitCallbackAdded || + getOptionValue('--heapsnapshot-near-heap-limit') > 0 + ) { + return; + } + heapSnapshotNearHeapLimitCallbackAdded = true; + _setHeapSnapshotNearHeapLimit(limit); +} + /* V8 serialization API */ /* JS methods for the base objects */ @@ -384,5 +398,6 @@ module.exports = { serialize, writeHeapSnapshot, promiseHooks, - startupSnapshot + startupSnapshot, + setHeapSnapshotNearHeapLimit, }; diff --git a/src/env-inl.h b/src/env-inl.h index 97a8bf37629a37..cb0eb2151edf00 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -896,6 +896,24 @@ v8::Local Environment::context() const { return PersistentToLocal::Strong(context_); } +inline void Environment::set_heap_snapshot_near_heap_limit(uint32_t limit) { + heap_snapshot_near_heap_limit_ = limit; +} + +inline void Environment::AddHeapSnapshotNearHeapLimitCallback() { + DCHECK(!heapsnapshot_near_heap_limit_callback_added_); + heapsnapshot_near_heap_limit_callback_added_ = true; + isolate_->AddNearHeapLimitCallback(Environment::NearHeapLimitCallback, this); +} + +inline void Environment::RemoveHeapSnapshotNearHeapLimitCallback( + size_t heap_limit) { + DCHECK(heapsnapshot_near_heap_limit_callback_added_); + heapsnapshot_near_heap_limit_callback_added_ = false; + isolate_->RemoveNearHeapLimitCallback(Environment::NearHeapLimitCallback, + heap_limit); +} + } // namespace node // These two files depend on each other. Including base_object-inl.h after this diff --git a/src/env.cc b/src/env.cc index 77b894bd361b2b..0ddced56d587bd 100644 --- a/src/env.cc +++ b/src/env.cc @@ -770,6 +770,9 @@ Environment::Environment(IsolateData* isolate_data, inspector_host_port_ = std::make_shared>( options_->debug_options().host_port); + heap_snapshot_near_heap_limit_ = + static_cast(options_->heap_snapshot_near_heap_limit); + if (!(flags_ & EnvironmentFlags::kOwnsProcessState)) { set_abort_on_uncaught_exception(false); } @@ -884,9 +887,8 @@ Environment::~Environment() { // FreeEnvironment() should have set this. CHECK(is_stopping()); - if (options_->heap_snapshot_near_heap_limit > heap_limit_snapshot_taken_) { - isolate_->RemoveNearHeapLimitCallback(Environment::NearHeapLimitCallback, - 0); + if (heapsnapshot_near_heap_limit_callback_added_) { + RemoveHeapSnapshotNearHeapLimitCallback(0); } isolate()->GetHeapProfiler()->RemoveBuildEmbedderGraphCallback( @@ -2029,8 +2031,7 @@ size_t Environment::NearHeapLimitCallback(void* data, Debug(env, DebugCategory::DIAGNOSTICS, "Not generating snapshots because it's too risky.\n"); - env->isolate()->RemoveNearHeapLimitCallback(NearHeapLimitCallback, - initial_heap_limit); + env->RemoveHeapSnapshotNearHeapLimitCallback(initial_heap_limit); // The new limit must be higher than current_heap_limit or V8 might // crash. return current_heap_limit + 1; @@ -2050,17 +2051,15 @@ size_t Environment::NearHeapLimitCallback(void* data, // Remove the callback first in case it's triggered when generating // the snapshot. - env->isolate()->RemoveNearHeapLimitCallback(NearHeapLimitCallback, - initial_heap_limit); + env->RemoveHeapSnapshotNearHeapLimitCallback(initial_heap_limit); heap::WriteSnapshot(env->isolate(), filename.c_str()); env->heap_limit_snapshot_taken_ += 1; // Don't take more snapshots than the number specified by // --heapsnapshot-near-heap-limit. - if (env->heap_limit_snapshot_taken_ < - env->options_->heap_snapshot_near_heap_limit) { - env->isolate()->AddNearHeapLimitCallback(NearHeapLimitCallback, env); + if (env->heap_limit_snapshot_taken_ < env->heap_snapshot_near_heap_limit_) { + env->AddHeapSnapshotNearHeapLimitCallback(); } FPrintF(stderr, "Wrote snapshot to %s\n", filename.c_str()); diff --git a/src/env.h b/src/env.h index 51cdbb9e7cb8e9..21a095194c4061 100644 --- a/src/env.h +++ b/src/env.h @@ -1459,6 +1459,12 @@ class Environment : public MemoryRetainer { template void ForEachBindingData(T&& iterator); + inline void set_heap_snapshot_near_heap_limit(uint32_t limit); + + inline void AddHeapSnapshotNearHeapLimitCallback(); + + inline void RemoveHeapSnapshotNearHeapLimitCallback(size_t heap_limit); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); @@ -1516,7 +1522,9 @@ class Environment : public MemoryRetainer { std::string exec_path_; bool is_processing_heap_limit_callback_ = false; - int64_t heap_limit_snapshot_taken_ = 0; + uint32_t heap_limit_snapshot_taken_ = 0; + uint32_t heap_snapshot_near_heap_limit_ = 0; + bool heapsnapshot_near_heap_limit_callback_added_ = false; uint32_t module_id_counter_ = 0; uint32_t script_id_counter_ = 0; diff --git a/src/node.cc b/src/node.cc index 69e817c8031078..dd1c8c81280e03 100644 --- a/src/node.cc +++ b/src/node.cc @@ -282,9 +282,8 @@ static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event, void Environment::InitializeDiagnostics() { isolate_->GetHeapProfiler()->AddBuildEmbedderGraphCallback( Environment::BuildEmbedderGraph, this); - if (options_->heap_snapshot_near_heap_limit > 0) { - isolate_->AddNearHeapLimitCallback(Environment::NearHeapLimitCallback, - this); + if (heap_snapshot_near_heap_limit_ > 0) { + AddHeapSnapshotNearHeapLimitCallback(); } if (options_->trace_uncaught) isolate_->SetCaptureStackTraceForUncaughtExceptions(true); diff --git a/src/node_v8.cc b/src/node_v8.cc index 70b294f1eca5de..b24b6f7973cf45 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -155,6 +155,15 @@ void CachedDataVersionTag(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } +void SetHeapSnapshotNearHeapLimit(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsUint32()); + Environment* env = Environment::GetCurrent(args); + uint32_t limit = args[0].As()->Value(); + CHECK_GT(limit, 0); + env->AddHeapSnapshotNearHeapLimitCallback(); + env->set_heap_snapshot_near_heap_limit(limit); +} + void UpdateHeapStatisticsBuffer(const FunctionCallbackInfo& args) { BindingData* data = Environment::GetBindingData(args); HeapStatistics s; @@ -212,6 +221,8 @@ void Initialize(Local target, CachedDataVersionTag); env->SetMethod( target, "updateHeapStatisticsBuffer", UpdateHeapStatisticsBuffer); + env->SetMethodNoSideEffect(target, "setHeapSnapshotNearHeapLimit", + SetHeapSnapshotNearHeapLimit); env->SetMethod( target, "updateHeapCodeStatisticsBuffer", UpdateHeapCodeStatisticsBuffer); @@ -259,6 +270,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(UpdateHeapCodeStatisticsBuffer); registry->Register(UpdateHeapSpaceStatisticsBuffer); registry->Register(SetFlagsFromString); + registry->Register(SetHeapSnapshotNearHeapLimit); } } // namespace v8_utils diff --git a/test/fixtures/workload/grow-and-set-near-heap-limit.js b/test/fixtures/workload/grow-and-set-near-heap-limit.js new file mode 100644 index 00000000000000..62d2c1ffdc5bd4 --- /dev/null +++ b/test/fixtures/workload/grow-and-set-near-heap-limit.js @@ -0,0 +1,9 @@ +'use strict'; +const path = require('path'); +const v8 = require('v8'); + +v8.setHeapSnapshotNearHeapLimit(+process.env.limit); +if (process.env.limit2) { + v8.setHeapSnapshotNearHeapLimit(+process.env.limit2); +} +require(path.resolve(__dirname, 'grow.js')); diff --git a/test/fixtures/workload/grow-worker-and-set-near-heap-limit.js b/test/fixtures/workload/grow-worker-and-set-near-heap-limit.js new file mode 100644 index 00000000000000..598088bcf90a78 --- /dev/null +++ b/test/fixtures/workload/grow-worker-and-set-near-heap-limit.js @@ -0,0 +1,15 @@ +'use strict'; +const path = require('path'); +const { Worker } = require('worker_threads'); +const max_snapshots = parseInt(process.env.TEST_SNAPSHOTS) || 1; +new Worker(path.join(__dirname, 'grow-and-set-near-heap-limit.js'), { + env: { + ...process.env, + limit: max_snapshots, + }, + resourceLimits: { + maxOldGenerationSizeMb: + parseInt(process.env.TEST_OLD_SPACE_SIZE) || 20 + } +}); + diff --git a/test/parallel/test-heapsnapshot-near-heap-limit-by-api-in-worker.js b/test/parallel/test-heapsnapshot-near-heap-limit-by-api-in-worker.js new file mode 100644 index 00000000000000..ac55933a47122b --- /dev/null +++ b/test/parallel/test-heapsnapshot-near-heap-limit-by-api-in-worker.js @@ -0,0 +1,41 @@ +// Copy from test-heapsnapshot-near-heap-limit-worker.js +'use strict'; + +require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); + +const env = { + ...process.env, + NODE_DEBUG_NATIVE: 'diagnostics' +}; + +{ + tmpdir.refresh(); + const child = spawnSync(process.execPath, [ + fixtures.path('workload', 'grow-worker-and-set-near-heap-limit.js'), + ], { + cwd: tmpdir.path, + env: { + TEST_SNAPSHOTS: 1, + TEST_OLD_SPACE_SIZE: 50, + ...env + } + }); + console.log(child.stdout.toString()); + const stderr = child.stderr.toString(); + console.log(stderr); + const risky = /Not generating snapshots because it's too risky/.test(stderr); + if (!risky) { + // There should be one snapshot taken and then after the + // snapshot heap limit callback is popped, the OOM callback + // becomes effective. + assert(stderr.includes('ERR_WORKER_OUT_OF_MEMORY')); + const list = fs.readdirSync(tmpdir.path) + .filter((file) => file.endsWith('.heapsnapshot')); + assert.strictEqual(list.length, 1); + } +} diff --git a/test/pummel/test-heapsnapshot-near-heap-limit-by-api.js b/test/pummel/test-heapsnapshot-near-heap-limit-by-api.js new file mode 100644 index 00000000000000..b8b87139126105 --- /dev/null +++ b/test/pummel/test-heapsnapshot-near-heap-limit-by-api.js @@ -0,0 +1,144 @@ +// Copy from test-heapsnapshot-near-heap-limit.js +'use strict'; + +const common = require('../common'); + +if (common.isPi) { + common.skip('Too slow for Raspberry Pi devices'); +} + +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); +const v8 = require('v8'); + +const invalidValues = [-1, '', {}, NaN, undefined]; +for (let i = 0; i < invalidValues.length; i++) { + assert.throws(() => v8.setHeapSnapshotNearHeapLimit(invalidValues[i]), + /ERR_INVALID_ARG_TYPE|ERR_OUT_OF_RANGE/); +} + +// Set twice +v8.setHeapSnapshotNearHeapLimit(1); +v8.setHeapSnapshotNearHeapLimit(2); + +const env = { + ...process.env, + NODE_DEBUG_NATIVE: 'diagnostics', +}; + +{ + console.log('\nTesting set by cmd option and api'); + tmpdir.refresh(); + const child = spawnSync(process.execPath, [ + '--trace-gc', + '--heapsnapshot-near-heap-limit=1', + '--max-old-space-size=50', + fixtures.path('workload', 'grow-and-set-near-heap-limit.js'), + ], { + cwd: tmpdir.path, + env: { + ...env, + limit: 2, + }, + }); + console.log(child.stdout.toString()); + const stderr = child.stderr.toString(); + console.log(stderr); + assert(common.nodeProcessAborted(child.status, child.signal), + 'process should have aborted, but did not'); + const list = fs.readdirSync(tmpdir.path) + .filter((file) => file.endsWith('.heapsnapshot')); + const risky = [...stderr.matchAll( + /Not generating snapshots because it's too risky/g)].length; + assert(list.length + risky > 0 && list.length <= 1, + `Generated ${list.length} snapshots ` + + `and ${risky} was too risky`); +} + +{ + console.log('\nTesting limit = 1'); + tmpdir.refresh(); + const child = spawnSync(process.execPath, [ + '--trace-gc', + '--max-old-space-size=50', + fixtures.path('workload', 'grow-and-set-near-heap-limit.js'), + ], { + cwd: tmpdir.path, + env: { + ...env, + limit: 1, + }, + }); + console.log(child.stdout.toString()); + const stderr = child.stderr.toString(); + console.log(stderr); + assert(common.nodeProcessAborted(child.status, child.signal), + 'process should have aborted, but did not'); + const list = fs.readdirSync(tmpdir.path) + .filter((file) => file.endsWith('.heapsnapshot')); + const risky = [...stderr.matchAll( + /Not generating snapshots because it's too risky/g)].length; + assert(list.length + risky > 0 && list.length <= 1, + `Generated ${list.length} snapshots ` + + `and ${risky} was too risky`); +} + +{ + console.log('\nTesting set limit twice'); + tmpdir.refresh(); + const child = spawnSync(process.execPath, [ + '--trace-gc', + '--max-old-space-size=50', + fixtures.path('workload', 'grow-and-set-near-heap-limit.js'), + ], { + cwd: tmpdir.path, + env: { + ...env, + limit: 1, + limit2: 2 + }, + }); + console.log(child.stdout.toString()); + const stderr = child.stderr.toString(); + console.log(stderr); + assert(common.nodeProcessAborted(child.status, child.signal), + 'process should have aborted, but did not'); + const list = fs.readdirSync(tmpdir.path) + .filter((file) => file.endsWith('.heapsnapshot')); + const risky = [...stderr.matchAll( + /Not generating snapshots because it's too risky/g)].length; + assert(list.length + risky > 0 && list.length <= 1, + `Generated ${list.length} snapshots ` + + `and ${risky} was too risky`); +} + +{ + console.log('\nTesting limit = 3'); + tmpdir.refresh(); + const child = spawnSync(process.execPath, [ + '--trace-gc', + '--max-old-space-size=50', + fixtures.path('workload', 'grow-and-set-near-heap-limit.js'), + ], { + cwd: tmpdir.path, + env: { + ...env, + limit: 3, + }, + }); + console.log(child.stdout.toString()); + const stderr = child.stderr.toString(); + console.log(stderr); + assert(common.nodeProcessAborted(child.status, child.signal), + 'process should have aborted, but did not'); + const list = fs.readdirSync(tmpdir.path) + .filter((file) => file.endsWith('.heapsnapshot')); + const risky = [...stderr.matchAll( + /Not generating snapshots because it's too risky/g)].length; + assert(list.length + risky > 0 && list.length <= 3, + `Generated ${list.length} snapshots ` + + `and ${risky} was too risky`); +} diff --git a/test/pummel/test-heapsnapshot-near-heap-limit.js b/test/pummel/test-heapsnapshot-near-heap-limit.js index 1af4e61e08c028..7677fe64ad854a 100644 --- a/test/pummel/test-heapsnapshot-near-heap-limit.js +++ b/test/pummel/test-heapsnapshot-near-heap-limit.js @@ -71,7 +71,7 @@ const env = { .filter((file) => file.endsWith('.heapsnapshot')); const risky = [...stderr.matchAll( /Not generating snapshots because it's too risky/g)].length; - assert(list.length + risky > 0 && list.length <= 3, + assert(list.length + risky > 0 && list.length <= 1, `Generated ${list.length} snapshots ` + `and ${risky} was too risky`); }