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 a mean to get the current threadpool #841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lu-zero
Copy link

@lu-zero lu-zero commented Mar 29, 2021

It is useful when you want to share the same threadpool across
threads outside the threadpool.

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 hesitate to expose ThreadPool like this because it leaves us with no clear owner of the pool. The Registry is shared because each thread in that pool also holds a handle, but it would be nice if ThreadPool remains a bit stronger, as the Drop semantics imply by calling registry.terminate(). There have also been requests to support a synchronized shutdown, which should naturally consume self by value, but that semantic doesn't work as well if ThreadPool is a shared handle.

Can you say more about what you want to do with this? Most of the free functions already work naturally with the current pool, whether that's global or something custom, so I wonder why you actually need the ThreadPool itself.

rayon-core/src/thread_pool/mod.rs Show resolved Hide resolved
It is useful when you want to share the same threadpool across
threads outside the threadpool.
@lu-zero
Copy link
Author

lu-zero commented Apr 8, 2021

So far I used it to println!() the threadpool to make sure which is which.

@nikomatsakis
Copy link
Member

Maybe we can return some kind of opaque id

@cuviper
Copy link
Member

cuviper commented Apr 9, 2021

What would you be able to do with an opaque id?

It seems to me what's wanted is just to run things on the pool, not share ownership, so maybe we need some kind of ThreadPoolRef mirroring ThreadPool's &self methods? But then there are still ownership questions -- does this act like Weak and fail/panic if the real ThreadPool owner has dropped? (since the threads will then be exiting...)

@nikomatsakis
Copy link
Member

@cuviper print it out so you can make sure which threadpool is which

@lu-zero
Copy link
Author

lu-zero commented Apr 13, 2021

It is also nice so I do not have the annoying pattern of

match maybe_threadpool {
  Some(pool) => pool.spawn(run),
  None => rayon::spawn(run),
}

I currently have in rav1e.

@nikomatsakis
Copy link
Member

Doesn't rayon::spawn use the current threadpool, if any? I forget. Maybe we can make a function for that?

@lu-zero
Copy link
Author

lu-zero commented Apr 14, 2021

Doesn't rayon::spawn use the current threadpool, if any? I forget. Maybe we can make a function for that?

My usecase is to not have to special case between a custom pool and the global pool. Some users would enjoy setting the threadpool according to certain constraint, others would just use the global threadpool they already set for their needs or share a threadpool otherwise created.

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.

3 participants