From 137a5526411fc987271623eed8d80d208e1beaaa Mon Sep 17 00:00:00 2001 From: Robo Date: Sat, 15 Feb 2025 01:00:39 +0900 Subject: [PATCH] fix: support for v8.setHeapSnapshotNearHeapLimit api (#45606) * fix: support for v8.setHeapSnapshotNearHeapLimit api * docs: add support --- docs/api/command-line-switches.md | 6 +++++ shell/browser/electron_browser_main_parts.cc | 3 ++- shell/browser/javascript_environment.cc | 11 +++++++-- shell/browser/javascript_environment.h | 4 ++++ shell/common/node_bindings.cc | 7 +++++- shell/common/node_bindings.h | 2 ++ shell/renderer/electron_renderer_client.cc | 2 +- shell/services/node/node_service.cc | 3 ++- spec/api-utility-process-spec.ts | 23 +++++++++++++++++++ spec/fixtures/api/utility-process/oom-grow.js | 11 +++++++++ 10 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 spec/fixtures/api/utility-process/oom-grow.js diff --git a/docs/api/command-line-switches.md b/docs/api/command-line-switches.md index 6c80fde7c179..a07a3eb9523d 100644 --- a/docs/api/command-line-switches.md +++ b/docs/api/command-line-switches.md @@ -313,6 +313,12 @@ Set the default value of the `verbatim` parameter in the Node.js [`dns.lookup()` The default is `verbatim` and `dns.setDefaultResultOrder()` have higher priority than `--dns-result-order`. +### `--diagnostic-dir=directory` + +Set the directory to which all Node.js diagnostic output files are written. Defaults to current working directory. + +Affects the default output directory of [v8.setHeapSnapshotNearHeapLimit](https://nodejs.org/docs/latest/api/v8.html#v8setheapsnapshotnearheaplimitlimit). + [app]: app.md [append-switch]: command-line.md#commandlineappendswitchswitch-value [debugging-main-process]: ../tutorial/debugging-main-process.md diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 4b3a61ec4b66..66c3cef19ed8 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -237,7 +237,8 @@ void ElectronBrowserMainParts::PostEarlyInitialization() { node_bindings_->Initialize(js_env_->isolate()->GetCurrentContext()); // Create the global environment. node_env_ = node_bindings_->CreateEnvironment( - js_env_->isolate()->GetCurrentContext(), js_env_->platform()); + js_env_->isolate()->GetCurrentContext(), js_env_->platform(), + js_env_->max_young_generation_size_in_bytes()); node_env_->set_trace_sync_io(node_env_->options()->trace_sync_io); diff --git a/shell/browser/javascript_environment.cc b/shell/browser/javascript_environment.cc index 314562e5c297..1632c31d665c 100644 --- a/shell/browser/javascript_environment.cc +++ b/shell/browser/javascript_environment.cc @@ -33,9 +33,15 @@ namespace electron { namespace { -gin::IsolateHolder CreateIsolateHolder(v8::Isolate* isolate) { +gin::IsolateHolder CreateIsolateHolder(v8::Isolate* isolate, + size_t* max_young_generation_size) { std::unique_ptr create_params = gin::IsolateHolder::getDefaultIsolateParams(); + // The value is needed to adjust heap limit when capturing + // snapshot via v8.setHeapSnapshotNearHeapLimit(limit) or + // --heapsnapshot-near-heap-limit=max_count. + *max_young_generation_size = + create_params->constraints.max_young_generation_size_in_bytes(); // Align behavior with V8 Isolate default for Node.js. // This is necessary for important aspects of Node.js // including heap and cpu profilers to function properly. @@ -55,7 +61,8 @@ gin::IsolateHolder CreateIsolateHolder(v8::Isolate* isolate) { JavascriptEnvironment::JavascriptEnvironment(uv_loop_t* event_loop, bool setup_wasm_streaming) : isolate_holder_{CreateIsolateHolder( - Initialize(event_loop, setup_wasm_streaming))}, + Initialize(event_loop, setup_wasm_streaming), + &max_young_generation_size_)}, isolate_{isolate_holder_.isolate()}, locker_{isolate_} { isolate_->Enter(); diff --git a/shell/browser/javascript_environment.h b/shell/browser/javascript_environment.h index adb7be448cee..ed78e980f7d7 100644 --- a/shell/browser/javascript_environment.h +++ b/shell/browser/javascript_environment.h @@ -36,6 +36,9 @@ class JavascriptEnvironment { node::MultiIsolatePlatform* platform() const { return platform_.get(); } v8::Isolate* isolate() const { return isolate_; } + size_t max_young_generation_size_in_bytes() const { + return max_young_generation_size_; + } static v8::Isolate* GetIsolate(); @@ -43,6 +46,7 @@ class JavascriptEnvironment { v8::Isolate* Initialize(uv_loop_t* event_loop, bool setup_wasm_streaming); std::unique_ptr platform_; + size_t max_young_generation_size_ = 0; gin::IsolateHolder isolate_holder_; // owned-by: isolate_holder_ diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 8c682aba1c1e..c2dde178cf7b 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -324,6 +324,7 @@ bool IsAllowedOption(const std::string_view option) { // This should be aligned with what's possible to set via the process object. static constexpr auto options = base::MakeFixedFlatSet({ + "--diagnostic-dir", "--dns-result-order", "--no-deprecation", "--throw-deprecation", @@ -596,6 +597,7 @@ void NodeBindings::Initialize(v8::Local context) { std::shared_ptr NodeBindings::CreateEnvironment( v8::Local context, node::MultiIsolatePlatform* platform, + size_t max_young_generation_size, std::vector args, std::vector exec_args, std::optional> on_app_code_ready) { @@ -646,6 +648,7 @@ std::shared_ptr NodeBindings::CreateEnvironment( args.insert(args.begin() + 1, init_script); auto* isolate_data = node::CreateIsolateData(isolate, uv_loop_, platform); + isolate_data->max_young_gen_size = max_young_generation_size; context->SetAlignedPointerInEmbedderData(kElectronContextEmbedderDataIndex, static_cast(isolate_data)); @@ -783,8 +786,10 @@ std::shared_ptr NodeBindings::CreateEnvironment( std::shared_ptr NodeBindings::CreateEnvironment( v8::Local context, node::MultiIsolatePlatform* platform, + size_t max_young_generation_size, std::optional> on_app_code_ready) { - return CreateEnvironment(context, platform, ElectronCommandLine::AsUtf8(), {}, + return CreateEnvironment(context, platform, max_young_generation_size, + ElectronCommandLine::AsUtf8(), {}, on_app_code_ready); } diff --git a/shell/common/node_bindings.h b/shell/common/node_bindings.h index a870e97c1fd5..d50ad9d52d65 100644 --- a/shell/common/node_bindings.h +++ b/shell/common/node_bindings.h @@ -133,6 +133,7 @@ class NodeBindings { std::shared_ptr CreateEnvironment( v8::Local context, node::MultiIsolatePlatform* platform, + size_t max_young_generation_size, std::vector args, std::vector exec_args, std::optional> on_app_code_ready = @@ -141,6 +142,7 @@ class NodeBindings { std::shared_ptr CreateEnvironment( v8::Local context, node::MultiIsolatePlatform* platform, + size_t max_young_generation_size = 0, std::optional> on_app_code_ready = std::nullopt); diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index d22b829d9a59..1c7f79516c04 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -109,7 +109,7 @@ void ElectronRendererClient::DidCreateScriptContext( blink::LoaderFreezeMode::kStrict); std::shared_ptr env = node_bindings_->CreateEnvironment( - renderer_context, nullptr, + renderer_context, nullptr, 0, base::BindRepeating(&ElectronRendererClient::UndeferLoad, base::Unretained(this), render_frame)); diff --git a/shell/services/node/node_service.cc b/shell/services/node/node_service.cc index edcdb127bffc..52a7a7e7bca1 100644 --- a/shell/services/node/node_service.cc +++ b/shell/services/node/node_service.cc @@ -137,7 +137,8 @@ void NodeService::Initialize( // Create the global environment. node_env_ = node_bindings_->CreateEnvironment( js_env_->isolate()->GetCurrentContext(), js_env_->platform(), - params->args, params->exec_args); + js_env_->max_young_generation_size_in_bytes(), params->args, + params->exec_args); // Override the default handler set by NodeBindings. node_env_->isolate()->SetFatalErrorHandler(V8FatalErrorCallback); diff --git a/spec/api-utility-process-spec.ts b/spec/api-utility-process-spec.ts index 364b7e864844..5e6d4fe0380c 100644 --- a/spec/api-utility-process-spec.ts +++ b/spec/api-utility-process-spec.ts @@ -5,6 +5,8 @@ import { expect } from 'chai'; import * as childProcess from 'node:child_process'; import { once } from 'node:events'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; import * as path from 'node:path'; import { setImmediate } from 'node:timers/promises'; import { pathToFileURL } from 'node:url'; @@ -760,5 +762,26 @@ describe('utilityProcess module', () => { expect(loginAuthInfo!.realm).to.equal('Foo'); expect(loginAuthInfo!.scheme).to.equal('basic'); }); + + it('supports generating snapshots via v8.setHeapSnapshotNearHeapLimit', async () => { + const tmpDir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-spec-utility-oom-')); + const child = utilityProcess.fork(path.join(fixturesPath, 'oom-grow.js'), [], { + stdio: 'ignore', + execArgv: [ + `--diagnostic-dir=${tmpDir}`, + '--js-flags=--max-old-space-size=50' + ], + env: { + NODE_DEBUG_NATIVE: 'diagnostic' + } + }); + await once(child, 'spawn'); + await once(child, 'exit'); + const files = (await fs.readdir(tmpDir)).filter((file) => file.endsWith('.heapsnapshot')); + expect(files.length).to.be.equal(1); + const stat = await fs.stat(path.join(tmpDir, files[0])); + expect(stat.size).to.be.greaterThan(0); + await fs.rm(tmpDir, { recursive: true }); + }); }); }); diff --git a/spec/fixtures/api/utility-process/oom-grow.js b/spec/fixtures/api/utility-process/oom-grow.js new file mode 100644 index 000000000000..c20c10c708da --- /dev/null +++ b/spec/fixtures/api/utility-process/oom-grow.js @@ -0,0 +1,11 @@ +const v8 = require('node:v8'); + +v8.setHeapSnapshotNearHeapLimit(1); + +const arr = []; +function runAllocation () { + const str = JSON.stringify(process.config).slice(0, 1000); + arr.push(str); + setImmediate(runAllocation); +} +setImmediate(runAllocation);