Proposal: New channels for Rust's standard library


#1

This is actually a pre-RFC written in the form of a blog post:

I’m eager to hear what everyone thinks and would be happy to discuss!


#2

A well written proposal and I fully agree with deprecating the mpsc module for the channel module. The transition would be pretty easy. Simply changing mpsc::channel to channel::unbounded and mpsc::sync_channel to channel::bounded for most users with the additional benefit of making your code more readable. It feels like we can have our cake and eat it by having a multiple producer multiple consumer channel with a better performance than the current multiple producer single consumer channel while being able to leave the confusing terminology behind.


#3

Is omitting any form of selection clearly the right thing?

I remember first looking at mpsc after 1.0 and being very surprised that selection was missing (it feels like one of the basic primitives: Occam’s ALT).

From the history it looks more like select! was omitted from 1.0 because it was unfit for stabilisation, rather than because anyone positively thought that channels without selection make a coherent feature which is worth including in a standard library.

Maybe it’s worth looking to see what proportion of crossbeam-channel's users use selection.


#4

I think we should deprecate mpsc long-term. Channels without select are a bad interface, because they are not composable.

I also think that, long-term, channels should be in std, because they are such a core primitive. However, I am not sure that we are ready for std::sync::channel right now. I think it perhaps makes sense to wait for async/await to stabilize, to be confident that sync::channel works both in blocking and in non-blocking contexts.

Is improving mpsc performance really beneficial? Perhaps adding a note to the docs “use crossbeam-channel” would be more helpful for end users?

OTOH, if we can decrease the maintainence burden of mpsc by simplifying code, that seems like a good idea. So perhaps we can introduce nightly only sync::channel with the right interface, and make mpsc a shim on top of that? We can than move sync::channel to stabilization in the future, together with the Stream abstraction.


#5

Replacing of the existing implementation with a slightly smaller, better version seems like a definite improvement to me. If the missing Sync could be added without breaking things, that’d be even better.

I’m not sure about deprecation and replacement.

mpsc works OK for basic cases. I’ve used it in a few places and never actually needed select, so to me mpsc isn’t broken.

If we agree that mpsc shouldn’t have been in the stdlib in the first place, then adding another channel module just repeats the mistake. We’re already sending people to use 3rd party crates for relatively basic things, so I don’t think proper channels have to be in the stdlib. The stdlib doesn’t have queues (rayon), which I consider a more common and more basic primitive.


#6

And excellent points about inconsistent naming. That’s something we can mostly fix right away by updating the docs and examples.


#7

It’s the right thing at the moment. We could add selection later on in the future, but I think we should focus on replacing std::sync::mpsc with a minimal std::sync::channel first.

Well, I get the sentiment, and perhaps bad is a too strong word, but plenty of languages have some form of channels without selection and are doing just fine - see D, Nim, Crystal, Java. Even very basic channels go a long way towards building an essential toolbox for concurrency.

For better or worse, I don’t expect anything in std::sync to ever work with async/await. Instead, I believe we’ll have two distinct sets of synchronization primitives - the std::sync module will focus on thread-based primitives and another module will focus on futures-based primitives.

Yes, that’d be a great first step and would (I hope) make everyone happy!

I feel channels are a bit too basic to be left out, however. With channels, you can send messages between threads or simulate mutexes, conditional variables, wait groups, actors… Even without select, it’s an incredibly versatile primitive. Channels rule!

We can fix the documentation only, unfortunately. :frowning: We cannot unify Sender and SyncSender into a single type. The Sync prefix will stay, as well as sync_channel. Also, the word disconnected is set in stone because the error types are using it.


#8

I agree with the deprecations, but once we put something in standard, we can never remove it.

If we add a different channel API and implementation to the standard library, and in 5-10 years time it turns out that it’s also “bad”, then that would be two deprecated channel implementations that we would have to maintain forever.

I agree that channels are a core concurrency primitive, but so are many things in many fields (like, rayon is a core parallelism primitive, but that’s not enough of a reason to move rayon into the standard library). The question is whether channels are a core standard library primitive. That is, whether they are a vocabulary type / trait, or whether it is impossible to implement them in stable Rust efficiently because some compiler magic is required (think: Atomic types).

AFAIK these APIs can be implemented without issues outside the standard library. We already made this mistake once, let’s learn from it and not make it again. So my vote here is for no, let’s not move crossbeam channels into the standard library.

I wouldn’t be opposed for the deprecation message to recommend users to use crossbeam channels directly (as opposed to some more generic deprecation message that does not recommend any particular library).

EDIT: also another downside could be having to maintain two different channel implementations if crossbeam and the standard library diverge. I don’t know what’s our manpower for this, but if the issue is one of discoverability, or that the Rust book has a policy of only using libstd primitives, then we should improve those instead and try to keep libstd lean.


#9

At first after reading this blog post I was really excited about getting a new std::sync::channel library that contains the core of crossbeam-channel, but in the end I think I agree with @gnzlbg that it might make more sense to deprecate std::sync::mpsc and point to the crossbeam-channel project directly. And if we do decide to deprecate it, it’s not even clear it makes sense to spend the effort to simpler channel implementation for the current std API.

