Replace Scope with Guard #31
Conversation
ghost
mentioned this pull request
Refactor intrusive linked list from src/internal.rs
|
@jeehoonkang Just a reminder that this is waiting for review. :) |
There was a problem hiding this comment.
Besides minor comments, I'm happy with all the changes. Thanks!
But I'm a little bit hesitant to merge this PR now, because the following issue has not been resolved yet: crossbeam-rs/rfcs#23 In order to address the issue, maybe we need to once more change this repo.
What do you think?
| /// Decomposes the internal data into the epoch and the pin state. | ||
| #[inline] | ||
| fn decompose(&self) -> (usize, bool) { | ||
| (self.data >> 1, (self.data & 1) == 1) |
There was a problem hiding this comment.
What about returning (self.data, (self.data & 1) == 1)? Maybe it's a little bit more difficult interface to use, but may we can save an instruction 😃
There was a problem hiding this comment.
Well, it's just a shift operation that gets executed on cold paths, so it doesn't really matter... :)
src/guard.rs
Outdated
| #[inline] | ||
| pub unsafe fn unprotected() -> &'static Guard { | ||
| static UNPROTECTED: usize = 0; | ||
| &*(&UNPROTECTED as *const _ as *const Guard) |
There was a problem hiding this comment.
I couldn't figure out why it is safe -- would you please explain a little bit more?
| pin_count: Cell<usize>, | ||
| /// | ||
| /// This is just an auxilliary counter that sometimes kicks off collection. | ||
| pin_count: Cell<Wrapping<usize>>, |
There was a problem hiding this comment.
wow, Wrapping is very convenient :)
|
@jeehoonkang Do you think we could merge this now and release version 0.1.0? Regarding crossbeam-rs/rfcs#23, I think we will have to make slight adjustments to |
|
Also, I forgot to ask: Do you have an opinion on whether |
|
@stjepang I also thought of releasing 0.1.0 after merging it, but was also concerned with backward compatibility. If we release it now, it's quite clear that 0.2.0 will break some API. That's legal in semantic versioning scheme, but still it's better to avoid breaking changes if we can.. On the other hand, crossbeam-epoch 0.1.0 is so waited in many parties, including crossbeam itself and rayon If we can provide a clear migration path from 0.1.0 to 0.2.0, breaking changes will not be fatally painful. I'm torn between the two options, but anyway I'm willing to follow your decision. Let's merge it! I'd like to say thank you for your great job! |
|
On |
ghost
merged commit 
Apologies for the huge PR. I'll try to summarize what's going on here:
atomic.rs- No big changes,&Scopewas just changed to&Guard.collector.rs-Handle::clonenow creates another reference instead of a new handle.epoch.rs- Almost everything is new. I've implemented typesEpochandAtomicEpoch, which are similar tousizeandAtomicUsize.guard.rs- No big deal, just a lot of documentation.internal.rs- The meatiest part of this PR.sync/list.rs- This file is gone. I've found it easier to work directly with the linked list ininternal.rsrather than through aList<T>abstraction. But we could bringList<T>back in a future PR, though.sync/queue.rs- Scopes replaced with guards, that's all.Closes #25.