Skip to content

Commit

Permalink
worker: add stack size resource limit option
Browse files Browse the repository at this point in the history
Add `stackSizeMb` to the `resourceLimit` option group.

Refs: #31593 (comment)

PR-URL: #33085
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax committed Apr 29, 2020
1 parent ef85bd1 commit e7b99e0
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 20 deletions.
4 changes: 4 additions & 0 deletions doc/api/worker_threads.md
Expand Up @@ -174,6 +174,7 @@ added:
* `maxYoungGenerationSizeMb` {number}
* `maxOldGenerationSizeMb` {number}
* `codeRangeSizeMb` {number}
* `stackSizeMb` {number}

Provides the set of JS engine resource constraints inside this Worker thread.
If the `resourceLimits` option was passed to the [`Worker`][] constructor,
Expand Down Expand Up @@ -584,6 +585,8 @@ changes:
recently created objects.
* `codeRangeSizeMb` {number} The size of a pre-allocated memory range
used for generated code.
* `stackSizeMb` {number} The default maximum stack size for the thread.
Small values may lead to unusable Worker instances. **Default:** `4`.

### Event: `'error'`
<!-- YAML
Expand Down Expand Up @@ -679,6 +682,7 @@ added:
* `maxYoungGenerationSizeMb` {number}
* `maxOldGenerationSizeMb` {number}
* `codeRangeSizeMb` {number}
* `stackSizeMb` {number}

Provides the set of JS engine resource constraints for this Worker thread.
If the `resourceLimits` option was passed to the [`Worker`][] constructor,
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/worker.js
Expand Up @@ -54,6 +54,7 @@ const {
kMaxYoungGenerationSizeMb,
kMaxOldGenerationSizeMb,
kCodeRangeSizeMb,
kStackSizeMb,
kTotalResourceLimitCount
} = internalBinding('worker');

Expand Down Expand Up @@ -379,14 +380,17 @@ function parseResourceLimits(obj) {
ret[kMaxYoungGenerationSizeMb] = obj.maxYoungGenerationSizeMb;
if (typeof obj.codeRangeSizeMb === 'number')
ret[kCodeRangeSizeMb] = obj.codeRangeSizeMb;
if (typeof obj.stackSizeMb === 'number')
ret[kStackSizeMb] = obj.stackSizeMb;
return ret;
}

function makeResourceLimits(float64arr) {
return {
maxYoungGenerationSizeMb: float64arr[kMaxYoungGenerationSizeMb],
maxOldGenerationSizeMb: float64arr[kMaxOldGenerationSizeMb],
codeRangeSizeMb: float64arr[kCodeRangeSizeMb]
codeRangeSizeMb: float64arr[kCodeRangeSizeMb],
stackSizeMb: float64arr[kStackSizeMb]
};
}

Expand Down
20 changes: 16 additions & 4 deletions src/node_worker.cc
Expand Up @@ -39,6 +39,8 @@ using v8::Value;
namespace node {
namespace worker {

constexpr double kMB = 1024 * 1024;

Worker::Worker(Environment* env,
Local<Object> wrap,
const std::string& url,
Expand Down Expand Up @@ -93,8 +95,6 @@ bool Worker::is_stopped() const {
void Worker::UpdateResourceConstraints(ResourceConstraints* constraints) {
constraints->set_stack_limit(reinterpret_cast<uint32_t*>(stack_base_));

constexpr double kMB = 1024 * 1024;

if (resource_limits_[kMaxYoungGenerationSizeMb] > 0) {
constraints->set_max_young_generation_size_in_bytes(
resource_limits_[kMaxYoungGenerationSizeMb] * kMB);
Expand Down Expand Up @@ -589,9 +589,20 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {

w->stopped_ = false;

if (w->resource_limits_[kStackSizeMb] > 0) {
if (w->resource_limits_[kStackSizeMb] * kMB < kStackBufferSize) {
w->resource_limits_[kStackSizeMb] = kStackBufferSize / kMB;
w->stack_size_ = kStackBufferSize;
} else {
w->stack_size_ = w->resource_limits_[kStackSizeMb] * kMB;
}
} else {
w->resource_limits_[kStackSizeMb] = w->stack_size_ / kMB;
}

uv_thread_options_t thread_options;
thread_options.flags = UV_THREAD_HAS_STACK_SIZE;
thread_options.stack_size = kStackSize;
thread_options.stack_size = w->stack_size_;
int ret = uv_thread_create_ex(&w->tid_, &thread_options, [](void* arg) {
// XXX: This could become a std::unique_ptr, but that makes at least
// gcc 6.3 detect undefined behaviour when there shouldn't be any.
Expand All @@ -601,7 +612,7 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {

// Leave a few kilobytes just to make sure we're within limits and have
// some space to do work in C++ land.
w->stack_base_ = stack_top - (kStackSize - kStackBufferSize);
w->stack_base_ = stack_top - (w->stack_size_ - kStackBufferSize);

w->Run();

Expand Down Expand Up @@ -834,6 +845,7 @@ void InitWorker(Local<Object> target,
NODE_DEFINE_CONSTANT(target, kMaxYoungGenerationSizeMb);
NODE_DEFINE_CONSTANT(target, kMaxOldGenerationSizeMb);
NODE_DEFINE_CONSTANT(target, kCodeRangeSizeMb);
NODE_DEFINE_CONSTANT(target, kStackSizeMb);
NODE_DEFINE_CONSTANT(target, kTotalResourceLimitCount);
}

Expand Down
3 changes: 2 additions & 1 deletion src/node_worker.h
Expand Up @@ -16,6 +16,7 @@ enum ResourceLimits {
kMaxYoungGenerationSizeMb,
kMaxOldGenerationSizeMb,
kCodeRangeSizeMb,
kStackSizeMb,
kTotalResourceLimitCount
};

Expand Down Expand Up @@ -95,7 +96,7 @@ class Worker : public AsyncWrap {
void UpdateResourceConstraints(v8::ResourceConstraints* constraints);

// Full size of the thread's stack.
static constexpr size_t kStackSize = 4 * 1024 * 1024;
size_t stack_size_ = 4 * 1024 * 1024;
// Stack buffer size that is not available to the JS engine.
static constexpr size_t kStackBufferSize = 192 * 1024;

Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-worker-resource-limits.js
Expand Up @@ -12,6 +12,7 @@ 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.
Expand Down
47 changes: 33 additions & 14 deletions test/parallel/test-worker-stack-overflow-stack-size.js
Expand Up @@ -8,7 +8,7 @@ const { Worker } = require('worker_threads');
// Verify that Workers don't care about --stack-size, as they have their own
// fixed and known stack sizes.

async function runWorker() {
async function runWorker(options = {}) {
const empiricalStackDepth = new Uint32Array(new SharedArrayBuffer(4));
const worker = new Worker(`
const { workerData: { empiricalStackDepth } } = require('worker_threads');
Expand All @@ -18,26 +18,45 @@ async function runWorker() {
}
f();`, {
eval: true,
workerData: { empiricalStackDepth }
workerData: { empiricalStackDepth },
...options
});

const [ error ] = await once(worker, 'error');

common.expectsError({
constructor: RangeError,
message: 'Maximum call stack size exceeded'
})(error);
if (!options.skipErrorCheck) {
common.expectsError({
constructor: RangeError,
message: 'Maximum call stack size exceeded'
})(error);
}

return empiricalStackDepth[0];
}

(async function() {
v8.setFlagsFromString('--stack-size=500');
const w1stack = await runWorker();
v8.setFlagsFromString('--stack-size=1000');
const w2stack = await runWorker();
// Make sure the two stack sizes are within 10 % of each other, i.e. not
// affected by the different `--stack-size` settings.
assert(Math.max(w1stack, w2stack) / Math.min(w1stack, w2stack) < 1.1,
`w1stack = ${w1stack}, w2stack ${w2stack} are too far apart`);
{
v8.setFlagsFromString('--stack-size=500');
const w1stack = await runWorker();
v8.setFlagsFromString('--stack-size=1000');
const w2stack = await runWorker();
// Make sure the two stack sizes are within 10 % of each other, i.e. not
// affected by the different `--stack-size` settings.
assert(Math.max(w1stack, w2stack) / Math.min(w1stack, w2stack) < 1.1,
`w1stack = ${w1stack}, w2stack = ${w2stack} are too far apart`);
}

{
const w1stack = await runWorker({ resourceLimits: { stackSizeMb: 0.5 } });
const w2stack = await runWorker({ resourceLimits: { stackSizeMb: 1.0 } });
// Make sure the two stack sizes are at least 40 % apart from each other,
// i.e. affected by the different `stackSizeMb` settings.
assert(w2stack > w1stack * 1.4,
`w1stack = ${w1stack}, w2stack = ${w2stack} are too close`);
}

// Test that various low stack sizes result in an 'error' event.
for (const stackSizeMb of [ 0.001, 0.01, 0.1, 0.2, 0.3, 0.5 ]) {
await runWorker({ resourceLimits: { stackSizeMb }, skipErrorCheck: true });
}
})().then(common.mustCall());

0 comments on commit e7b99e0

Please sign in to comment.