Crate evaluation for 2017-08-29: rayon

Crate evaluation for 2017-08-29: rayon

For additional contribution opportunities, see the main libz blitz thread.

This post is a wiki. Feel free to edit it.

Links

Needs your help!

Anything that is not finished still needs your help! There is no need to sign up or ask for permission.

Guidelines checklist

Legend

  • [y] = guideline is adhered to, no work needed.
  • [n] = guideline may need work, see comments nearby
  • [/] = guideline not applicable to this crate

Checklist

Guidelines checklist

This document is collaboratively editable. Pick a few of the guidelines, compare the rayon crate against them, and fill in the checklist with [y] if the crate conforms to the guideline, [n] if the crate does not conform, and [/] if the guideline does not apply to this crate. Each guideline is explained in more detail here. If [n], please add a brief note on the following line with an explanation. For example:

  - [n] Crate name is a palindrome (C-PALINDROME)
        - rayon backwards is noyar which is not the same as rayon

Cookbook recipes

Cookbook example ideas

Come up with ideas for nice introductory examples of using rayon, possibly in combination with other crates, that would be good to show in the Rust Cookbook. Please leave a comment in that issue with your ideas! You don’t necessarily have to write the example code yourself but PRs are always welcome.

API guideline updates

What lessons can we learn from rayon that will be broadly applicable to other crates? Please leave a comment here with your ideas, or file an issue in the api-guidelines repo.

Discussion topics

4 Likes

Apologies for getting this started a little late! We may end up postponing the evaluation meeting from 08-29 a week or so due to the lateness here, but I figured we can see where we are in a week!

@cuviper and @nikomatsakis if y’all (or others!) have known discussion topics for the rayon it’d be great if you could post those questions here!

1 Like

Right now, rayon is re-exporting the threadpool API provided by rayon_core. I don’t know much about how the rayon stack is designed, but I think one of the motivations for rayon_core is to have a stable base that everyone must agree on so version mismatches upstream will simply fail to build. Which is actually a really interesting idea. I guess it’s so there’s only one global threadpool or something.

Will stable rayon continue to re-export the rayon_core API? And if it does, should it be discouraged to depend on rayon_core instead of rayon?

1 Like

I expect yes and yes. The hope is that even if we eventually reach for rayon-2.0, the core can remain the same. But this may be a good discussion topic for the meeting!

1 Like

Ok I’ve done a more thorough review of the current rayon API to familiarize myself with it. Nice work @nikomatsakis and @cuviper, the crate looks pretty sweet to use! Who doesn’t want a one-liner for parallel sorting an array?

I filled out a number of discussion topics on the bottom of the etherpad, some larger than others. The main ones I think we’ll have a good discussion about are:

  • Conventions for management of global resources. This is something that we haven’t come across before in the crates we’ve used and it’s also something that we in Tokio have been thinking about a lot recently. I’m sure we’ll love to hear rayon’s experiences here and see what others think!
  • Conventions around per-crate preludes. I don’t think there’s much to debate here, but we haven’t actually encountered this yet!
  • Unstable APIs of crates. This was an interesting feature of the rayon crate that it has unstable features (like rustc sorta). Seemed like something neat we could draw some conventions from!

Yes, let me elaborate on this point. Our goal with the current design is that there should be exactly one global thread-pool for all users of Rayon, regardless of how many different versions they use simultaneously. This is for two reasons:

  • Throughput: a single thread-pool is better, all things being equal, since you don't oversubscribe the CPU. (It can, however, lead to observed latency, since you might wind up waiting for work that some other library is doing.)
  • Deadlock potential: if you combine multiple thread-pools, there is some potential for dead-lock, since you can wind up with one thread-pool blocked on another
    • This is similar to doing blocking I/O, effectively.
    • It's a bit tricky to do, you have to effectively have thread-pool A spawn some work into thread-pool B and block on the result (e.g., using join()), and then have thread-pool B call back into thread-pool A and do the same. But it could arise if you have library A and B, both using rayon, but with different global threadpools, and they have callbacks that call into one another.

Now, even with the current design, there are no strict guarantees. In fact, I think it's safe to say that having a library use Rayon and invoke some callback from the Rayon thread is slightly risky, at least if you haven't informed users about your intentions (e.g., that callback could do blocking I/O, you don't know). Since one can create multiple thread-pools explicitly, you could induce the same sort of deadlocks that way. But overall it was deemed better to push that concern onto those few users that create their own global thread-pools. Ergonomically, Rayon definitely tries to steer you to the global thread-pool, which means you get both improved throughput and a reduced risk of deadlock.

Yes, it will continue to do so, and no, there is no particular reason not to depend on rayon_core directly. The idea is that there is only one major version of rayon_core for all time: i.e., we stay in the 1.x series indefinitely. therefore, cargo's resolution algorithms will always give you one version of rayon-core across your project, and hence one global pool.

1 Like

I added this discussion topic:

  • before we declare 1.0, any language features that would change our public API?
    • e.g. ProducerCallback is kind of messy, and might be avoidable with generic associated types

Heh, it seems like @cuviper and I disagree here (notably on the wisdom of using rayon_core directly). I guess that I would revisit my answer in two ways:

  • I would discourage people from doing it because there is really no reason to do so. I'd prefer that rayon_core is seem as an "implementation detail" of rayon.
  • However, I don't see that doing so causes any particular problems that we wouldn't have otherwise. That is, if you have a bunch of crates using rayon 1.x (and hence rayon_core 1.x), and then we release Rayon 2.0, and that requires rayon_core 2.0, then you will have two global thread-pools, the same as if people were using rayon_core 1.x directly.

So we came to this system in a somewhat "traumatic" way -- in particular, we had an older system where we used a cargo feature, but naturally people enabled it and wound up depending on internal details of those APIs. Using cfg does at least require every user, all the way up the chain, to acknowledge the unsafety.

I added a few discussion points myself, mostly about how to factor rayon-core itself:

  • How to factor out futures; unsafe APIs in rayon_core
  • rayon_core could be more minimal

cc @Amanieu, with whom I discussed the latter topic at ECOOP.

By the way, I left some minor responses scattered in amongst the nits, and added a few more.

Could there be a concrete error type (either using error-chain or otherwise) rather than returning a trait object, or change the bound on the trait object to std::error::Error + Send + 'static?

Alright we’ve had a great discussion in person now and there’s a tracking issue for all the work items for Rayon now!

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.