Add Parker for thread parking

[ghost]

This is just an extracted copy of the current implementation of thread::park() and thread::unpark().

Parker is a low-level thread synchronization primitive useful for building others (like locks etc.). It would be useful in tokio-rs/tokio#528 and might remove some cruft and unnecessary TLS access from context.rs in crossbeam-channel.

I've also added a fast-path check for timeout == Duration::from_secs(0), which Tokio relies on.

An interesting peculiarity of Parker is that it's not split into Parker and Unparker, which means any thread can call park() at any time. However, if multiple threads call park() at the same time, expect deadlocks or panics - that is the user's problem. Splitting the primitive into an owned Parker and shared Unparker would require us to wrap the inner structure into an Arc, which comes at a cost of allocation and indirection. Since this is a very low-level primitive, I chose not to do that.

The reason why this is in crossbeam-utils is because it's a really simple primitive with no dependencies. Also, tokio-executor and tokio-threadpool really don't want to pull in the whole crossbeam in order to use this.

cc @carllerche

[carllerche]

I wonder if it would be worth to enforce the single caller of park in the API. You could structure it as:

struct Parker(Unparker(Arc<Inner>));

This would require an arc allocation, but compared to the mutex / condvar, perhaps that isn't too bad?

disclaimer: I don't know if this is a good idea.

I'm also working on trying to validate this w/ syncbox.

[ghost]

@carllerche My thinking is that if someone needs this synchronization primitive, they will usually care more about optimization opportunities than how bulletproof the interface is.

As such, it might be better to leave the choice of allocation vs no allocation to them. For example, in Tokio, we'll have Arc<Parker> and wrap it inside DefaultPark and DefaultUnpark. In crossbeam-channel, we wouldn't put the Parker inside an Arc.

Having just a single Parker struct doesn't affect memory safety (only the potential for deadlocks and panics) so it's not that much of an issue.

[carllerche]

@stjepang seems good to me. We can try to remove SeqCst later.

[jeehoonkang]

I'll have a closer look today night. Thanks!

[ghost]

@jeehoonkang Note: there's no need to review the actual implementation since it's just a copy-paste from libstd. The only thing to review is the interface.

[carllerche]

I would like to use this :)

[carllerche]

One question is: should this use parking_lot.

In general, it would be nice for parking_lot to be opt-in as a feature flag.

[jeehoonkang]

I think it's in good shape, but I think the Parker/Unparker design also looks fine. The use of Arc will not harm the overall performance significantly because we're waking up a thread, which is much more costly.

[ghost]

@carllerche

One question is: should this use parking_lot.

In general, it would be nice for parking_lot to be opt-in as a feature flag.

I doubt parking_lot would bring us any tangible benefits because locking only happens in the slow path.

By the way, folks over here at RustFest are preparing a PR that moves parking_lot into libstd. It's almost done. Hopefully soon we will never have to add it as a dependency anymore.

@jeehoonkang

The use of Arc will not harm the overall performance significantly because we're waking up a thread, which is much more costly.

Ah, I guess you're right. There's probably no point in overoptimizing this if there won't be any measurable difference. I've split the interface into Parker/Unparker. There's now method Parker::unparker() that returns a reference to Unparker (which can also be cloned).

[ghost]

Ok, let's merge this so that @carllerche can start using it.

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push