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

Make it possible to disable the cross pool dispatch optimization. #765

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

Conversation

khuey
Copy link

@khuey khuey commented Jun 2, 2020

This optimization can lead to unexpected reentrancy when using multiple rayon threadpools.

@cuviper
Copy link
Member

cuviper commented Jun 13, 2020

I disagree with calling this work stealing an optimization -- as you note, there's a deadlock possibility without it. But reentrancy can cause its own deadlocks, so... it's complicated.

The other thing to to consider is whether this should really be a pool-wide option, versus a specific method for this behavior, perhaps blocking_install.

@nikomatsakis
Copy link
Member

@khuey can you say a bit more about what you are doing that you want this option? I guess you are invoking thread_pool.join() or thread_pool.scope() from some other thread-pool? What is the behavior you ultimately expect?

@khuey
Copy link
Author

khuey commented Jul 26, 2020

I'm on thread pool A and I invoke .scope() on thread pool B. I expect this to be a blocking operation (like it would be if instead of being on thread pool A I was on a non-rayon thread). Because it's not, we keep processing jobs on my existing thread. In my case I have a RefCell in TLS that's borrowed across the ThreadPoolB.scope() call so a job that's processed re-entrantly might try to borrow the same RefCell and die.

@cuviper
Copy link
Member

cuviper commented Jul 27, 2020

I expect this to be a blocking operation (like it would be if instead of being on thread pool A I was on a non-rayon thread).

Maybe we could document this better? We currently say, "it will block until all tasks that have been spawned into s complete," but that's blocking in the imperative sense, that the scope won't return at all until then. It doesn't really say what else we may be doing within this thread.

In my case I have a RefCell in TLS that's borrowed across the ThreadPoolB.scope() call so a job that's processed re-entrantly might try to borrow the same RefCell and die.

This would be a hazard for any kind of blocking, even local to one thread pool. For example, if the second half of a join is stolen, the current thread will itself participate in work-stealing while it waits.

I mentioned in #690 (comment) that we could have a "critical section" primitive that prevents work-stealing on the current thread. You would want that for the duration of your RefCell borrow, for example.

@nikomatsakis
Copy link
Member

I don't quite understand the ref-cell scenario. Ref-cells can't be shared across threads, so you must have some master task that owns the ref-cell. How could a re-entrant task try to access it?

@nikomatsakis
Copy link
Member

In other words, I believe that an task that we might execute while blocked here could also be stolen and executed by some other available thread if there were one -- am I missing something?

@khuey
Copy link
Author

khuey commented Jul 28, 2020

The RefCell lives in TLS.

@nikomatsakis
Copy link
Member

I see, so you have some "per-thread" resources and you want to "truly block". OK.

@nikomatsakis
Copy link
Member

Now that I understand the situation better, I think I agree with @cuviper that focusing on "cross-thread blocking" doesn't seem right -- this is really about wanting any sort of blocking to hold off on executing tasks. I guess that you're just very careful not to hold the ref-cell across any "intra-pool parallel operations", @khuey?

The "critical section" idea is interesting and seems somewhat more general -- I guess that there is never a need for a thread to do anything but block while it blocks, there should always be some thread still executing and making progress.

I could also imagine just configuring a thread-pool to have all blocking operations "just block" and not steal while blocking. I'm not sure @khuey if this would work for what you're doing or not.

@khuey
Copy link
Author

khuey commented Aug 3, 2020

I don't think we use any intra-pool parallel operations.

Edit (in the relevant section, anyways).

@nikomatsakis
Copy link
Member

@khuey OK, I guess the idea is that each task in pool A is a "leaf task" that doesn't spawn any more tasks, except for some in other pools?

@khuey
Copy link
Author

khuey commented Aug 1, 2022

I would like to dust this off so I can finally stop using scoped-pool for one of the thread pools.

I could also imagine just configuring a thread-pool to have all blocking operations "just block" and not steal while blocking. I'm not sure @khuey if this would work for what you're doing or not.

Yes, I believe this would work for me. Would you take a PR that implements this?

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