-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 per-scheduler stack pools (let's recycle) #14100
Use per-scheduler stack pools (let's recycle) #14100
Conversation
f82fc19
to
d9db37b
Compare
NOTE: I rebased the branch on top of #14099 which was required (must assign each stack collector fiber to its own thread). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
It's particularly nice that this simplifies the code quite a bit.
The stack pool was only used to create new stacks when MT was enabled. The stacks were then pushed to each scheduler's free stacks, and eventually dropped on reschedule (after context swap, so we're sure to only ever deallocated stacks that are no longer used). This led the stacks to never be reused with MT. Only created then deallocated. This patch changes the behavior to have a stack pool running on each scheduler, and to use it to create and collect the stacks, and reuse them when possible. It also drops the mutex since the stack pool can never be accessed in parallel (in fact it never was). Also starts a collecting fiber on each thread. It may only lead to better performance if there are different fibers, running on multiple threads that are spawning fibers. It won't have much (or any) impact if there is only one fiber spawning other fibers (e.g. a HTTP::Server) as the stack of fibers that run on another thread won't be reused (different stack pool).
d9db37b
to
ea8a3c1
Compare
Rebased from master & force pushed to remove the duplicate commit (set current thread) from #14099 |
The stack pool was only used to create new stacks when MT was enabled. The stacks were then pushed to each scheduler's free stacks, and eventually dropped on reschedule (after context swap, so we're sure to only ever deallocate stacks that are no longer used). This led the stacks to never be reused with MT. Only allocated, put back into a pool, then deallocated.
This patch changes the behavior to have a stack pool running on each scheduler, and to use it to create and collect the stacks, and reuse them when possible. It also drops the mutex since the stack pool can never be accessed in parallel (in fact it never was).
Also starts a collecting fiber on each thread.
Stacks won't necessarily return to the pool that created them. For example if thread A allocates a stack, then the scheduler sends the fiber to thread B, then the stack will be returned to thread B's scheduler.
It may thus only lead to better performance if there are different fibers, running on multiple threads that are spawning fibers. It won't have much (or any) impact if there is only one fiber spawning other fibers (e.g. a HTTP::Server).