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

Increase default stack size to 2MB with opt-out available #59

Merged

Conversation

ian-h-chamberlain
Copy link
Member

Fix / workaround for issue seen in #56

Add a Cargo feature to allow users to set the stack size if desired, but
default to 2MB to handle more use cases.

@Meziu @AzureMarker what do you think of this approach? Open to alternate naming suggestions on the cargo feature as well, I don't love big-stack but wanted to post this as a starting point for discussion.

Another approach I considered was exporting a macro like

ctru::stack_size!(64 * 1024);

But it felt a bit clunky and would require every user to set this if their program had large enough stacks. I would imagine that custom stack size is a pretty advanced use case (assuming we pick a good default) so I think it should still be possible for users to opt-out of the default, but I'm not sure of another way besides a Cargo feature.

Add a Cargo feature to allow users to set the stack size if desired, but
default to 2MB to handle more use cases.
Copy link
Member

@AzureMarker AzureMarker left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!!

@AzureMarker
Copy link
Member

The feature name seems good enough to me. We could in the future have both the feature and a macro to set the stack size.

@ian-h-chamberlain
Copy link
Member Author

Another option I considered was something like custom-stack-size and invert the semantics – i.e. configure this size when #[cfg(not(feature = "custom-stack-size"))]. But the guidance from documentation kidn of seems to discourage this (https://doc.rust-lang.org/cargo/reference/features.html#feature-unification):

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

So I guess this can be good enough for now.

@Meziu Meziu merged commit c09b2e8 into rust3ds:master May 4, 2022
@ian-h-chamberlain ian-h-chamberlain deleted the fix/increase-default-stack-size branch February 24, 2024 19:55
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.

3 participants