Bump default maximum stack size #11238
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Xavier Leroy (2022/05/04 06:40 -0700):
Now that stack allocation is entirely under the control of the OCaml runtime system, it's time to use more generous stack limits than those provided by operating systems.
This commit sets the default maximum stack size to 128 Miwords, i.e. 1 Gib for 64-bit platforms and 512 Mib for 32-bit platforms.
See #10736 and #10954 for context.
You can view, comment on, or merge this pull request online at:
#11238
-- Commit Summary --
* Bump default maximum stack size
-- File Changes --
M runtime/caml/config.h (3)
-- Patch Links --
https://github.com/ocaml/ocaml/pull/11238.patch
https://github.com/ocaml/ocaml/pull/11238.diff
How about documenting in which unit the stack size is expressed? I
assume it's in words but it could be helpful to write it explicitly in
the comment to make it easier to grasp why a constant size yields
different sizes on different platforms.
Also, I'm sorry, I didn't understand what you did in the test. You sem
to specify a constant size, still you say that it's to make the test
robust to increases of the stack size. Would you mind clarifying?
|
Contributor
Author
The unit is indeed words, and it's documented in the reference manual where the OCAMLRUNPARAM variable is documented, as well as in the
The test wants to trigger a stack overflow. After increasing the default max stack size, it would no longer trigger it. Hence the test must be run with a fixed max stack size that is known to trigger the stack overflow. |
Contributor
|
Xavier Leroy (2022/05/10 05:37 -0700):
> How about documenting in which unit the stack size is expressed? I assume it's in words
The unit is indeed words, and it's documented in the reference manual
where the OCAMLRUNPARAM variable is documented, as well as in the
`runtime/caml/config.h` source file. What extra documentation is
needed?
My bad, I missed the "(words)" bit. Sorry.
> Also, I'm sorry, I didn't understand what you did in the test. You sem
> to specify a constant size, still you say that it's to make the test
> robust to increases of the stack size. Would you mind clarifying?
The test wants to trigger a stack overflow.
Yes.
After increasing the default max stack size, it would no longer
trigger it. Hence the test must be run with a fixed max stack size
that is known to trigger the stack overflow.
Yes yes. I misunderstood that you were emphasizing the fact that it's
*fixed* wihch matters, but of course what matters is that it's larger
than the max size.
|
shindere
approved these changes
May 10, 2022
Contributor
|
@xavierleroy don't you want to add a Changes entry? To please |
Now that stack allocation is entirely under the control of the OCaml runtime system, it's time to use more generous stack limits than those provided by operating systems. This commit sets the default maximum stack size to 128 Miwords, i.e. 1 Gib for 64-bit platforms and 512 Mib for 32-bit platforms.
So that the test is robust against bumping the default max stack size.
Contributor
Author
|
Changes entry added, now merging ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that stack allocation is entirely under the control of the OCaml runtime system, it's time to use more generous stack limits than those provided by operating systems.
This commit sets the default maximum stack size to 128 Miwords, i.e. 1 Gib for 64-bit platforms and 512 Mib for 32-bit platforms.
See #10736 and #10954 for context.