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

Provide functions to get the global Collector #25

Closed
joshlf opened this issue Oct 14, 2017 · 9 comments
Closed

Provide functions to get the global Collector #25

joshlf opened this issue Oct 14, 2017 · 9 comments

Comments

@joshlf
Copy link

joshlf commented Oct 14, 2017

If code accepts a Handle or a Collector that the user is expected to pass, it'd be nice for them to be able to opt out of having to manage their own GC by simply doing crossbeam::global_collector() or crossbeam::global_collector().handle() or something similar.

@jeehoonkang
Copy link
Contributor

jeehoonkang commented Oct 15, 2017

exposing the default Handle

It would be good to expose fn epoch::default_handle() -> &Handle that accesses the default global handle, because data structures will speak to Crossbeam with Collector and Handle API. For example, an operation to a shared object may accept &Handle, e.g.:

fn Stack<T>::push(val: T, handle: &Handle) {
    handle.pin(|scope| { ... })
}

The stack that uses the default collector will be implemented as below:

struct DefaultStack<T> {
    inner: Stack<T>,
}

fn DefaultStack<T>::push(val: T) {
    self.inner.push(val, epoch::default_handle())
}

exposing the default Collector

However, I don't think it's necessary to expose the default collector. I don't see the use case of crossbeam::global_collector().handle(): what's the benefit of it over using the default handle?

@joshlf
Copy link
Author

joshlf commented Oct 15, 2017

It would be good to expose fn epoch::default_handle() -> &Handle that accesses the default global handle, because data structures will speak to Crossbeam with Collector and Handle API.

I like that, except it wouldn't work for a global handle because Handles aren't Sync. Instead, you could have epoch::default_handle() return a reference to the thread-local handle.

However, I don't think it's necessary to expose the default collector. I don't see the use case of crossbeam::global_collector().handle(): what's the benefit of it over using the default handle?

What if a library has a function like foo(gc: &Collector)? It'd be good to support that as well as foo(gc: &Handle).

Another way of doing it would be to add a fn collector(&self) -> Collector method to Handle so that default_handle() would allow you to just do default_handle().collector().

@ghost
Copy link

ghost commented Oct 16, 2017

This is similar to Rayon's ThreadPool::global():

pub fn global() -> &'static Arc<ThreadPool> {
    // ...
}

@Vtec234
Copy link
Member

Vtec234 commented Oct 16, 2017

However, I don't think it's necessary to expose the default collector. I don't see the use case of crossbeam::global_collector().handle(): what's the benefit of it over using the default handle?

I agree. Currently the Collector cannot do anything besides spawning Handles and doing that instead of using TLS also has the overhead of participant registration. Adding an epoch::default_handle() -> &Handle that returns the default TLS seems like a good idea.

What if a library has a function like foo(gc: &Collector)? It'd be good to support that as well as foo(gc: &Handle).

As above, there is probably no need for a library to ever do that.

@joshlf
Copy link
Author

joshlf commented Oct 16, 2017

OK, fair enough. I guess just a getter for the default TLS handle is enough.

@ghost
Copy link

ghost commented Oct 21, 2017

If we replace Scope with Guard (see here), Handle will have to be non-Send. While this is definitely a limitation, it actually brings some good news. :)

Non-Send handles would allow us to implement the following interface, which we cannot do now:

// Increments a reference count, but should be very cheap since this is a non-atomic operation.
fn default_handle() -> Handle {
    HANDLE.with(|h| h.clone())
}

// Cloning a handle returns just another reference to the same thing.
// This is the same idea as with `std::thread::Thread` -- cloning a `Thread` simply
// gives you another handle to the same thread.
pub struct Handle {
    node: *const Node,
}

impl Clone for Handle {
    fn clone(&self) -> Self {
        unsafe {
            *(*self.node).refcnt.get() += 1;
            Handle { node: self.node }
        }
    }
}

impl Drop for Handle {
    fn drop(&mut self) {
        unsafe {
            let cnt = &mut *(*self.node).refcnt.get();
            *cnt -= 1;

            if *cnt == 0 {
                let guard = &unprotected();
                (*self.node).next.fetch_or(1, SeqCst, guard);
            }
        }
    }
}

// A node in the linked list of registered participants.
struct Node {
    local: Local,
    global: Arc<Global>,
    refcnt: UnsafeCell<usize>,
    next: Atomic<Node>,
}

@jeehoonkang
Copy link
Contributor

@stjepang What's the use of next: Atomic<Node>?

@ghost
Copy link

ghost commented Oct 22, 2017

It's just the next-pointer in the linked list. I didn't manage to reuse the List<T> interface in my branch that switches from scopes to guards.

@arthurprs
Copy link

If we replace Scope with Guard (see here), Handle will have to be non-Send. While this is definitely a limitation, it actually brings some good news. :)

Nice.

@ghost ghost mentioned this issue Nov 3, 2017
@ghost ghost closed this as completed in #31 Nov 20, 2017
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants