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

util/growstack: investigate Go 1.19 stack growth #88232

Open
erikgrinaker opened this issue Sep 20, 2022 · 1 comment
Open

util/growstack: investigate Go 1.19 stack growth #88232

erikgrinaker opened this issue Sep 20, 2022 · 1 comment
Labels
C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Sep 20, 2022

As seen in #88038, the upgrade to Go 1.19 caused a significant performance regression that was found to be due to the new stack growth behavior in Go 1.19. The release notes say:

The runtime will now allocate initial goroutine stacks based on the historic average stack usage of goroutines. This avoids some of the early stack growth and copying needed in the average case in exchange for at most 2x wasted space on below-average goroutines.

We already have pkg/util/growstack which will initialize stacks to at least 32 KB:

// Grow grows the goroutine stack by 16 KB. Goroutine stacks currently start
// at 2 KB in size. The code paths through the storage package often need a
// stack that is 32 KB in size. The stack growth is mildly expensive making it
// useful to trick the runtime into growing the stack early. Since goroutine
// stacks grow in multiples of 2 and start at 2 KB in size, by placing a 16 KB
// object on the stack early in the lifetime of a goroutine we force the
// runtime to use a 32 KB stack for the goroutine.
func Grow()

The fix for the performance regression was to double the initial stack size here, as seen in #88187.

We should investigate further why doubling the initial stack size helped, and whether we can avoid using the growstack package at all. One possibility is that the old behavior would double the stack size when growing it, while the new behavior possibly uses a smaller multiple, leading to more frequent growing.

We should also communicate this upstream to the Go team and see if the runtime needs improvements.

Jira issue: CRDB-19739

@erikgrinaker erikgrinaker added the C-performance Perf of queries or internals. Solution not expected to change functional behavior. label Sep 20, 2022
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Development

No branches or pull requests

1 participant