Skip to content
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

Add stream selection early exit #2597

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

414owen
Copy link
Contributor

@414owen 414owen commented May 10, 2022

select_early_exit creates a stream that produces elements while either substream contains elements, and terminates when either substream runs out of elements.

Previous work

#1881
#1422

Benchmarks:

Old New
1,065,438ns/iter 1,159,341ns/iter

Performance hit: +8.8%.

The select_streams benchmark is quite contrived, using long streams nested deeply under multiple Selects to produce statistically significant results. As such, I'm happy with the +8.8% performance hit, and would prefer it over a separate version of SelectWithStrategy just for early exit.

Side note, if you leave out the #[inline] over the is_finished function, you get 1,830,713ns/iter, or a +71.8% performance hit.

I've tried to specialize SelectWithStrategy over an exit strategy trait in this branch, but surprisingly, got a small performance hit.

I can't say for sure what's going on, but my best guess is that the performance difference between this branch and the one linked above is due to a layout change, rather than how the logic is encoded.

It would be possible to use something like stabilizer to know for sure, but I doubt we actually want to go that deep on this.

@414owen
Copy link
Contributor Author

414owen commented May 10, 2022

Clippy is failing (on master too!). Fixed here: #2598

@414owen 414owen force-pushed the early-exit-streams branch 2 times, most recently from 3c27fd3 to 7aefa94 Compare May 11, 2022 10:11
@taiki-e taiki-e added this to the futures-0.4, futures-core-1.0 milestone May 28, 2022
@taiki-e taiki-e added A-stream Area: futures::stream breaking change labels May 28, 2022
@414owen 414owen force-pushed the early-exit-streams branch 3 times, most recently from 9a34f07 to daeeec9 Compare June 1, 2022 17:32
`select_early_exit` creates a stream that produces elements while
either stream contains elements, and terminates when either stream
runs out of elements.
@414owen
Copy link
Contributor Author

414owen commented Jun 1, 2022

I've decided that I need more granularity than when both finished/when either finished.

The new API is trait-based.
Why is it trait-based? Because I tried implementing it with a function, and this got the best performance by far.
The function-based api is here: 414owen@eba9a37
Presumably the trait instance functions were inlined more aggressively.

With this change, we actually improve performance, beating master by a whopping 3.4%.

Old New
863,006 ns/iter 832,950 ns/iter

I don't really know why that is, but see my comment on layout changes in the PR description.

@taiki-e taiki-e added the S-needs-decision Status: A decision on whether or not to do this is needed. label Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stream Area: futures::stream breaking change S-needs-decision Status: A decision on whether or not to do this is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants