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

[mono/win] Increase stack size (reserve) to 8MB #60265

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Oct 11, 2021

Fix #57141 in windows build.

Set the stack size (reserve) to 8MB, which is usually a default
on linux. This way we should get similar behavior on windows.

> dumpbin.exe /headers artifacts\bin\mono\Browser.wasm.Debug\cross\browser-wasm\mono-aot-cross.exe|Select-String -Pattern stack

      800000 size of stack reserve
        1000 size of stack commit

Fix dotnet#57141 in windows build.

Set the stack size (reserve) to 8MB, which is usually a default
on linux. This way we should get similar behavior on windows.
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

Should we be doing this on al Windows builds or just the cross compiler?

@radekdoulik
Copy link
Member Author

I think on all windows builds producing .exe files. So that we have similar stack size as on linux. It is not specific to cross compilers.

@lateralusX
Copy link
Member

lateralusX commented Oct 13, 2021

Did similar change on Mono couple of years ago, mono/mono@29a2383, so that will make default stack size for binary, 8MB for debug builds and matching CoreCLR stack size for release build 1.5 MB (mainly due to interpreter getting larger stackframes when running debug builds). All threads that we create later (through our PAL API) will get default stack size of 1MB (that part of the change is carried over to dotnet/runtime).

@lateralusX
Copy link
Member

lateralusX commented Oct 13, 2021

Reading up on the issue, so sounds like increasing the stack size is just a temporary fix for the underlying recursion problem in WASM AOT compiler?

@radekdoulik
Copy link
Member Author

Reading up on the issue, so sounds like increasing the stack size is just a temporary fix for the underlying recursion problem in WASM AOT compiler?

I think we can keep this change long term, to have similar run conditions for the executables on windows and linux.

For the underlying recursion I have opened #60266 where I would like to look closer at it.

@lateralusX
Copy link
Member

lateralusX commented Oct 13, 2021

I assume fix still keeps initial commit size of stack memory? If so we should just keep in mind that this will change stack size of main thread. All threads created through our PAL will continue to get default stack size of 1MB (for example managed threads) unless explicitly setting stack size to larger value. This will only affect runtime when run through mono-sgen.exe and won't have any impact when running using coreclr host. We also had similar discussion when doing similar change in mono/mono, mono/mono#14760 (comment).

@radekdoulik
Copy link
Member Author

Yes, this change should affect just .exe files we build. If we worry about mono-sgen.exe, I can make it affect just mono-aot-cross.exe.

@lateralusX
Copy link
Member

I think it is less of a concern in dotnet/runtime since its pretty much just used by the cross compilers.

@radekdoulik radekdoulik merged commit 7c4e606 into dotnet:main Oct 15, 2021
@radekdoulik
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1354497221

@github-actions
Copy link
Contributor

@radekdoulik an error occurred while backporting to release/6.0, please check the run log for details!

Error: @radekdoulik is not a repo collaborator, backporting is not allowed.

@lewing
Copy link
Member

lewing commented Oct 18, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1355427492

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Wasm][AOT] The AOT compiler fails silently when running on Windows
3 participants