-
Notifications
You must be signed in to change notification settings - Fork 85
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
use sync.Pool
for runner pooling
#88
base: master
Are you sure you want to change the base?
Conversation
Thanks for this @Gusted -- what's the impact on CPU benchmarks around this change? |
It is quite noticeable in specific benchmarks, mostly due to be benchstat
|
Currently as noted in the comment of `putRunner`, there's no attempt being made to limit the size of the runner pooling - this can result in the pool containing a lot of runners that were once created in a spur but will likely not be used anymore. Instead of trying to do gc within this code, move the pooling to `sync.Pool` which will deallocated objects in idle and therefore keep the size of the pool as small as possible. The pool is on a per-regexp scope, this means certain properties can be re-used for optimal performance. The motivation for this change is that I'm seeing a lot of memory (~300MiB) being hold by these runners until the Go program is restarted which feels like an unoptimal usage of memory, with this change after a spur of these runners have been created in a small amount of time they are gracefully deallocated over time and no longer hold memory indefinitely.
The change is now much smaller and the performance is much better, overhead of atomic operations is a few ns which results in average 3.6% performance drop.
|
Currently as noted in the comment of
putRunner
, there's no attemptbeing made to limit the size of the runner pooling - this can result in
the pool containing a lot of runners that were once created in a spur
but will likely not be used anymore. Instead of trying to do gc within
this code, move the pooling to
sync.Pool
which will deallocated objectsin idle and therefore keep the size of the pool as small as possible.
The pool is on a per-regexp scope, this means certain properties can be
re-used for optimal performance.
The motivation for this change is that I'm seeing a lot of memory (~300MiB) being
hold by these runners until the Go program is restarted which feels like
an unoptimal usage of memory, with this change after a spur of these
runners have been created in a small amount of time they are gracefully
deallocated over time and no longer hold memory indefinitely.