Node: "inconvenient" stack overflow when using spread operator - changing stack size does not work

[olydis]

• Version: 7.10 (or most likely earlier) until 9.1.0 as far as I can tell
• Platform: Windows 10
• Subsystem: x64

Happy about recent ES features, we did stuff like someArray.push(...someOtherArray) which turns out to be problematic for large someOtherArray (say, 1 million elements), causing a stack overflow presumably because of the huge number of "arguments". There is the obvious workaround of loop-pushing (...with some helper method, since it's ugly to do that in tons of places), but given that the spread array doesn't even have too large for this to happen, I was also investigating stack size. I read about the --stack-size/--stack_size parameter, but setting that to anything significantly larger than 1000 on my Windows box caused the process to just silently crash instead of getting the nice stack overflow error.

I'm not expecting any magic, the error and its reason makes sense to me, I just wondered:

• is there a plan regarding enabling scenarios like push(...LARGE_ARRAY) since I assume that's the pattern people will happily use these days, unaware of the caveat
• is there any plan/way to support larger stacks on Windows?

Now that I wrote this, I think maybe these questions make more sense on the V8 side, but maybe you have valuable insights/ideas to share 🙂

[addaleax]

I read about the --stack-size/--stack_size parameter, but setting that to anything significantly larger than 1000 on my Windows box caused the process to just silently crash instead of getting the nice stack overflow error.

--stack-size doesn’t let you set the stack size; it tells V8 what to assume for the usable stack size. So when you set it to a value that exceed the actual size, V8 just runs past the end of the stack and crashes.

/cc @nodejs/v8 for the question about .push(...array)

[olydis]

Thanks for the clarification about stack size - it felt that way given that it crashed silently 👍

[vsemozhetbyt]

Some refs that I have stumbled upon in a similar case:

https://stackoverflow.com/questions/22123769/rangeerror-maximum-call-stack-size-exceeded-why

https://stackoverflow.com/questions/18308700/chrome-how-to-solve-maximum-call-stack-size-exceeded-errors-on-math-max-apply

https://stackoverflow.com/questions/3978492/javascript-fastest-way-to-duplicate-an-array-slice-vs-for-loop

[hashseed]

This is working as intended. The spread operator simply pushes all array elements onto the stack. You essentially have the same restrictions as Function.prototype.apply. You are better served doing something like

for (let e of someOtherArray) someArray.push(e)

[bmeurer]

How about using array spread instead?

someArray = [...someArray, ...someOtherArray];

[psmarshall]

Yeah this is one of the ways in which spread calls are surprising in JS. There is some more discussion about it on this bug.

• Is there a plan for allowing larger spreads?

Basically, no. In the bug above I mention a few things we thought about to enable that, but the benefits are kind of unclear and it won't be particularly easy. Other JS engines would also need to do the same thing to enable spread to be used widely in this way. I guess for node you know much more about the JS engine you are using, so it would be more useful there.

Unfortunately, the best advice I can give is to avoid spread calls unless you know something about the length of your array and thus the stack space it will use. It is still useful for some other things particularly in combination with rest parameters, but this style where you call Array push or Math max/min is a bit dangerous for exactly the reason you describe.

[olydis]

@hashseed @bmeurer Absolutely, note that this was not about how to work around this issue, but whether there are plans to support large spreads in calls. We used them "accidentally" since they are super convenient and while it makes perfect sense that you're exploding the stack, it's easy to miss. Also, tons of online resources will suggest stuff like that as the "ES6-way" of doing things, so I was curious whether there are plans to, say, implement calls differently in the future to work around that.

@psmarshall Thanks for the insights, that makes perfect sense!