Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arrayBind causes Maximum call stack size exceeded #309

Closed
acple opened this issue Jan 24, 2024 · 0 comments · Fixed by #314 · May be fixed by #311
Closed

arrayBind causes Maximum call stack size exceeded #309

acple opened this issue Jan 24, 2024 · 0 comments · Fixed by #314 · May be fixed by #311

Comments

@acple
Copy link

acple commented Jan 24, 2024

In short,

> import Data.Array
> [1] >>= \_ -> replicate 200000 0
file:///tmp/p/.psci_modules/Control.Bind/foreign.js:5
      Array.prototype.push.apply(result, f(arr[i]));
                           ^

RangeError: Maximum call stack size exceeded
    at file:///tmp/p/.psci_modules/Control.Bind/foreign.js:5:28
    at file:///tmp/p/.psci_modules/$PSCI/index.js:5:74
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)

Node.js v20.5.1

When the return value is just a huge array, RangeError is raised immediately, by passing too many arguments to Array.prototype.push function.
Of course this is very rare case but I think it should not be a purescript limitation.
Do someone know any solutions with minimum performance impact?

michaelficarra added a commit to michaelficarra/purescript-prelude that referenced this issue Jan 25, 2024
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
Demonstrating that the current implementation of `Array`'s `Bind` instance
causes `RangeError: Maximum call stack size exceeded` when the output of `f` in
`ma >>= f` is sufficiently large.

This is due to usage of `Function.prototype.apply`. From
[MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions):

> But beware: by using apply() (or the spread syntax) with an arbitrarily long
arguments list, you run the risk of exceeding the JavaScript engine's argument
length limit.

> The consequences of calling a function with too many arguments (that is, more
than tens of thousands of arguments) is unspecified and varies across engines.
(The JavaScriptCore engine has a hard-coded [argument limit of
65536](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions).)

Node v20.18.1 seems to have a higher limit around 106,000.
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
Demonstrating that the current implementation of `Array`'s `Bind` instance
causes `RangeError: Maximum call stack size exceeded` when the output of `f` in
`ma >>= f` is sufficiently large.

This is due to usage of `Function.prototype.apply`. From
[MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions):

> But beware: by using apply() (or the spread syntax) with an arbitrarily long
arguments list, you run the risk of exceeding the JavaScript engine's argument
length limit.

> The consequences of calling a function with too many arguments (that is, more
than tens of thousands of arguments) is unspecified and varies across engines.
(The JavaScriptCore engine has a hard-coded [argument limit of
65536](https://webkit.org/b/80797).)

Node v20.18.1 seems to have a higher limit around 106,000.
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 10, 2025
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 15, 2025
Using static check to determine if `Array.prototype.flatMap` is available, and
use `var` instead of `let` in for loop to match existing code style.
pete-murphy added a commit to pete-murphy/purescript-prelude that referenced this issue Feb 15, 2025
Using static check to determine if `Array.prototype.flatMap` is available, and
use `var` instead of `let` in for loop to match existing code style.
natefaubion pushed a commit that referenced this issue Feb 15, 2025
* test(#309): Failing test

Demonstrating that the current implementation of `Array`'s `Bind` instance
causes `RangeError: Maximum call stack size exceeded` when the output of `f` in
`ma >>= f` is sufficiently large.

This is due to usage of `Function.prototype.apply`. From
[MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions):

> But beware: by using apply() (or the spread syntax) with an arbitrarily long
arguments list, you run the risk of exceeding the JavaScript engine's argument
length limit.

> The consequences of calling a function with too many arguments (that is, more
than tens of thousands of arguments) is unspecified and varies across engines.
(The JavaScriptCore engine has a hard-coded [argument limit of
65536](https://webkit.org/b/80797).)

Node v20.18.1 seems to have a higher limit around 106,000.

* fix(#309): Use `flatMap` if supported by runtime

* fix(#309): Use simple stack-safe fallback

* chore(#309): Add to CHANGELOG.md

* feat(#309): Address feedback from code review

Using static check to determine if `Array.prototype.flatMap` is available, and
use `var` instead of `let` in for loop to match existing code style.

---------

Co-authored-by: Peter Murphy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant