-
Notifications
You must be signed in to change notification settings - Fork 36
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
Better capacity and growth rate for {Stream,Future}Group
#172
Conversation
@matheus-consoli apologies for taking a little while to respond; I was out all of last week. I love that you've implemented a more robust version of #168, and I'm 100% on board with solving #169 the way you've done it. However, I'm unsure whether using the In the current implementation we have two structs:
In the implementation provided by this PR we have a hierarchy of structs, at the top level there is:
Which internally uses a shared impl:
My worry is that this creates a funnel-shaped abstraction: we're specializing behavior at the top of the abstraction, going through a single shared type, and then specializing again at the bottom of the abstraction. I worry that this may work because I think for that reason we might be better served with just duplicating the strategy between both impls. And perhaps we can explore ways to share more logic between impls without specializing at the lower levels in future PRs? Would that be ok with you? |
sure, i agree with you my initial idea was to write a single type I will close this PR and open a simpler one. |
Fixes the same problem as #168 by keeping up with the capacity and growth rate of the group.
When a group tries to grow over its capacity, we double the previous capacity.
FutureGroup
andStreamGroup
are structurally very similar, they only differ on theStream
implementation, so I isolated the common structure to reduce the duplicated code.Performance
Compared to the main branch, we gained 99,775% using this benchmark (with 1_000_000 futures).
Before:
After:
todos
FutureGroup
andStreamGroup
type aliases for a common struct generic over the poll behaviorresize
toreserve
and expose itCloses #169