-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom collector in Deque #23
Comments
Here's one idea off the top of my head. Custom collectors remind me very much of custom allocators ( We could introduce a trait GetHandle {
fn get_handle(&self) -> Handle;
}
struct DefaultGetHandle;
impl DefaultGetHandle {
fn get_handle(&self) -> Handle {
epoch::handle()
}
} Then we can implement struct Deque<T, GH: GetHandle = DefaultGetHandle> {
// ...
collector: GH,
}
impl<T, GH: GetHandle> Deque<T> {
fn with_collector(get_handle: GetHandle) -> Deque<T> {
Deque {
// ...
get_handle,
}
}
}
impl<T, GH: GetHandle> Stealer<T> {
fn steal(&self) -> Steal<T> {
let handle = self.deque.get_handle.get_handle();
let guard = handle.pin();
// ...
}
} Finally, let's create a custom collector and give to a newly constructed lazy_static! {
static ref MY_HANDLE: Collector = Collector::new();
}
thread_local! {
static MY_HANDLE: Handle = MY_HANDLE.handle();
}
struct MyGetHandle;
impl GetHandle for MyGetHandle {
#[inline]
fn get_handle(&self) -> Handle {
MY_HANDLE.with(|h| h.clone())
}
}
fn main() {
let d = Deque::with_collector(MyGetHandle);
} |
Such a generic interface is definitely possible and could be a future extension, but what I meant for now is simply allowing a One complication is that if we make |
Guards are currently preventing The unsafe contract of unsafe Note that the global function Another option would be to add a lifetime to guards: pub fn pin() -> Guard<'static> {
// ...
}
impl Handle {
pub fn pin<'a>(&'a self) -> Guard<'a> {
// ...
}
} This way the type system would prevent sending a In any case, if we make |
@stjepang: With such a structure present (one that allows effectively sending a |
Yes, but the
That's okay - I don't think it's a big deal having it within the One thing to keep in mind when we talk about concurrent data structures is that they're much more rarely used than single-threaded data structures like Another thing I'm worried about is that giving a single My feeling is still that we should think of custom collectors not as something that is casually set up and torn down like in our unit tests, but instead as something that is typically set up at process startup and torn down at process termination. In that sense, collectors are a lot like allocators - and perhaps we should strive to come up with a similar interface to the cc @jeehoonkang |
Here's yet another proposition: When pinning, let's access When you do When registering a new Now let's have a table inside struct Table {
table: Box<[AtomicPtr<Local>; 1000]>,
next: AtomicPtr<Table>,
} When registering a new Now, to pin the current thread, instead of accessing the thread-local This idea seems like it would be too slow, but it's not! I've actually managed to code an ugly prototype that matches the pinning performance of our current implementation (both are 12 ns). The main benefit of this approach is that we don't need struct Collector { ... }
impl Collector {
fn new() -> Self;
fn pin(&self) -> Guard;
fn is_pinned(&self) -> bool;
}
impl Clone for Collector { ... } To pin the current thread, you can just do |
I very much like @stjepang's handless proposal! But I'm concerned with portability. Can we call |
We don't have to call Sorry for the vague explanation. I haven't exactly solidified the whole idea, but would like to implement a prototype in several few weeks so that we can experiment with it. But first I'd like to push forward with |
In PR #21, @jeehoonkang and @Vtec234 pointed out that it should be possible to specify a custom
Collector
in the deque constructor (Deque::new
).This is not a high-priority issue at the moment, but let's kick off a discussion - how would we go about implementing such a feature?
The text was updated successfully, but these errors were encountered: