Initial implementation #1
Conversation
ghost
mentioned this pull request
Cargo.toml
Outdated
|
|
||
| [dependencies] | ||
| crossbeam-epoch = { git = "https://github.com/stjepang/crossbeam-epoch", branch = "guard" } | ||
| crossbeam-utils = { git = "https://github.com/crossbeam-rs/crossbeam-utils" } |
There was a problem hiding this comment.
We published crossbeam-utils in crates.io. What about using it?
There was a problem hiding this comment.
Yeah, I should just publish a new version... The latest version was published a while ago.
| //! | ||
| //! Of course, there are many variations of this strategy. For example, sometimes it may be | ||
| //! beneficial for a thread to always [`steal`][Deque::steal] work from the top of its deque | ||
| //! instead of calling [`pop`] and taking it from the bottom. |
There was a problem hiding this comment.
I'm just curious, what would be the benefit of using fn Deque::steal() instead of fn Deque::pop()?
There was a problem hiding this comment.
Rayon uses steal instead of pop when in the "breadth-first" mode, which gives a pretty large speedup in Stylo. See rayon-rs/rayon#360 for more details.
| //! d.push('b'); | ||
| //! d.push('c'); | ||
| //! | ||
| //! assert_eq!(d.pop(), Some('c')); |
There was a problem hiding this comment.
In other data structures, we named to popping function try_pop() instead of pop(). I don't have a strong opinion on what's better, but I'd like to have a consistent name.
There was a problem hiding this comment.
I don't have a strong opinion either... It really depends. Here are some counterexamples:
Vec::pop(nottry_pop) returns anOption<T>.HashMap::remove(nottry_remove) returns anOption<V>.LinkedList::pop_front(nottry_pop_front) returns anOption<T>.BinaryHeap::pop(nottry_pop) returns anOption<T>.String::pop(nottry_pop) returns anOption<char>.
There was a problem hiding this comment.
Then I prefer pop. Let's use this name for queues and other data structures :)
There was a problem hiding this comment.
I have several comments on the implementation, but I'd like to merge it as-is as the initial version of crossbeam-deque, because (a slight variation of) this implementation in coco works for rayon and other projects.
Here are some changes I'd like to propose later:
-
interface: I want to expose a deque that uses the
Handleinterface rather than usingepoch::pin(), because nowHandleis one of our first-class API. Of course, we need to expose deque usingepoch::pin()for the majority of use cases. -
optimization: e.g. I think it's not necessary to
swapthe buffer when resizing. Actually I'm proving it (https://github.com/jeehoonkang/crossbeam-rfcs/blob/deque/text/2017-08-30-deque.md), but it's not done yet (even after 3 months!). I'm proposing this RFC with a corresponding PR to this repo.
| //! d.push('b'); | ||
| //! d.push('c'); | ||
| //! | ||
| //! assert_eq!(d.pop(), Some('c')); |
There was a problem hiding this comment.
Then I prefer pop. Let's use this name for queues and other data structures :)
ghost
merged commit 
This is the initial implementation, based upon the one in Coco, which was in turn based on the old implementation in Crossbeam and the deque crate.