Skip to content
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

Can we find a better name for select? #760

Closed
KasMA1990 opened this issue Feb 13, 2018 · 36 comments
Closed

Can we find a better name for select? #760

KasMA1990 opened this issue Feb 13, 2018 · 36 comments

Comments

@KasMA1990
Copy link

When the crossbeam-channel RFC was introduced a few months ago, I found it very hard to understand what select meant. I have the same issue in future-rs, where I find the name equally as opaque as in crossbeam. Luckily I have since been pointed in the direction of some Unix history which explains the design and naming behind select, but I feel like we can still do better than requiring a history lesson for a function name to make sense.

Therefore I would like to propose that select is slightly renamed to make it more clear what it is actually doing, to e.g. select_any or select_any_ready maybe (or any other name that is deemed better). Although the presence of select_ok already makes the naming tough, and not having a pure select breaks somewhat with the precedent for this functionality (since it is named as such in Unix and Go for example), but I think there are many Rust users who are not familiar with these precedents and will hence be confused by the name (myself included). At the same time, by keeping the word "select" in the name, it is still easy to search for, for those that do know the precedent (and if "select" is kept as the first part of the name, anyone just looking to type select in their IDE will also be helped by autocomplete).

I feel this would be a very rustic improvement to make; it should still be easy for veterans to use, but welcomes newcomers at the same time, by optimizing for readability over familiarity.

@alexcrichton
Copy link
Member

I think one suggested by @aturon historically as well has been Future::race akin to Promise.race in JS, which may be more evocative as well for non-Unix programmers perhaps?

@KasMA1990
Copy link
Author

race is much better to my mind, though I'm still in doubt about the implied semantics. select (in Unix terms) can have multiple "futures" complete at once (and hence not have a "winner" of the race), and select will choose a random one to execute. But what happens if two Rust futures complete at the same time? Is that even possible? If it is, then race still seems insufficient as a name, but if, on the other hand, there is always a definite winner in the race, then I'm all for it :)

@alexcrichton
Copy link
Member

It's true yeah it may not still be perfect, but I personally sort of like what race makes you think of in terms of prior art as it's closer to what's happening rather than select (which can indeed be alien)

@KasMA1990
Copy link
Author

I think I would be quite satisfied with race as well, even if it's not a perfect description. I can't really think of any other words that fit so well (something with tiebreaker?) but also keeps the name short and sweet (select_any_ready is at least accurate, but it's a bit verbose, and it doesn't evoke a nice analogy like race to make it easy to understand).

@alexcrichton
Copy link
Member

Ok! @KasMA1990 want to send a PR for this?

@KasMA1990
Copy link
Author

Sure! Against the 0.2 branch?

@alexcrichton
Copy link
Member

Indeed!

@KasMA1990
Copy link
Author

KasMA1990 commented Feb 14, 2018

What about select_ok and select_all? The easy thing would be to go with race_ok and race_all, but it seems like there are better names waiting to be discovered out there.

Just to make sure I understand them correctly:
select_all will return with the value from the first terminating future (plus the index of said future, and the "tail" futures), regardless if that future terminates with an Ok() or Err().
select_ok, on the other hand, returns only when one of its constituent futures return Ok() or it runs out of futures, in which case the Err() from the final future is returned instead--in both cases alongside the tail of futures (but not any index).

I have a few questions:

  1. Why does SelectAll::poll return the index of the terminated future? Is it used for an optimization somewhere? I'm curious because I'd like to try and name select_all in a way that describes its functionality, but I'm puzzled why it would actually return that index.
  2. SelectOk::poll calls drop explicitly, but SelectAll::poll does not; does this make a difference, or is it just style?

Based on this understanding, what about, instead of select, select_ok, and select_all for names, we use race, race_ok, and race_termination? I'm open to other naming, but I like that race_ok (racing to first Ok()) and race_termination (racing to first termination) are consistent about describing their end states, as opposed to select_ok (describes when it returns) and select_all (describes... I'm not sure actually).

What do you think?

Bonus question: Both SelectAll::poll and SelectOk::poll use mem::replace to grab their inner list of futures to return it as the tail list. mem::replace is called with a Vec::new, but wouldn't it be more efficient to call it with a Vec::with_capacity(0) instead, since it seems like the new inner value is never used?

@KasMA1990
Copy link
Author

KasMA1990 commented Feb 15, 2018

Another question: I think race makes excellent sense on futures instead of select, but on a stream, race seems misleading. But isn't merge a good name for the stream version? I mean, select on streams has the description: "An adapter for merging the output of two streams."

Thoughts?

EDIT: For precedent, Rx uses merge as the name for their equivalent method.

@KasMA1990
Copy link
Author

I just realized that 0.1 already had a merge and this was deprecated in #449
Since it's deprecated, is it okay to reuse the name in 0.2? I would also still like input if this name change is something you guys want. I'm for it, since I both find merge much more intuitive than select, but also because I think using select is actually a bit misleading in this case; the Unix semantics say that only one action will be taken, but I don't think that can be applied to streams in a reasonable form.

I'd also suggest renaming select_all on streams to merge_all for the same reasons as select and merge :)

@cramertj
Copy link
Member

merge sounds too similar to join, IMO.

@KasMA1990
Copy link
Author

KasMA1990 commented Feb 16, 2018

I agree, though I'm not sure what the best fix is. If we want to limit how many renames we do, we need a replacement for merge only, but I have a hard time thinking of a better name. Or maybe, since join is not defined on streams, it's okay anyway, since future combinators are separate from stream combinators anyway.

On the other hand, if we're not shy about getting better names for combinators while we're at it, I'd suggest renaming join to something which better reflects its semantics; like synchronize maybe. I mean, the word "join" only implies that a coupling takes place, not what that coupling actually is. But the function actually makes the futures sync up, so I think synchronize better reflects what is actually going on. This would then free up "merge" for use instead of select on stream.

I'd go for the latter personally. And sorry for making such a fuss about a few function names, but I really think we can make a useful difference here :)

EDIT: Or maybe just sync or wait instead of synchronize.

EDIT2: Since join is the semantic opposite of select, I actually think renaming join to wait makes really good sense; race and wait are basically using the same analogy, while highlighting the fact that they have opposite behavior.

@cramertj
Copy link
Member

It's worth noting that crossbeam-channel also offers this functionality under the name select: https://docs.rs/crossbeam-channel/0.1.2/crossbeam_channel/macro.select_loop.html

@KasMA1990
Copy link
Author

Yeah, that was actually what started this whole ordeal; I proposed a name change in the RFC, because I had trouble figuring out what select was supposed to mean. Nobody seemed interested in making a change in that RFC though.

My issue with select is this: as someone who doesn't know it by convention (from e.g. Unix or Go), it is very hard to figure out what it is supposed to do just going by the name. The name indicates that I am making some kind of selection, but what criteria am I selecting based on? And how big is my selection (is it any subset of the input that is a valid selection?)? What does it even mean to make a selection over some actions that may have different temporal state?
By contrast, race immediately puts my mind on the right track; I want the "winner" of the race, which makes sense both from a temporal and selection criteria perspective, with no need for further explanation.

I realize changing names of functions introduces churn, and in this case also brings some inconsistency with crossbeam, but I can't help but feel that select is a terrible name, that only works because some people have seen it elsewhere; all the people who don't know it will be left to visit the documentation until the name begins to stick. And going by names like select_all, it also seems to me that select has become kind of a catch all name for much async code too; e.g. with select on streams, you don't actually make a single selection, but but you merge all the content from all the streams, which seems like it has no relation to the original semantics of select. Again it just makes it all very confusing to navigate.

@alexcrichton I would like more input before I start actually making any changes :)

@KasMA1990
Copy link
Author

Or maybe you've got some input @aturon? :)

@carllerche
Copy link
Member

Personally, I don't think race is a good name (due to behavior). I think this blog post sums it up pretty well: https://www.jcore.com/2016/12/18/promise-me-you-wont-use-promise-race/.

Java futures use: "all_of" for join and "any_of" for select.

@KasMA1990
Copy link
Author

I like all_of and any_of actually. But what about select_ok and select_all? Maybe any_success (any_ok seems too close to any_of for my liking) and any_of_iter?

@KasMA1990
Copy link
Author

Sorry, I have to back off from this task; I hope we can still get some good names for these methods though :)

@aturon aturon added this to the 0.2 release milestone Mar 19, 2018
@aturon
Copy link
Member

aturon commented Mar 19, 2018

I was recently reading C# async/await docs and saw WhenAny and WhenAll for select and join.

I wonder whether these would be better as just free functions, rather than methods? Then we could call them when_any and when_all, with variants like when_any_ok and when_all_ok.

@cramertj
Copy link
Member

@aturon yes, i think they should actually be macros (so that they can be variadic).

@aturon aturon removed this from the 0.2 release milestone Mar 19, 2018
@KasMA1990
Copy link
Author

Doing them as macros with varargs would make it all much more sensible to me as well, since we wouldn't need the select_all variation or all the join3, etc. variants. But they won't work as combinators then, will they?

@cramertj
Copy link
Member

@KasMA1990

But they won't work as combinators then, will they?

I'm not quite sure what you mean by this-- if you mean they won't be usable in method position, then yes-- that's correct.

@Nemo157
Copy link
Member

Nemo157 commented Mar 20, 2018

As prior precedent, JavaScript's Promise.all, Promise.race and C#'s Task.WhenAny, Task.WhenAll are all static methods (a.k.a. free functions).

