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

controls/filter: Avoid spread syntax with potentially large arrays #1293

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Feb 25, 2021

Push each value individually instead of all at once, which results in
more method calls but a much smaller call stack size. Alternatively,
Array.concat could be used, but this follows the pattern of surrounding
code and avoids reassignment of "options", which would also necessitate
removal of the "const" declaration.

The /tb/global build has ~277k genotype states, which resulted in a call
to Array.push with as many arguments when the spread syntax was used.
This blew through the call stack size limit on Chrome with an error
like:

RangeError: Maximum call stack size exceeded
    at FilterData.eval [as makeOptions] (filter.js?6bcb:65)
    at FilterData.render (filter.js?6bcb:100)
    …

Firefox was unaffected, so presumably has a larger limit.

Debugging was waylaid for a bit by the assumption that exceeding the
call stack size necessarily meant deep recursion, but the lack of a deep
stack trace led to the realization that it can also occur when a
function's arguments are too many.

Resolves #1292.

@jameshadfield jameshadfield temporarily deployed to auspice-filter-call-sta-0kp6mr February 25, 2021 21:24 Inactive
Push each value individually instead of all at once, which results in
more method calls but a much smaller call stack size.  Alternatively,
Array.concat could be used, but this follows the pattern of surrounding
code and avoids reassignment of "options", which would also necessitate
removal of the "const" declaration.

The /tb/global build has ~277k genotype states, which resulted in a call
to Array.push with as many arguments when the spread syntax was used.
This blew through the call stack size limit on Chrome with an error
like:

    RangeError: Maximum call stack size exceeded
        at FilterData.eval [as makeOptions] (filter.js?6bcb:65)
        at FilterData.render (filter.js?6bcb:100)
        …

Firefox was unaffected, so presumably has a larger limit.

Debugging was waylaid for a bit by the assumption that exceeding the
call stack size necessarily meant deep recursion, but the lack of a deep
stack trace led to the realization that it can also occur when a
function's arguments are too many.

Resolves #1292.
@tsibley tsibley force-pushed the filter-call-stack-size branch from 2fa6a88 to 7f0787a Compare February 25, 2021 22:27
@tsibley tsibley temporarily deployed to auspice-filter-call-sta-0kp6mr February 25, 2021 22:27 Inactive
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jameshadfield jameshadfield merged commit f4fa3fd into master Feb 26, 2021
@jameshadfield jameshadfield deleted the filter-call-stack-size branch February 26, 2021 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large datasets will result in a RangeError
2 participants