Another argument that @gnzlbg didn’t mention is that it’s clear that a lot of evolution has happened already between the release of std::sync::mpsc and the current iteration of crossbeam-channel, and it’s not clear that crossbeam-channel has matured enough (in the sense of the pace of change slowing down – not in the sense of being mature) that it would be a good fit for the “forever frozen” promise we have for std.

I’m not sure if we already have clearly defined guidelines for what kinds of things should be allowed to go in std, but “core primitives”, “compiler magic is required” might be interesting first steps to define something like that.

If you want to argue that the crossbeam-channel library contains fundamentals that lots of projects need, I think we should also consider making chrono part of std (610k vs 740k recent downloads). By which I mean, that’s a slippery slope and not a very clear-cut argument IMO.


#10

Speaking as somebody relatively new, the common guidance in the community is “just use crossbeam”. I don’t think that’s a bad thing, since, as seen already, “never breaking” APIs are really really hard to get right. There’s a high bar to be included in stdlib, and frankly I think it should be higher than it is.

The current stdlib implementation does work for what it is, and those who care can find (and we can help them) better options.

Think about how long some boost libraries in C++ took to be included in their standard library.


#11

Thanks everyone for commenting! You gave good arguments for not having any channels in the standard library and, honestly, I’d be happy with that outcome.

I want to remind that the mpsc implementation has been broken for years and needs to be fixed one way or another as soon as possible. So we need to do something about it.

A serious bug that is trivial to trigger was reported two years ago but the complicated codebase makes it difficult to fix. There was a PR attempting to fix it, got blocked on removing mpsc_select (will be a big refactor), and was subsequently closed. The bug is now even documented as a known issue, which is a bad look for Rust and doesn’t inspire confidence in the quality of our code. :frowning:

And even if we fix the bug, I still think the API has so many flaws it would be better to not have it at all.


#12

From a surface level, it sounds like deprecation with a pointer to alternatives is better than the confusion of two implementations in stdlib (to me, that wouldn’t inspire much confidence - though it’s not unheard of in other languages, java’s vector/arraylist comes to mind). I’m all for fixing the current implementation in whatever way we can though.


#13

My humble opinion here is: replacing the guts of mpsc without changing the interface doesn’t prevent the deprecation or switchover later on. If it’s already (mostly) written, it might be the first step no matter if the deprecation happens later on or not. And much faster than agreeing on new naming, if it should be deprecated, etc.

I’m not sure how far we can get with fixing the current API with soft-renaming (keeping the old one deprecated and hidden from docs, calling the new one from within it) methods and such. It would be nice if we didn’t have two eg. receiver types that would be incompatible and if mpsc and channel were actually only aliases and the types were completely the same.


#14

Are you arguing that we should fix it even though we deprecate it?

I don’t think that is a good use of resources, but I’m not going to complain if someone decides to do that.


#15

Sorry, perhaps I could’ve been more clear. My point was that we should either fix the bug, replace the implementation, or deprecate it.


#16

Besides batteries-included availability, are there any special advantages for including channels in std? For example, are there improvements that could be made with unstable features?


#17

If a new API is provided, it would probably make sense to consider trying to align it with the futures channel API: https://rust-lang-nursery.github.io/futures-api-docs/0.3.0-alpha.13/futures/channel/mpsc/index.html


#18

I would say one advantage to including them in std is that we can trivially use them for tests, examples, and implementation of internals:

  • The docs for Mutex, thread::spawn, and Iterator::for_each use channels.
  • There’s a bunch of tests for threads, networking, allocators, and others that use channels. See this list.
  • The testing framework libtest (that’s the #[test] thing) itself uses channels internally.

Now, this doesn’t mean we need channels just because of these uses, but I think these uses mean channels are really common: pretty much as soon as one calls thread::spawn(), chances are they’ll also want channels, and channels are typically more handy than Arc<Mutex<_>>.

Other than that there are no advantages. So the question is: are channels essential enough to have in the standard library, just like we have basic collections? My feeling is people will start missing mpsc if/when we deprecate it. But I could be wrong and appreciate that some disagree. It’s a tough call. :slight_smile:


#19

futures-channel::Sender will still require a function that does the current operation of disconnect to satisfy other traits it implements, specifically Sink requires a way to “close” the sink from a mutable reference.

Other than that, I think simplifying the API as much as possible and standardising it across implementations would be a great idea.


#20

This is actually really interesting to me. Channels being a basic primitive are one thing, but there’s no reason not to leave them in a crate. But other parts of std benefitting from channels, or a desire to standardize channels across crates (so they can talk to each other with channels) are two good reasons to include it. We should be absolutely sure we can provide a good, general, performant, and stable interface and implementation though.

It sounds like replacing the guts of mpsc and deprecating it both have 0 downsides. I’d like to know if we can use channels more thoroughly in std (just for tests seems kind of meh).