Skip to content

Commit

Permalink
worker: add flag to control old space size
Browse files Browse the repository at this point in the history
This adds a new flag `--thread-max-old-space-size` (name completely
provisional). This has two advantages over the existing
`--max-old-space-size` flag:

1. It allows setting the old space size for the main thread and using
   `resourceLimits` for worker threads. Currently `resourceLimits` will
   be ignored when `--max-old-space-size` is set (see the attached
   issues).
2. It is implemented using V8's public API, rather than relying on V8's
   internal flags whose stability and functionality are not guaranteed.

The downside is that there are now two flags which (in most cases) do
the same thing, so it may cause some confusion. I also think that we
should deprecate `--max-old-space-size`, since the semantics feel pretty
error-prone, but that's a story for another day.

Refs: nodejs#41066
Refs: nodejs#43991
Refs: nodejs#43992
  • Loading branch information
kvakil committed Jul 26, 2022
1 parent 0484022 commit 40caeb0
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 34 deletions.
22 changes: 22 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,20 @@ added: v18.0.0
Configures the test runner to only execute top level tests that have the `only`
option set.

### `--thread-max-old-space-size`

<!-- YAML
added: REPLACEME
-->

Sets the max memory size of V8's old memory section for the main thread (in
megabytes). As memory consumption approaches the limit, V8 will spend more time
on garbage collection in an effort to free unused memory.

Unlike [`--max-old-space-size`][], this option doesn't affect any additional
[worker threads][]. To configure the old space size for worker threads, pass in
an appropriate [`resourceLimits`][] to their constructor.

### `--throw-deprecation`

<!-- YAML
Expand Down Expand Up @@ -1710,6 +1724,7 @@ Node.js options that are allowed are:
* `--secure-heap-min`
* `--secure-heap`
* `--test-only`
* `--thread-max-old-space-size`
* `--throw-deprecation`
* `--title`
* `--tls-cipher-list`
Expand Down Expand Up @@ -2042,6 +2057,9 @@ Sets the max memory size of V8's old memory section. As memory
consumption approaches the limit, V8 will spend more time on
garbage collection in an effort to free unused memory.

Unlike [`--thread-max-old-space-size`][], this sets the max old space size of
all [worker threads][].

On a machine with 2 GiB of memory, consider setting this to
1536 (1.5 GiB) to leave some memory for other uses and avoid swapping.

Expand Down Expand Up @@ -2093,8 +2111,10 @@ done
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--experimental-wasm-modules`]: #--experimental-wasm-modules
[`--heap-prof-dir`]: #--heap-prof-dir
[`--max-old-space-size`]: #--max-old-space-sizesize-in-megabytes
[`--openssl-config`]: #--openssl-configfile
[`--redirect-warnings`]: #--redirect-warningsfile
[`--thread-max-old-space-size`]: #--thread-max-old-space-size
[`Atomics.wait()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/wait
[`Buffer`]: buffer.md#class-buffer
[`CRYPTO_secure_malloc_init`]: https://www.openssl.org/docs/man1.1.0/man3/CRYPTO_secure_malloc_init.html
Expand All @@ -2107,6 +2127,7 @@ done
[`dnsPromises.lookup()`]: dns.md#dnspromiseslookuphostname-options
[`import` specifier]: esm.md#import-specifiers
[`process.setUncaughtExceptionCaptureCallback()`]: process.md#processsetuncaughtexceptioncapturecallbackfn
[`resourceLimits`]: worker_threads.md#new-workerfilename-options
[`tls.DEFAULT_MAX_VERSION`]: tls.md#tlsdefault_max_version
[`tls.DEFAULT_MIN_VERSION`]: tls.md#tlsdefault_min_version
[`unhandledRejection`]: process.md#event-unhandledrejection
Expand All @@ -2126,3 +2147,4 @@ done
[semi-space]: https://www.memorymanagement.org/glossary/s.html#semi.space
[timezone IDs]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
[ways that `TZ` is handled in other environments]: https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html
[worker threads]: worker_threads.md
5 changes: 5 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,11 @@ Starts the Node.js command line test runner.
Configures the test runner to only execute top level tests that have the `only`
option set.
.
.It Fl -thread-max-old-space-size
Sets the max memory size of V8's old memory section for the main thread (in
megabytes). As memory consumption approaches the limit, V8 will spend more time
on garbage collection in an effort to free unused memory.
.
.It Fl -throw-deprecation
Throw errors for deprecations.
.
Expand Down
4 changes: 3 additions & 1 deletion src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,9 @@ IsolateData* CreateIsolateData(Isolate* isolate,
uv_loop_t* loop,
MultiIsolatePlatform* platform,
ArrayBufferAllocator* allocator) {
return new IsolateData(isolate, loop, platform, allocator);
auto options = std::make_shared<PerIsolateOptions>(
*(per_process::cli_options->per_isolate));
return new IsolateData(isolate, loop, std::move(options), platform, allocator);
}

void FreeIsolateData(IsolateData* isolate_data) {
Expand Down
5 changes: 2 additions & 3 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -367,17 +367,16 @@ void IsolateData::CreateProperties() {

IsolateData::IsolateData(Isolate* isolate,
uv_loop_t* event_loop,
std::shared_ptr<PerIsolateOptions> options,
MultiIsolatePlatform* platform,
ArrayBufferAllocator* node_allocator,
const std::vector<size_t>* indexes)
: isolate_(isolate),
event_loop_(event_loop),
options_(options),
node_allocator_(node_allocator == nullptr ? nullptr
: node_allocator->GetImpl()),
platform_(platform) {
options_.reset(
new PerIsolateOptions(*(per_process::cli_options->per_isolate)));

if (indexes == nullptr) {
CreateProperties();
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
public:
IsolateData(v8::Isolate* isolate,
uv_loop_t* event_loop,
std::shared_ptr<PerIsolateOptions> options,
MultiIsolatePlatform* platform = nullptr,
ArrayBufferAllocator* node_allocator = nullptr,
const std::vector<size_t>* indexes = nullptr);
Expand Down Expand Up @@ -642,9 +643,9 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {

v8::Isolate* const isolate_;
uv_loop_t* const event_loop_;
std::shared_ptr<PerIsolateOptions> options_;
NodeArrayBufferAllocator* const node_allocator_;
MultiIsolatePlatform* platform_;
std::shared_ptr<PerIsolateOptions> options_;
worker::Worker* worker_context_ = nullptr;
};

Expand Down
19 changes: 17 additions & 2 deletions src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate,
platform_(platform),
isolate_data_(nullptr),
snapshot_data_(nullptr) {
isolate_data_ =
std::make_unique<IsolateData>(isolate_, event_loop, platform, nullptr);
auto options = std::make_shared<PerIsolateOptions>(
*(per_process::cli_options->per_isolate));
isolate_data_ = std::make_unique<IsolateData>(
isolate_, event_loop, std::move(options), platform, nullptr);

SetIsolateMiscHandlers(isolate_, {});
}
Expand Down Expand Up @@ -77,20 +79,33 @@ NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data,

isolate_ = Isolate::Allocate();
CHECK_NOT_NULL(isolate_);

auto options = std::make_shared<PerIsolateOptions>(
*(per_process::cli_options->per_isolate));

// Register the isolate on the platform before the isolate gets initialized,
// so that the isolate can access the platform during initialization.
platform->RegisterIsolate(isolate_, event_loop);
SetIsolateCreateParamsForNode(isolate_params_.get());

size_t thread_max_old_space_size = options->thread_max_old_space_size;
if (thread_max_old_space_size != 0) {
isolate_params_->constraints.set_max_old_generation_size_in_bytes(
thread_max_old_space_size * 1024 * 1024);
}

Isolate::Initialize(isolate_, *isolate_params_);

// If the indexes are not nullptr, we are not deserializing
isolate_data_ = std::make_unique<IsolateData>(
isolate_,
event_loop,
std::move(options),
platform,
array_buffer_allocator_.get(),
snapshot_data == nullptr ? nullptr
: &(snapshot_data->isolate_data_indices));

IsolateSettings s;
SetIsolateMiscHandlers(isolate_, s);
if (snapshot_data == nullptr) {
Expand Down
6 changes: 6 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,12 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
&PerIsolateOptions::track_heap_objects,
kAllowedInEnvironment);

AddOption(
"--thread-max-old-space-size",
"set the maximum old space heap size (in megabytes) for this isolate",
&PerIsolateOptions::thread_max_old_space_size,
kAllowedInEnvironment);

// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
AddOption("--abort-on-uncaught-exception",
"aborting instead of exiting causes a core file to be generated "
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ class PerIsolateOptions : public Options {
bool report_uncaught_exception = false;
bool report_on_signal = false;
bool experimental_shadow_realm = false;
size_t thread_max_old_space_size = 0;
std::string report_signal = "SIGUSR2";
inline EnvironmentOptions* get_per_env_options();
void CheckOptions(std::vector<std::string>* errors) override;
Expand Down
34 changes: 34 additions & 0 deletions test/common/allocate-and-check-limits.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';
const assert = require('assert');
const v8 = require('v8');

function allocateUntilCrash(resourceLimits) {
const array = [];
while (true) {
const usedMB = v8.getHeapStatistics().used_heap_size / 1024 / 1024;
const maxReservedSize = resourceLimits.maxOldGenerationSizeMb +
resourceLimits.maxYoungGenerationSizeMb;
assert(usedMB < maxReservedSize);

let seenSpaces = 0;
for (const { space_name, space_size } of v8.getHeapSpaceStatistics()) {
if (space_name === 'new_space') {
seenSpaces++;
assert(
space_size / 1024 / 1024 < resourceLimits.maxYoungGenerationSizeMb * 2);
} else if (space_name === 'old_space') {
seenSpaces++;
assert(space_size / 1024 / 1024 < resourceLimits.maxOldGenerationSizeMb);
} else if (space_name === 'code_space') {
seenSpaces++;
assert(space_size / 1024 / 1024 < resourceLimits.codeRangeSizeMb);
}
}
assert.strictEqual(seenSpaces, 3);

for (let i = 0; i < 100; i++)
array.push([array]);
}
}

module.exports = { allocateUntilCrash };
3 changes: 3 additions & 0 deletions test/fixtures/thread-max-old-space-size.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { allocateUntilCrash } = require('../common/allocate-and-check-limits');
const resourceLimits = JSON.parse(process.argv[2]);
allocateUntilCrash(resourceLimits);
22 changes: 22 additions & 0 deletions test/parallel/test-thread-max-old-space-size.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';

require('../common');
const assert = require('assert');
const fixtures = require('../common/fixtures');
const fixture = fixtures.path('thread-max-old-space-size.js');
const { spawnSync } = require('child_process');
const resourceLimits = {
maxOldGenerationSizeMb: 16,
maxYoungGenerationSizeMb: 4,
// Set codeRangeSizeMb really high to effectively ignore it.
codeRangeSizeMb: 999999,
stackSizeMb: 4,
};
const res = spawnSync(process.execPath, [
`--stack-size=${1024 * resourceLimits.stackSizeMb}`,
`--thread-max-old-space-size=${resourceLimits.maxOldGenerationSizeMb}`,
`--max-semi-space-size=${resourceLimits.maxYoungGenerationSizeMb / 2}`,
fixture,
JSON.stringify(resourceLimits)
]);
assert(res.stderr.toString('utf8').includes('Allocation failed - JavaScript heap out of memory'));
35 changes: 35 additions & 0 deletions test/parallel/test-worker-max-old-space-size.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Flags: --thread-max-old-space-size=1024
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker, resourceLimits } = require('worker_threads');
const { allocateUntilCrash } = require('../common/allocate-and-check-limits');

const testResourceLimits = {
maxOldGenerationSizeMb: 16,
maxYoungGenerationSizeMb: 4,
codeRangeSizeMb: 16,
stackSizeMb: 1,
};

// Do not use isMainThread so that this test itself can be run inside a Worker.
if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = 1;
const w = new Worker(__filename, { resourceLimits: testResourceLimits });
assert.deepStrictEqual(w.resourceLimits, testResourceLimits);
w.on('exit', common.mustCall((code) => {
assert.strictEqual(code, 1);
assert.deepStrictEqual(w.resourceLimits, {});
}));
w.on('error', common.expectsError({
code: 'ERR_WORKER_OUT_OF_MEMORY',
message: 'Worker terminated due to reaching memory limit: ' +
'JS heap out of memory'
}));
return;
}

assert.deepStrictEqual(resourceLimits, testResourceLimits);
// resourceLimits should be used; --thread-max-old-space-size should only
// affect the main thread.
allocateUntilCrash(resourceLimits);
29 changes: 2 additions & 27 deletions test/parallel/test-worker-resource-limits.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const v8 = require('v8');
const { Worker, resourceLimits, isMainThread } = require('worker_threads');
const { allocateUntilCrash } = require('../common/allocate-and-check-limits');

if (isMainThread) {
assert.deepStrictEqual(resourceLimits, {});
Expand Down Expand Up @@ -33,29 +33,4 @@ if (!process.env.HAS_STARTED_WORKER) {
}

assert.deepStrictEqual(resourceLimits, testResourceLimits);
const array = [];
while (true) {
const usedMB = v8.getHeapStatistics().used_heap_size / 1024 / 1024;
const maxReservedSize = resourceLimits.maxOldGenerationSizeMb +
resourceLimits.maxYoungGenerationSizeMb;
assert(usedMB < maxReservedSize);

let seenSpaces = 0;
for (const { space_name, space_size } of v8.getHeapSpaceStatistics()) {
if (space_name === 'new_space') {
seenSpaces++;
assert(
space_size / 1024 / 1024 < resourceLimits.maxYoungGenerationSizeMb * 2);
} else if (space_name === 'old_space') {
seenSpaces++;
assert(space_size / 1024 / 1024 < resourceLimits.maxOldGenerationSizeMb);
} else if (space_name === 'code_space') {
seenSpaces++;
assert(space_size / 1024 / 1024 < resourceLimits.codeRangeSizeMb);
}
}
assert.strictEqual(seenSpaces, 3);

for (let i = 0; i < 100; i++)
array.push([array]);
}
allocateUntilCrash(resourceLimits);

0 comments on commit 40caeb0

Please sign in to comment.