Remove automaxprocs dependency#8696
Conversation
This is handled natively by Go since 1.25, so this dependency should no longer be needed. See references below for more information. Only notable difference seems to be that Go sets a minimum value of 2 while the automaxprocs lib has a minimum value of 1. Go seems to account for much more though, so I don't think that difference alone warrants the inclusion of this dependency. Users who really want GOMAXPROCS=1 can always set that themselves. References: - golang/go#73193 - uber-go/automaxprocs#98 Signed-off-by: Anders Eknert <anders.eknert@apple.com>
|
This was what kept me from proposing this earlier: https://github.com/uber-go/automaxprocs/pull/99/changes#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R5-R6 Erring on the side of caution suggest to keep it, what do you think? |
|
Yep, the behavior is slightly different, but OTOH the stdlib one is likely to become the norm / expectation over time, so I think going with that is perhaps the better long-term bet. Have we even documented this anywhere? While that's... not great, it makes the current behavior feel less like a contract to me. So my preference would be to remove, with a note in the release notes for awareness. I doubt it'll happen, but should some users feel strongly about this, we can of course always revert. Or add a few lines of custom code I suppose, if it's only a matter of rounding. But I don't feel strongly about it myself, so if you think we better keep this as is, I don't mind closing. |
|
Thanks for your reply, and I'm with you. It's probably fine. After all, anyone with a custom build can put this into their |
|
My interpretation of the issues is that in Kubernetes pods with a CPU limit of <1, Go 1.25 gives min GOMAXPROCS=2, which can cause more throttling. The alternative is GOMAXPROCS=1, the minimum for automaxprocs, disables all parallelism and can lead to longer pauses while completing GC. Neither is perfect for <1 CPU quotas. My intuition is that for OPA, while both latency and pauses are bad, it feels to me like some throttling latency is better (slowness rather than spikes) as we have a lot of per request objects to clean up. |
This is handled natively by Go since 1.25, so this dependency should no longer be needed. See references below for more information. Only notable difference seems to be that Go sets a minimum value of 2 while the automaxprocs lib has a minimum value of 1. Go seems to account for much more though, so I don't think that difference alone warrants the inclusion of this dependency. Users who really want GOMAXPROCS=1 can always set that themselves.
References: