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

task: implement FromIterator for JoinSet #6300

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

erikdesjardins
Copy link
Contributor

@erikdesjardins erikdesjardins commented Jan 23, 2024

Motivation

When using JoinSet I usually end up populating it from an iterator, which would be more convenient with collect.

Solution

Implement FromIterator.

(Possibly, spawning each element of the iter is too magical / side-effecty for a FromIterator impl though.)

Closes: #6132

@maminrayej maminrayej added A-tokio Area: The main tokio crate M-task Module: tokio/task labels Jan 23, 2024
@maminrayej
Copy link
Member

maminrayej commented Jan 23, 2024

Related to issue #6132.

@Darksonn does this have conflicts with #5924?
My understanding is that this would have conflicts if it collected handles. Since this implementation spawns the futures using the JoinSet itself, it's fine.

@hawkw
Copy link
Member

hawkw commented Jan 23, 2024

@Darksonn does this have conflicts with #5924?
My understanding is that this would have conflicts if it collected handles. Since this implementation spawns the futures using the JoinSet itself, it's fine.

If we were to make the change described in #5924, we could potentially add a FromIterator impl where the iterator's Item is JoinHandle. This would, potentially, be more flexible in terms of how the tasks in the iterator are spawned, because the current FromIterator impl spawns the tasks using JoinSet::spawn, but an iterator of JoinHandles could also contain JoinHandles that were spawned using spawn_local or spawn_blocking. However, because JoinHandle implements Future, the FromIterator implementation for iterators of JoinHandles would conflict with the FromIterator implementation for iterators of F: Futures.

Note that you could pass an iterator of JoinHandles to the current FromIterator implementation, but it would spawn a second task that just awaits the JoinHandle's completion, which introduces some unnecessary overhead (including an additional task allocation). But, it's worth mentioning that JoinHandles can be used with the interface in this PR, even if they probably shouldn't.

However, I think the conclusion from #5924 was that we're probably not going to add an API for adding JoinHandles to a JoinSet, because we'd like to eventually make an internal optimization in JoinSet where the JoinSet linked list nodes are inlined into the task's allocation (see #5924 (comment)). Therefore, I don't really think we should worry too much about the potential addition of a FromIterator implementation for Iterator<Item = JoinSet>, because we're probably not going to do that...

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@Darksonn Darksonn merged commit 0b31b2a into tokio-rs:master Feb 2, 2024
79 checks passed
@erikdesjardins erikdesjardins deleted the collect branch February 2, 2024 17:34
kodiakhq bot pushed a commit to pdylanross/fatigue that referenced this pull request Mar 29, 2024
Bumps tokio from 1.36.0 to 1.37.0.

Release notes
Sourced from tokio's releases.

Tokio v1.37.0
1.37.0 (March 28th, 2024)
Added

fs: add set_max_buf_size to tokio::fs::File (#6411)
io: add try_new and try_with_interest to AsyncFd (#6345)
sync: add forget_permits method to semaphore (#6331)
sync: add is_closed, is_empty, and len to mpsc receivers (#6348)
sync: add a rwlock() method to owned RwLock guards (#6418)
sync: expose strong and weak counts of mpsc sender handles (#6405)
sync: implement Clone for watch::Sender (#6388)
task: add TaskLocalFuture::take_value (#6340)
task: implement FromIterator for JoinSet (#6300)

Changed

io: make io::split use a mutex instead of a spinlock (#6403)

Fixed

docs: fix docsrs build without net feature (#6360)
macros: allow select with only else branch (#6339)
runtime: fix leaking registration entries when os registration fails (#6329)

Documented

io: document cancel safety of AsyncBufReadExt::fill_buf (#6431)
io: document cancel safety of AsyncReadExt's primitive read functions (#6337)
runtime: add doc link from Runtime to #[tokio::main] (#6366)
runtime: make the enter example deterministic (#6351)
sync: add Semaphore example for limiting the number of outgoing requests (#6419)
sync: fix missing period in broadcast docs (#6377)
sync: mark mpsc::Sender::downgrade with #[must_use] (#6326)
sync: reorder const_new before new_with (#6392)
sync: update watch channel docs (#6395)
task: fix documentation links (#6336)

Changed (unstable)

runtime: include task Id in taskdumps (#6328)
runtime: panic if unhandled_panic is enabled when not supported (#6410)

#6300: tokio-rs/tokio#6300
#6326: tokio-rs/tokio#6326
#6328: tokio-rs/tokio#6328
#6329: tokio-rs/tokio#6329
#6331: tokio-rs/tokio#6331
#6336: tokio-rs/tokio#6336
#6337: tokio-rs/tokio#6337


... (truncated)


Commits

9c337ca chore: prepare Tokio v1.37.0 (#6435)
e542501 io: document cancel safety of AsyncBufReadExt::fill_buf (#6431)
4601c84 stream: add next_many and poll_next_many to StreamMap (#6409)
deff252 util: document cancel safety of SinkExt::send and StreamExt::next (#6417)
4565b81 sync: add  a rwlock() method to owned RwLock guards (#6418)
3ce4720 sync: add is_closed, is_empty, and len to mpsc receivers (#6348)
8342e4b util: assert compatibility between LengthDelimitedCodec options (#6414)
4c453e9 readme: add description about benchmarks (#6425)
1846483 sync: expose strong and weak counts of mpsc sender handles (#6405)
baad270 sync: add Semaphore example for limiting the number of outgoing requests (#6419)
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-task Module: tokio/task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement FromIter for JoinSet
4 participants