builder: increase stack size margin for automatic stack allocation#5381
Open
digitalentity wants to merge 1 commit intotinygo-org:devfrom
Open
builder: increase stack size margin for automatic stack allocation#5381digitalentity wants to merge 1 commit intotinygo-org:devfrom
digitalentity wants to merge 1 commit intotinygo-org:devfrom
Conversation
Adjust the stack size margin to account for the tinygo_swapTask overhead.
There was a problem hiding this comment.
Pull request overview
This PR improves TinyGo’s automatic goroutine stack size calculation by adding a safety margin for context switches, accounting for stack usage in tinygo_swapTask that is not visible to Go-level call graph analysis.
Changes:
- Look up
tinygo_swapTaskin the computed call graph/symbol map and capture its frame size as context-switch overhead. - Add the context-switch overhead to all goroutine stack sizes that are compile-time Bounded.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // on every context switch. The static analysis correctly traces Go calls, | ||
| // but it cannot see into the assembly-level register push. | ||
| var contextSwitchOverhead uint64 | ||
| if swapFuncs, ok := functions["tinygo_swapTask"]; ok && len(swapFuncs) == 1 { |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When TinyGo calculates stack sizes automatically, it performs a static analysis of the call graph. While this analysis correctly identifies the stack requirements for Go functions, it cannot account for stack usage within assembly-level routines that are not visible to the high-level analysis.
Specifically,
tinygo_swapTaskpushes callee-saved registers onto the current goroutine's stack during every context switch. If this overhead is not accounted for, a goroutine might overflow its stack during a context switch even if its Go-level stack usage is within limits.With this change in
determineStackSizes, we now look up thetinygo_swapTaskfunction in the symbol table to determine its frame size. This context switch overhead is now added to the calculated stack size for all goroutines that have a bounded stack size.Fixes #5375