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

introduce internal API and rebase future support to use it #344

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@nikomatsakis
Copy link
Member

nikomatsakis commented May 23, 2017

This PR introduces an (unstable) internal API into rayon-core that gives a lower-level, and unsafe, interface to the Rayon scheduler. In particular, you can do two things:

  • check if you are in a Rayon worker thread and block until a condition is true;
  • get a "scope handle" that lets you spawn jobs and propagate errors.
    • Scope handles can be obtained from a scope or a ThreadPool.

This API allows us to factor out the future support (also unstable, as it is built on an unstable API) into the rayon package. This means that we can move from futures 0.2 to futures 0.3 without a major release of rayon-core.

The main concern here is whether this unstable API is suitably generic or can be improved. Probably time will tell! Some concerns:

  • The internal API is unsafe. I initially hoped to a safe API, but eventually concluded that there was no way to express the lifetime invariants in a purely safe fashion that did not add some overhead.
  • The internal API uses Arc. Given that the API is unsafe, one might wish that it was suitably generic that you could implement both join and Scope::spawn on top of it without loss of efficiency. The latter is probably possible, but not join -- this is because join stack-allocates its tasks, and this API currently requires an Arc<Task> to spawn a task.
    • We do need some form of pointer for the unsafe code to work out.
    • We could perhaps use a *mut or something, but that's pushing even more unsafety.
  • The API has a lot of modules. I made it be internal::task::ScopeHandle, for example, and internal::worker::WorkerThread, rather than something flat. Not sure why I did that.

I suspect the unsafe bit is inevitable, but I also suspect we can do somewhat better than this API. Still seems worth landing this as a start.

@nikomatsakis nikomatsakis force-pushed the factor-out-futures branch from 3d5276f to 3c2e422 May 23, 2017

@torkleyy

This comment has been minimized.

Copy link
Contributor

torkleyy commented Jul 7, 2017

It would be great if rayon could move towards stabilizing the future API. Do you intend to merge this PR in the near future? Is there a good workaround I could use for futures until then?

@cuviper

This comment has been minimized.

Copy link
Member

cuviper commented Sep 16, 2017

I rebased this in #436.

@cuviper cuviper closed this Sep 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.