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

Stabilize spawning an asynchronous task and convenience methods on ThreadPool #371

Merged
merged 3 commits into from Jun 14, 2017

Conversation

nikomatsakis
Copy link
Member

Building on #368, this branch stabilizes the spawn API and a bunch of convenience methods in ThreadPool:

  • rayon_core::ThreadPool::join
  • rayon_core::ThreadPool::scope
  • rayon_core::ThreadPool::spawn
  • rayon_core::spawn -- runs async task in current (or global) thread-pool

The only one that enables something fundamentally new that wasn't available in a stable API is rayon_core::spawn. The ThreadPool APIs are convenience wrappers around other APIs.

The following APIs remain unstable:

  • ThreadPool::global -- I don't know that we want to expose th Arc
    here, although it's no real commitment
  • everything related to futures -- that is all planned to change

Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

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

I have questions about #368 which you stacked this on, but I'm happy with the selected stabilizations.

ThreadPool::global -- I don't know that we want to expose the Arc here, although it's no real commitment

It's being &'static Arc that's weird to me. I don't think we have any APIs needing Arc<ThreadPool>, and 'static already proves it won't go away. (I've forgotten the reason why this is even Arc at all...)

- `rayon_core::ThreadPool::join`
- `rayon_core::ThreadPool::scope`
- `rayon_core::ThreadPool::spawn`
- `rayon_core::spawn` -- runs async task in current (or global) thread-pool

The only one that enables something fundamentally new that wasn't
available in a stable API is `rayon_core::spawn`. The `ThreadPool` APIs
are convenience wrappers around other APIs.

The following APIs remain **unstable**:

- `ThreadPool::global` -- I don't know that we want to expose th `Arc`
  here, although it's no real commitment
- everything related to futures -- that is all planned to change
@nikomatsakis
Copy link
Member Author

I've forgotten the reason why this is even Arc at all...

I'm not sure the reason is super good. I think it was because I found that some code I was using wanted to have an Arc<ThreadPool> so that it owned the thread-pool, and this way that thread-pool could be either locally created or the global one. I predicted that using Arc would be a common choice because it's handy to share the thread-pool widely.

But maybe just returning &'static ThreadPool is better, and people can use Cow or something to accommodate the "maybe shared" case (but Cow doesn't play nice with Arc...).

@nikomatsakis nikomatsakis merged commit 248113e into master Jun 14, 2017
@seanmonstar
Copy link

I haven't tested yet, but wouldn't a crate be able to automatically add this cfg in using a build.rs, without consumers knowing?

@cuviper
Copy link
Member

cuviper commented Jun 15, 2017

@seanmonstar I believe that can only enable a cfg for that crate itself, not its build of rayon.

@nikomatsakis
Copy link
Member Author

@seanmonstar I don't know, maybe. If so, I mean, what ya' gonna do...

@cuviper cuviper deleted the stabilize-spawn-async-etc branch October 6, 2017 21:40
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.

None yet

3 participants