I do wonder if the 2-ary Future::select(self, other) would be useful to keep (potentially with a different name) for timeouts, or whether there's a more specific alternative that would be useful for timeouts.

@cramertj
Copy link
Member

@Nemo157 I handle timeouts in fuchsia w/ a TimeoutExt trait that adds an on_timeout(time, || result) method to futures.

@aturon
Copy link
Member

aturon commented Mar 20, 2018

Punting from the initial 0.2 release; let's figure out a macro and then we can deprecate/remove these.

@Pzixel
Copy link

Pzixel commented Jul 27, 2018

Hey guys.

I wonder if you still plan rename these methods. I agree that they are quite not perfect now.

WhenAny/WhenAll sounds more logical to me, than race. join or other mentioned alternatives.

@cramertj
Copy link
Member

cramertj commented Jul 27, 2018

@Pzixel The current versions of these combinators in 0.3 are still called select and join, and are used like this:

async {
    select! {
        thing1 => ...
        thing2 => ...
    }
    ...
    let (a, b, c) = join!(a, b, c);
}

IMO none of when_any/when_all/race/any_of sound as good here. I could maybe get behind all_of for join!, but I'm not sure about the extended versions (e.g. all_of_many, etc.).

@Pzixel
Copy link

Pzixel commented Jul 27, 2018

@cramertj afaik select/join was picked because of C inheritage. But for people that don't know C (and there is plenty of them, including myself) it's confusing. join is probably comprehensible if you compare it to join method on thread (sleep until forked thread is running), but select is just nonsense. It doesn't select anything, it just a combinator that returns first computed result. It doesn't select among anything, it just consumes whatever executor want. On the other hand when_any/when_all clearly say that "this is future resolved when any of features is resolved"/"this is future resolved when all features are resolved". Beside that, it's very convinient that you have to coherent methods wait_xxx with possible third or more alternatives.

@archseer
Copy link
Contributor

I personally prefer select! over race!, because I believe Promise.race also behaves a bit differently. select will wait until one of the futures is ready, then run the specific case block for that future. Whereas Promise.race simply waits for the first future that's ready, then runs one general block of code after. (select also maps well to golang's select)

@cramertj
Copy link
Member

I'm gonna go ahead and close this-- people know this utility as select!, it matches the names of the similar tools in go and posix, and there isn't a clearly superior alternative.

@ghost
Copy link

ghost commented Aug 19, 2019

I think select is potentially a dangerous name because it's subtly different from POSIX or Go select:

  • In POSIX, select waits until any of the file descriptors become ready, but it doesn't change their state. No operations on them will be performed.

  • In Go, select waits until any of the channel operations become ready and then executes a single operation among them. No other channel operation will be started.

  • In the futures crate, select starts all operations by polling futures in order and then waits until any one of them completes.

In summary:

  • POSIX doesn't fire any operations.
  • Go fires exactly one operation.
  • futures fires all operations.

The danger lies in people believing that our select has similar behavior to Go. For example:

select! {
   _ = rx1.recv() => {}
   _ = rx2.recv() => {}
}

A Go programmer might believe only one of these operations will be executed, which would be a wrong assumption, potentially leading to difficult-to-find bugs.

The name "race" might be a better choice because it sort of implies that all operations start and then we wait until one of them "wins the race".

To me, "select" sounds like "pick one of these operations", which is not at all what our select does.

@Pzixel
Copy link

Pzixel commented Aug 20, 2019

@stjepang you are right, but it's unlikely anobody will change this.

@Matthias247
Copy link
Contributor

Matthias247 commented Sep 16, 2019

@stjepang

In the futures crate, select starts all operations by polling futures in order and then waits until any one of them completes.

futures fires all operations.

This seems wrong. select! should also only execute one branch and stop polling as soon as the first future has returned Ready. If it doesn't I would assume it is a bug.

@ghost
Copy link

ghost commented Sep 16, 2019

@Matthias247

Right, but just because a future is not polled anymore does not mean the operation it is driving has not started or that it will not complete.

The point is that all futures may get polled, and therefore all operations might complete (e.g. imagine we're selecting over a bunch of filesystem operations implemented using a blocking thread pool).

There's not much we can assume about the difference between polling a future once and polling a future to completion.

@Matthias247
Copy link
Contributor

@stjepang You are certainly right on that one. But I think the point that Futures might continue to run in the background is more a general Rust Future thingy, and not specific to select!

All Futures which are not purely readiness based will show that symptom, independent whether they are used in select or any other mechanism which might not drive the Future to completion.

@Nemo157
Copy link
Member

Nemo157 commented Sep 17, 2019

If you're not using select! over borrowed futures then all other futures will be dropped after the select! finishes (or before it enters the chosen branch after #1874), that should cancel the outstanding operations (obviously some operations like ones running on a blocking thread pool cannot cancel once started, but that's only relevant to a small subset of futures).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants