Skip to content
This repository has been archived by the owner on Nov 5, 2018. It is now read-only.

Implement Guard::safepoint() #43

Merged
5 commits merged into from
Nov 26, 2017
Merged

Implement Guard::safepoint() #43

5 commits merged into from
Nov 26, 2017

Conversation

jeehoonkang
Copy link
Contributor

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.

Copy link

@ghost ghost left a 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.
///
Copy link

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.

@ghost
Copy link

ghost commented Nov 24, 2017

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?

@jeehoonkang
Copy link
Contributor Author

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/

@Amanieu
Copy link
Contributor

Amanieu commented Nov 25, 2017

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);
Copy link
Contributor

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.

Copy link
Contributor Author

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) 😭

@jeehoonkang
Copy link
Contributor Author

I rebased and addressed @Amanieu's comment. Thanks!

@Amanieu
Copy link
Contributor

Amanieu commented Nov 26, 2017

I think the consensus from #45 is that this function should be renamed to repin.

@jeehoonkang
Copy link
Contributor Author

I rebased and changed the comment for repin().

src/guard.rs Outdated
///
/// # Examples
///
/// ```
Copy link
Contributor

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.

@ghost ghost merged commit 983b33c into crossbeam-rs:master Nov 26, 2017
@ghost ghost mentioned this pull request Dec 12, 2017
@jeehoonkang jeehoonkang deleted the safepoint branch October 31, 2018 15:25
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants