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

Support spawning asynchronous tasks #212

Merged
merged 16 commits into from
Feb 26, 2017
Merged

Support spawning asynchronous tasks #212

merged 16 commits into from
Feb 26, 2017

Conversation

nikomatsakis
Copy link
Member

@nikomatsakis nikomatsakis commented Jan 17, 2017

This is the successor to #161. This branch adds support for two new top-level functions (there are also corresponding methods on ThreadPool):

  • spawn_async() -- executes a FnOnce() for side-effects in the Rayon threadpool. Like Scope::spawn(), but no scope is needed, and hence the closure must be 'static.
  • spawn_future_async() -- executes a future in the Rayon threadpool. Like Scope::spawn_future(), but no scope is needed, and hence the future must be 'static.

The use-case here is that sometimes you would like to have work that executes asynchronously from your main thread (so that you don't have to block for it). You'd prefer for that job to share a thread-pool with the rest of your parallel work. You can now use spawn_async for that purpose. @glennw encountered just such a scenario in Servo's texture cache. (I think?)

WIP Aspects

There are a number of things I am not sure of where I would like feedback:

  • I'm not sure of the async naming. It seems to suggest async-await. Perhaps detach would be better.
  • I've added a panic handler that can be used to handle cases where panics occur that have no obvious place to be propagated to. An example would be a panic in the spawn_async() function.
    • The current default behavior if no panic handler is set is to abort. The principle here is that one ought not to have an unobserved panic. Does this make sense, or should we dump to stderr? Aborting seems pretty strict!
    • Similarly, what should we do if this "last resort" panic handler panics?
    • Similarly, what if a future panics but its result is dropped? This currently means the panic is ignored, which seems in line with the general future principle that dropping the value without polling it means you don't care about the result.

To Do Items

This isn't terribly polished but I wanted to bring it up to start getting some feedback.

  • Tests around termination.
  • Tests for spawn_future_async -- pretty minimal right now.

@nikomatsakis
Copy link
Member Author

@cuviper I'm planning to merge this after adding a few more tests. Last chance to complain. 👅

@nikomatsakis nikomatsakis changed the title WIP: support spawning asynchronous tasks Support spawning asynchronous tasks Feb 26, 2017
For now, we abort in various anomalous scenarios. This is likely not the
right default.
This fixes a latent bug in the splitting code for the parallel iterator,
which would have grabbed the wrong number of threads if we were working
inside a custom threadpool.
This will be useful when we permit spawning async jobs.
It is no longer dependent on scope, and instead serves a cross-cutting
role.
The unsafe keyword was not needed, because the safety is implied
by the unsafe trait `FutureScope`.

The `future` module now re-exports the relevant parts of `futures` crate
for convenience in the rest of Rayon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant