worker: set stack size for worker threads

[addaleax]

(The first commit is #26037.)

This is so we can inform V8 about a known limit for the stack.

Otherwise, on some systems recursive functions may lead to
segmentation faults rather than “safe” failures.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[nodejs-github-bot]

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2572/pipeline

[addaleax]

CI: https://ci.nodejs.org/job/node-test-pull-request/20733/

[gireeshpunathil]

For platforms where 4MB is not the default stack size, does this prove as an added memory pressure, and if so, should we document this?

[addaleax]

does this prove as an added memory pressure

@gireeshpunathil Not on most platforms in their default configuration, I assume – usually, memory pages are only actually committed when first accessed.

I wouldn’t explicitly document this, as it shouldn’t make much of a difference for users.

[gireeshpunathil]

Is it possible that some compilers decide to take this address from the caller thread's stack as opposed to the current thread's stack? I guess it should not, but I am not up-to-date with compiler optimizations.

[addaleax]

The compiler doesn’t know whether there are different threads involved here. And if there’s something invalid about this, then V8 already has the same problem in their code (which I assume they don’t).

[gireeshpunathil]

Should / Could we also expose this into the JS land? say: new Worker(...{stack: <size>});
that way, different workers could be spawned that vary in thread stack size based on the workload that they specializes on? Users can calibrate the values for specific tasks, and thereby optimize memory (in deployments where memory costs).

[bnoordhuis]

Should / Could we also expose this into the JS land? say: new Worker(...{stack: <size>});

Too small or too big stack sizes can be dangerous and the limits depend on the platform and other factors.

There's also the question of how a programmer can know what a reasonable stack size is for a given workload without knowing all about Node's inner workings, or the add-ons they use.

[addaleax]

CI: https://ci.nodejs.org/job/node-test-pull-request/20760/

[addaleax]

Rebased, new CI: https://ci.nodejs.org/job/node-test-pull-request/20763/

[addaleax]

Landed in a80b29b

(If this should get backported to v10.x at some point, it depends on #26037.)

[ulrichb]

So, there is now a hard-coded stack size limit of 4 MB, right?

Is there any possibility to override this limit (e.g. using worker.resourceLimits)?

[addaleax]

@ulrichb There isn’t, currently – as you can see, there was a brief discussion above about that: #26049 (comment)

If you’d like to see a feature like that, could you open a new issue and ideally provide some information about your use case?

[ulrichb]

@addaleax I'm not quite sure why, but it seems that the stack limit can be influenced by the V8 switch --stack-sizealso for the worker threads. Could you confirm that? (Using Node 13.7)

BTW: My use case is that I'm hosting the TypeScript compiler in a worker thread which does a lot of recursion, and this hit the stack limit for really big files (I have generated code files with ~ 20kLOC.)

[ulrichb]

... oh and I used --stack-size=2048 (2 MB), to solve my RangeError: Maximum call stack size exceeded exceptions. This is strange because it's more than the default for --stack-size, but it's less than the 4 MB of this pull request. (And yes, I'm sure that the error happens within the worker thread.)

@addaleax Any ideas? Do the two values (--stack-size and kStackSize) mean something slightly different? Or corresponds --stack-size to kStackBufferSize (192 kB)?

[addaleax]

@ulrichb So…

• --stack-size does not set the actual stack size; instead, it tells V8 about what the stack size is, mostly so that it knows when to bail out with a stack overflow exception.
• kStackSize is the full stack size that is natively allocated for a Worker thread.
• kStackBufferSize refers to a part of the full stack that is not free for use by V8, and instead only there so that C++ code that does not perform stack checks can still reasonably run when near the stack limit.

--stack-size corresponds to kStackSize - kStackBufferSize, then.

This is strange because it's more than the default for --stack-size, but it's less than the 4 MB of this pull request. (And yes, I'm sure that the error happens within the worker thread.)

I’m not sure what to say except that I fully agree with you – this is strange. I feel like --stack-size shouldn’t even have an effect on Worker threads at all, although it defintiely does. I think that qualifies as a separate bug, I’ll look into it.

One thing that I can tell you from anecdotal experience is that I’ve also been in situations in which lowering --stack-size actually removed stack overflow exceptions surfacing; the reason being that they then were thrown from another part of the program that was part of a try/catch. Something like that may or may not be the case here.