Add automaxprocs #2724
Add automaxprocs #2724
Conversation
| @@ -175,6 +176,13 @@ func init() { | |||
| } | |||
|
|
|||
| func execute() { | |||
| // update GOMAXPROCS to container cpu limit if necessary | |||
| _, err := maxprocs.Set(maxprocs.Logger(log.Info)) | |||
There was a problem hiding this comment.
I thought this library uses a package init function that runs as soon as you import the package
import _ "go.uber.org/automaxprocs"
Are we not setting it twice now (not sure if it matters much tbh)?
There was a problem hiding this comment.
Importing go.uber.org/automaxprocs triggers the init. Importing go.uber.org/automaxprocs/maxprocs as Anya has done here should not trigger it (in theory).
There was a problem hiding this comment.
I should have explained this better in the PR. The package docs describe the use case a little bit better:
// Package maxprocs lets Go programs easily configure runtime.GOMAXPROCS to
// match the configured Linux CPU quota. Unlike the top-level automaxprocs
// package, it lets the caller configure logging and handle errors.
So in our case it's just so we can inject our logger.
| @@ -175,6 +176,13 @@ func init() { | |||
| } | |||
|
|
|||
| func execute() { | |||
| // update GOMAXPROCS to container cpu limit if necessary | |||
| _, err := maxprocs.Set(maxprocs.Logger(log.Info)) | |||
|
jenkins test this please |
Follow up to #2720
Automatically sets GOMAXPROCS to the container cpu limit if it exists. Otherwise it defaults to the number of CPUs on the host. Our current default cpu limit is 1, so on big hosts golang may use many more processes than we can actually use.
Even if we do end up removing the default cpu limit in our default configuration, I think I would be in favor of keeping this to better support users who do end up setting a cpu limit.
Some further reading:
Benchmarks: uber-go/automaxprocs#12
And a thread with a lot more references: https://threadreaderapp.com/thread/1149654812595646471.html