Steal-half in Chase-Lev #148
Conversation
Questions: * We could change the signature to make callers supply a vec, rather than allocate our own in `steal_half`. This could potentially be more efficient if the caller is repeatedly retrying. * Is this safe? I think so, but I'm not 100% sure. The paper mentions that it would be interesting to apply a steal-half operation to this deque, but they didn't do it. I couldn't find any other papers or libraries that implemented this operation either, although my google-fu may be weak in this regard. The most important part, as far as I can tell, is populating the results array before performing the CAS to ensure that we get the correct set of resulting items (after the CAS, the circular buffer could circle back over the slots we're stealing and overwrite them with new values). Everything seems to be pretty much the same as stealing one item, which leads to my next question. * Should we factor out the common parts between `steal` and `steal_half`? There's a ton of duplication here, as I've authored it. If you think this is valuable, then I can write some tests and fix this up.
| } else { | ||
| while data.len() > 0 { | ||
| mem::forget(data.pop()); | ||
| } |
There was a problem hiding this comment.
You can rewrite this loop in a different, perhaps more idiomatic way:
while let Some(x) = data.pop() {
mem::forget(x);
}| data.push(a.get(i)); | ||
| } | ||
|
|
||
| if self.top.compare_and_swap(t, t + half, SeqCst) == t { |
There was a problem hiding this comment.
What happens if just before this CAS the worker thread pops most of the elements, so that t + half overshoots the bottom?
There is an if-statement in fn pop where we deal with a special racy case when popping the last element. With the presence of the steal-half operation, now steal_half and pop can race in more complex ways.
The racy case in fn pop is currently handled by incrementing top rather than decrementing bottom. When racing with steal-half we can't perform the same trick.
Perhaps this is the difficulty with implementing steal-half we've been missing all along? :)
There was a problem hiding this comment.
@stjepang is right -- my bad. This implementation of steal_half() is wrong, and I cannot imagine how to fix it.
The reason I thought it was correct is I only considered races among stealers. As @stjepang said, b can be decremented beyond t + half before a steal_half() is successfully stealing from t to t + half. That breaks deque's invariant.
Thank you pointing out that. I think I should be more humble in front of concurrency... 🤕 I am closing this PR.
@fitzgen this PR is finally resolved, but negatively. I will work more on designing a proper steal_half() in deque!
|
I made another attempt for After proving the correctness of Chase-Lev, I realized that I can use a similar reasoning for the |
ghost
mentioned this pull request
this fixes a bug in calculating the size in the steal_half method in chase-lev deque.
obsletes #72
@fitzgen I couldn't add you as a reviewer, but please review it :)