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

Remove register stack size override in hermes.cpp #969

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

lunaleaps
Copy link

neildhar and others added 2 commits April 12, 2023 14:03
Summary:
We are currently overriding any specified register stack size with a
size that is fairly low. This was added to allow the register stack to
be placed on a thread stack when StackRuntime is enabled, however, it
seems like that functionality was removed some time ago, so this limit
no longer serves a purpose.

Remove it so that sizes set through RuntimeConfig are actually
honoured. Set the default RuntimeConfig value to be roughly what the
old limit was (no need to suddenly increase the stack size if we
haven't had problems yet). Keeping the limit low helps us retain the
ability to place the register stack in StackRuntime again in the future.

Reviewed By: mhorowitz

Differential Revision: D42610775

fbshipit-source-id: 94db4e3eb1d6365749a6db8adfe73a5d298f127c
Summary:
As discussed in facebook#135, the default stack size doesn't work for all use cases. In particular, when very large and complex bundles are loaded in dev mode.

This PR bumps the default stack size from `64*1024` (512kB) to `128*1024` (1MB). As suggested by tmikov in this comment - facebook#135 (comment).

Pull Request resolved: facebook#923

Reviewed By: tmikov

Differential Revision: D43630032

Pulled By: neildhar

fbshipit-source-id: 5f8cff91a5f01b6507870c61efa1ce507de67940
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 12, 2023
@lunaleaps lunaleaps merged commit c9b539b into facebook:rn/0.70-stable Apr 13, 2023
@lunaleaps lunaleaps deleted the rn/0.70-stable branch April 13, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants