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

refactoring: have latches tickle registries directly #691

Closed
wants to merge 3 commits into from

Conversation

nikomatsakis
Copy link
Member

As a start to improving our sleep system, this PR does a refactoring to how tickle works. We used to tickle the registry after each job executed: the idea was that executing a job would set some latch, and we needed to wake up any threads that might be blocked, waiting on that latch. However, this was a bit overapproximated: for example, the latch might've been a LockLatch, which indicates a thread from outside the pool, in which case there is no need to tickle threads at all.

In this PR, the latches themselves track the registry that they must tickle when they are set. In the case of a Countdown latch, they are given it from the outside; this is because countdown latches are sometimes owned by the registry itself, so it would be impossible for them to have a reference to the registry.

I'd like to work towards a scenario where we know not only the registry that must be awoken but the exact helper thread. This could avoid needless wakeups. But this refactoring is as far as I got for now.

This eliminates the need for a TickleLatch, since the spin latch can
just track the registry that it needs to wake up directly.
The latches now tickle the appropriate registry directly, instead.
@ktaeleman
Copy link

Tried this patch and it does seem to reduce our cpu load immensely. Thanks!

@cuviper
Copy link
Member

cuviper commented Sep 23, 2019

Is this still relevant / worth reviewing on its own, in relation to rayon-rs/rfcs#5?

@nikomatsakis
Copy link
Member Author

closing in favor of #746

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