-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after adding a comment.
src/guard.rs
Outdated
/// Since `&mut self` is given, we can statically deduce that there is no [`Shared`] pointer | ||
/// that is created before an invocation of `safepoint()` and then survives after the | ||
/// invocation. | ||
/// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably also mention that the current thread will be repinned only if this is the only active guard.
By the way, do you think your queue would be better served with HP instead of EBR for memory reclamation? The published paper says they used HP - probably for a good reason? |
I just rebased and added a comment. I think HP is more friendly for wait-free constructions. In almost every wait-free construction, a data structure (usually linked list) contains all the threads, and a thread has a pointer to another thread in the data structure. "A points to B" means that A will help B in the next turn. That help pointer is usually long-living: it is updated only if A succeeds an operation, and the pointer points to a valid object until an iteration of the data structure is over. So If this pointer is managed by EBR, the global epoch will be very slowly advanced. I'm trying to deal with long-living pointers with crossbeam-epoch by designating an independent collector that only deals with the help pointers. It's fine to slowly advance its epoch, because there will not be so many help-pointers. (WARNING: it's not finished, so I still don't know whether it works.) I'm also interested in mixing HP and EBR as discussed in crossbeam-rs/rfcs#8. Maybe MSR's Snowflake is an interesting example of that direction: https://www.microsoft.com/en-us/research/publication/project-snowflake-non-blocking-safe-manual-memory-management-net/ |
I actually implemented something similar: Amanieu@7f535bd |
src/internal.rs
Outdated
let global_epoch = self.global().epoch.load(Ordering::Relaxed); | ||
|
||
if epoch != global_epoch { | ||
self.update_epoch(epoch, global_epoch, Ordering::Release); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A SeqCst
fence isn't required here, a Release
store should be enough to ensure correct ordering.
- We need to ensure any loads/stores from the previous epoch do not leak into the new one.
- However we don't care if any loads/store from the new epoch happen before the epoch counter is updated. At worse , other threads will see us in an older epoch and will delay GC slightly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Amanieu you're right. Thanks for pointing out that! Also it seems I forgot what I said a month ago: crossbeam-rs/rfcs#18 (comment) 😭
I rebased and addressed @Amanieu's comment. Thanks! |
I think the consensus from #45 is that this function should be renamed to |
I rebased and changed the comment for |
src/guard.rs
Outdated
/// | ||
/// # Examples | ||
/// | ||
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You accidentally deleted part of a comment here.
This is the implementation of
Guard::safepoint()
we discussed in crossbeam-rs/rfcs#18.I don't expect it to be merged before 0.1.0, but I'm personally in favor of doing so; I need it in implementing a wait-free queue.