Skip to content

Workstealing for the scheduler #8356

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

Closed
wants to merge 1 commit into from
Closed

Conversation

toddaaro
Copy link
Contributor

@toddaaro toddaaro commented Aug 6, 2013

This pull request converts the scheduler from a naive shared queue scheduler to a naive workstealing scheduler. The deque is still a queue inside a lock, but there is still a substantial performance gain. Fiddling with the messaging benchmark I got a ~10x speedup and observed massively reduced memory usage.

There are still many locations for optimization, but based on my experience so far it is a clear performance win as it is now.

@brson
Copy link
Contributor

brson commented Aug 7, 2013

This is a great start, and I'm happy it was so easy to implement.

Most of my comments are minor:

  • There are some whitespace errors with extra spaces at the end of blank lines. These won't pass tidy.
  • The benchmarks should be modified to work with #[bench]
  • Please use a XorShift random number generator from std::rand. These are fast and still have a very random distribution.

Finally, the way that the vectors of work queues are managed here doesn't allow more schedulers that participate in work stealing to be added dynamically. This isn't important now, but I believe it will be necessary in the future. We don't necessarily need to fix this, but should have a plan for it.

@brson
Copy link
Contributor

brson commented Aug 7, 2013

Also, the notes about perf are very encouraging.

@graydon
Copy link
Contributor

graydon commented Aug 7, 2013

Wooo!

@toddaaro
Copy link
Contributor Author

toddaaro commented Aug 7, 2013

Re the RNG stuff:

This is a bit of a finicky issue. The actual "correct" solution is that we need to put an XorShift rng back in the task struct and expose it via Task::rng(), which would fetch it from TLS. I'll write that code up and put up a separate PR as it is an entirely separate issue.

@toddaaro
Copy link
Contributor Author

toddaaro commented Aug 7, 2013

Dynamically adding schedulers to the workstealing set looks like it could be simple. We could add a new type of scheduler message that wraps a new WorkQueue handle and to process it we could modify the queue vector. When creating a new scheduler to be inserted just send that message to every other scheduler. To make this work we would need a set of scheduler handles accessible someplace though.

…ork queue shared by each scheduler. Now there is a separate work queue for each scheduler, and work is "stolen" from other queues when it is exhausted locally.
bors added a commit that referenced this pull request Aug 8, 2013
This pull request converts the scheduler from a naive shared queue scheduler to a naive workstealing scheduler. The deque is still a queue inside a lock, but there is still a substantial performance gain. Fiddling with the messaging benchmark I got a ~10x speedup and observed massively reduced memory usage.

There are still *many* locations for optimization, but based on my experience so far it is a clear performance win as it is now.
@bors bors closed this Aug 9, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 18, 2022
Simplify if let statements

fixes: rust-lang#8288

---

changelog: Allowing [`qustion_mark`] lint to check `if let` expressions that immediatly return unwrapped value
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.

4 participants