Repository navigation
Conversation
The leaking GC never frees memory, so every goroutine that exited lost its stack. A program that starts many short lived goroutines ran out of memory, for example a test binary with many subtests, since testing.T.Run starts a goroutine for every subtest. With the leaking GC, keep the stacks of exited goroutines in a list and give them to new goroutines of the same stack size. Stacks are zeroed on reuse.
There was a problem hiding this comment.
🟢 Approval recommended
No blocking issues remain in the collector-specific stack reuse, exit handling, or regression-test integration.
No open issues 🎉
What changed in this PR
Reuses exited goroutine stacks under asyncify with the leaking GC, reducing memory growth in workloads with many short-lived goroutines.
Changes:
- Reuses and zeroes stacks of matching sizes.
- Preserves stack allocation and release behavior for other collectors.
- Adds a 100,000-goroutine regression test.
| File | Description |
|---|---|
| testdata/goroutines-leaking.txt | Records expected test output. |
| testdata/goroutines-leaking.go | Exercises stack reuse across goroutine batches. |
| src/internal/task/task_asyncify.go | Tracks stack sizes and delegates allocation and release. |
| src/internal/task/task_asyncify_stack.go | Preserves behavior for non-leaking collectors. |
| src/internal/task/task_asyncify_stack_leaking.go | Implements stack reuse and zeroing. |
| main_test.go | Registers the leaking-GC regression test for wasip1. |
Review effort: Balanced
Member
|
Two comments from my robot I asked it about the dead guard claim, |
Every goroutine gets the same stack size with Asyncify, so the search for a free stack of the right size always stops at the first entry. Say so where the list is searched. releaseStack skipped stacks too small to hold a freeStack. A stack that small cannot run main, so no goroutine exits with one and the check never fires.
Contributor
Author
|
Added these in f1c2d1f. |
Member
|
I'd say we could consider trying to apply this to other non-asyncify platforms, but I'm guessing if you're running on a pico with |
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.
I'll be very upfront and say: I'm not sure about this PR. But I would like opinions on it.
Beginning in 0.42,
testingstarted a goroutine per subtest. This interacts poorly with-gc=leakingwhen you have a lot of subtests, because goroutine stacks are discarded and never released. In a production project we have over 100,000 subtests that run, and starting with 0.42 we now run out of memory after running about 4,000.This pull request, when
scheduler.asyncifyandgc.leakingare both enabled, puts used stacks onto a free list when the goroutine exits and will reuse them when subsequent goroutines begin. The reason why I'm on the fence about it is because then it's not really "leaking." However, one of the reasons to use the leaking GC mode is because you don't want the overhead of accounting for and processing allocations. This adds accounting but there's no real cost to it.Stacks are zeroed when reused. The overhead for reusing a 64 KB stack over creating a new one is about 400 ns. (It is substantially lower overhead if the stack is not zeroed.)