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

[untested] v8(arm): use the same stack size as most other architectures #46896

Closed
wants to merge 1 commit into from
Closed

[untested] v8(arm): use the same stack size as most other architectures #46896

wants to merge 1 commit into from

Conversation

jayaddison
Copy link
Contributor

@jayaddison jayaddison commented Mar 1, 2023

May resolve mitigate #41163.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Mar 1, 2023
@jayaddison jayaddison changed the title [untested] Bring the ARM stack size into line with almost all other architectures (984 KB) [untested] v8(arm): use the same stack size as most other architectures Mar 1, 2023
@mscdex
Copy link
Contributor

mscdex commented Mar 1, 2023

Changes to V8 should be made upstream.

@jayaddison jayaddison closed this Mar 1, 2023
@jayaddison jayaddison deleted the issue-41163/harmonize-v8-stack-sizes branch March 1, 2023 17:41
@jayaddison
Copy link
Contributor Author

Thanks, @mscdex - I may need to spend some more time thinking about the options before deciding whether to take this upstream; it's good to know that those are generally where V8 changes affecting Node should be applied.

In particular in this case, my understanding so far is that the reduced V8 stack sizes for ARM were introduced primarily to resolve failures in single-process, web-related use cases -- I figured that those would be less relevant for Node.

(and, as mentioned in the linked issue thread: a better solution is likely to use rlimit-based detection of appropriate stack sizes.. potentially again a topic of conversation to have with the V8 team, though)

@jayaddison
Copy link
Contributor Author

⚠️ Although this wasn't merged, a caution for any future readers: after discussion this with the V8 development team on their v8-dev mailing list: it may currently be safe to restore the 984KB stack size for 32-bit ARM architectures, but it is not currently considered safe to do so for 64-bit ARM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants