# Out of band crate evaluation for 2017-07-06: threadpool **URL:** \ **Category:** libs\ **Created:** [July 7, 2017, 3:43am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495 "2017-07-07T03:43:19Z")\ **Posts on this page:** 20\ **Page:** 1
**Author:** ![ericho](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ericho/32/2406_2.png) [@ericho](https://internals.rust-lang.org/u/ericho)\ **Post date:** [July 7, 2017, 3:43am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/1 "2017-07-07T03:43:19Z")
# Crate evaluation for 2017-07-06: threadpool For additional contribution opportunities, see the [main libz blitz thread](https://internals.rust-lang.org/t/rust-libs-blitz/5184). **This post is a wiki. Feel free to edit it.** ## Links - [crates.io: Rust Package Registry](https://crates.io/crates/threadpool) - [GitHub - rust-threadpool/rust-threadpool: A very simple thread pool for parallel task execution](https://github.com/frewsxcv/rust-threadpool) - [threadpool - Rust](https://docs.rs/threadpool) # Needs your help! Anything that is not checked off still needs your help! There is no need to sign up or ask for permission - follow any of these links and leave your thoughts: ## 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](https://public.etherpad-mozilla.org/p/rust-api-guidelines-threadpool) This document is collaboratively editable. Pick a few of the guidelines, compare the `threadpool` 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 $ ```plaintext - [n] Crate name is a palindrome (C-PALINDROME) - error-chain backwards is niahc-rorre which is not the same as error-chain ``` ## Cookbook recipes [Cookbook example ideas](https://github.com/brson/rust-cookbook/issues/225) Come up with ideas for nice introductory examples of using `threadpool`, 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 `threadpool` that will be broadly applicable to other crates? Please leave a comment in that issue with your ideas! ## Discussion topics > Anything that’s not a concrete crate issue yet. We want to eventually promote any topics here into actionable crate issues below. ## Crate issues - [Missing hyperlinks in documentation](https://github.com/rust-threadpool/rust-threadpool/issues/85) - [Missing categories and keywords in Cargo.toml](https://github.com/rust-threadpool/rust-threadpool/issues/84) # How are we tracking? > **** > > ## Pre-review checklist > > Do these before the libs team review. > > - Create evaluation thread based on [this template](https://public.etherpad-mozilla.org/p/rust-libz-blitz-eval-template) > - Work with author and issue tracker to identify known blockers > - Compare crate to guidelines by filling in checklist > - Record other questions and notes about crate > - Draft several use case statements to serve as cookbook examples > - Record recommendations for updated guidelines > > ## Post-review checklist > > Do these after the libs team review. > > - Publish libs team meeting video > - Create new issues and tracking issue on crate's issue tracker > - Solicit use cases for cookbook examples related to the crate > - File issues to implement cookbook examples > - File issues to update guidelines > - Post all approachable issues to TWiR call for participation thread > - Update links in coordination thread ---
**Author:** ![brson](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/brson/32/5197_2.png) [@brson](https://internals.rust-lang.org/u/brson)\ **Post date:** [July 11, 2017, 4:28am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/2 "2017-07-11T04:28:28Z")
Thanks @ericho. I’m just here to bump this thread for now, but I’ll be back… ---
**Author:** ![Phaiax](https://avatars.discourse-cdn.com/v4/letter/p/6bbea6/32.png) [@Phaiax](https://internals.rust-lang.org/u/Phaiax)\ **Post date:** [July 13, 2017, 3:13pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/3 "2017-07-13T15:13:31Z")
During reading of the documentation, I asked myself what the semantics of `ThreadPool::clone()` are. Does it spawns new threads or is it more like a reference to the existing ThreadPool? When does it spawn its threads? On creation or on demand? ---
**Author:** ![Phaiax](https://avatars.discourse-cdn.com/v4/letter/p/6bbea6/32.png) [@Phaiax](https://internals.rust-lang.org/u/Phaiax)\ **Post date:** [July 13, 2017, 4:49pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/4 "2017-07-13T16:49:22Z")
I’ve completed the missing checkpoints in the etherpad. But there are three things that i don’t know where to put: - In contrast to `crossbeam::scope`, this crate requires the worker closure to be static. I think there should be an example on how to deal with the borrow checker error if the user tries to use some stack variable. -\> Box - The Readme should explain the difference between the `Similar libraries` and itself, especially the scoping difference. It should also mention the pros of a non-scoped thread pool. (The word scoping is potentially unfamiliar to the user) - The usage section in the Readme does only show half the usage. Rename that section into `Setup` or give more details about usage in code? ---
**Author:** ![ericho](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ericho/32/2406_2.png) [@ericho](https://internals.rust-lang.org/u/ericho)\ **Post date:** [July 16, 2017, 6:47pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/5 "2017-07-16T18:47:22Z")
Thanks @Phaiax for the checklist update 🙂 I think the three items mentioned could be tracked with issues, the first will probably be into the cookbook issue and the other two in threadpool repo. However we’ll need more discussion before go to the next steps. ---
**Author:** ![KodrAus](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kodraus/32/2816_2.png) [@KodrAus](https://internals.rust-lang.org/u/KodrAus)\ **Post date:** [July 23, 2017, 11:07pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/6 "2017-07-23T23:07:56Z")
The crate layout itself is a bit non-standard, where `lib.rs` lives in the root instead of under `/src`. Looks like it was changed in an appropriately named PR: [_Cleanup and other bikeshedding_](https://github.com/frewsxcv/rust-threadpool/pull/43). Is the idea that because this library is self-contained you can easily compile it sans-cargo like `rustc --crate-type lib ./lib.rs`? ~~That’s also true of `scoped-threadpool-rs`, but not of `crossbeam`.~~ None of the other libraries mentioned under _Similar libraries_ have this constraint though. They can all be build with a simple `rustc` invocation. I still think it’s a nice feature though. ---
**Author:** ![crlf0710](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/crlf0710/32/1987_2.png) [@crlf0710](https://internals.rust-lang.org/u/crlf0710)\ **Post date:** [August 3, 2017, 5:10am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/7 "2017-08-03T05:10:48Z")
I think we should also compare this to `futures_cpupool`, which is often mentioned along with `futures-rs` and `tokio`. When should people use this and when should people use that? ---
**Author:** ![frewsxcv](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/frewsxcv/32/1268_2.png) [@frewsxcv](https://internals.rust-lang.org/u/frewsxcv)\ **Post date:** [August 3, 2017, 12:01pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/8 "2017-08-03T12:01:30Z")
I added the `lib.rs` at the root level since we’re only working with a single file, the `src/` directory felt a little overkill, but I’m happy to move it back to `src/lib.rs` for the sake of consistency across Rust crates. ---
**Author:** ![frewsxcv](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/frewsxcv/32/1268_2.png) [@frewsxcv](https://internals.rust-lang.org/u/frewsxcv)\ **Post date:** [August 3, 2017, 12:16pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/9 "2017-08-03T12:16:25Z")
Do you find it confusing enough that we should get rid of the `Clone` implementation? I've never personally used `clone` on a `ThreadPool`, and am not entirely certain what the use-case is. We could also just add documentation to the trait implementation. Tangential though, there's an open PR for adding more trait implementations that could use some feedback: > ---
**Author:** ![frewsxcv](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/frewsxcv/32/1268_2.png) [@frewsxcv](https://internals.rust-lang.org/u/frewsxcv)\ **Post date:** [August 3, 2017, 1:56pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/10 "2017-08-03T13:56:57Z")
I agree there should be some distinction or description about the threadpool in the ‘threadpool’ crate and scoped threadpools in the ‘similar libraries’ section of the readme. There’s a [pretty good description of what a scoped thread is in the crossbeam crate](https://docs.rs/crossbeam/0.3.0/crossbeam/struct.Scope.html#method.spawn). For what it’s worth, I’m not opposed to extending the ‘threadpool’ crate to include a scoped threadpool in addition to the existing one. How do people feel about that? The crate could be an all-in-one sort of solution ---
**Author:** ![KodrAus](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kodraus/32/2816_2.png) [@KodrAus](https://internals.rust-lang.org/u/KodrAus)\ **Post date:** [August 3, 2017, 9:29pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/11 "2017-08-03T21:29:12Z")
I think a scoped threadpool wouldn’t go astray, if that’s additional complexity and scope (get it?) you’re willing to accept. Some other random thoughts: - Having a quick poke around `crates.io`, `threadpool` looks like the most depended on thread pool library. - I think the big difference between `threadpool` and `futures-cpupool` is the use of `futures` for driving units of work. Futures introduces more complexity, but if you’re already in the context of a future then it makes sense to use them. ---
**Author:** ![dns2utf8](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dns2utf8/32/1713_2.png) [@dns2utf8](https://internals.rust-lang.org/u/dns2utf8)\ **Post date:** [August 5, 2017, 8:21pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/12 "2017-08-05T20:21:16Z")
Implementing the open features is fun. I will open another PR and implement `Clone` by hand. That way I can add documentation to it. EDIT: [https://github.com/frewsxcv/rust-threadpool/pull/79](https://github.com/frewsxcv/rust-threadpool/pull/79) ---
**Author:** ![ericho](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ericho/32/2406_2.png) [@ericho](https://internals.rust-lang.org/u/ericho)\ **Post date:** [August 8, 2017, 3:53am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/13 "2017-08-08T03:53:09Z")
I’m wondering if the suggestion of the scoped threadpool is out of the purpose of this evaluation. I mean, the purpose of this effort is to get the crate in a better shape, so i’m not sure if a new feature fits here unless is a blocking issue. ---
**Author:** ![KodrAus](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kodraus/32/2816_2.png) [@KodrAus](https://internals.rust-lang.org/u/KodrAus)\ **Post date:** [August 8, 2017, 5:21am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/14 "2017-08-08T05:21:34Z")
> [@frewsxcv](#): > > I’ve never personally used clone on a ThreadPool, and am not entirely certain what the use-case is. I've used `clone` a bit on the `futures-cpupool` so it can be shared by multiple things that want to offload work. I think it's a reasonable thing to do in the way the proposed implementation does it. The [docs for `futures-cpupool`](https://docs.rs/futures-cpupool/0.1.5/futures_cpupool/struct.CpuPool.html) have this to say: > Currently `CpuPool` implements `Clone` which just clones a new reference to the underlying thread pool. > [@ericho](#): > > I’m wondering if the suggestion of the scoped threadpool is out of the purpose of this evaluation. You're right it could well be out of scope, but I think it's a good opportunity to explore the possibility. It could be added in the future as a non-breaking change so I don't think we need to block on it. ---
**Author:** ![dns2utf8](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dns2utf8/32/1713_2.png) [@dns2utf8](https://internals.rust-lang.org/u/dns2utf8)\ **Post date:** [August 8, 2017, 3:32pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/15 "2017-08-08T15:32:06Z")
I think with the current behavior of clone we get a reasonable interface for sharing an entrypoint to the pool over many threads. While developping the `join` feature I noticed that moving the `Sender` [(see docs)](https://doc.rust-lang.org/stable/std/sync/mpsc/struct.Sender.html) from the job-queue between too many threads will cause a panic at some point because it is not `Sync`. So using an `Arc` to move the pool around will lead to problems. I will revisite the implementation of the bookkeeping with the input I received from other people. Probably less related: We managed to finish post-processing my talk about the threadpool. You can watch it here: - [Direct link](https://data.estada.ch/rust_z%C3%BCrichsee/Rust-Meetup_2017-08-02_ThreadPool.mp4) - [Youtube](https://youtu.be/N0b9fNw5lBM) - [Mozilla Air](https://air.mozilla.org/rust-zurichsee-august-meetup-stefan-schindler-threadpool-and-cargo-helpers/) ---
**Author:** ![KodrAus](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kodraus/32/2816_2.png) [@KodrAus](https://internals.rust-lang.org/u/KodrAus)\ **Post date:** [August 15, 2017, 12:22am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/17 "2017-08-15T00:22:38Z")
From the etherpad: > Since it [`ThreadPool`] is `Clone`, it could implement `Eq`. But I can not figure out a use case. \* use case: image you have many pools and you pass them around. At some point you would like to know if two entrypoints work on the same pool. Hmm, this seems a bit strange to me. Even if you had the knowledge that 2 `ThreadPool`s pointed to the same pool I'm not sure what you could really do with that information. At least it seems very niche, and something you could do by wrapping a `ThreadPool` in a newtype along with an identifier if you _really_ need it. ---
**Author:** ![KodrAus](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kodraus/32/2816_2.png) [@KodrAus](https://internals.rust-lang.org/u/KodrAus)\ **Post date:** [August 15, 2017, 12:32am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/18 "2017-08-15T00:32:22Z")
Ah nevermind, I see `ThreadPool` is actually already `Eq`. ---
**Author:** ![frewsxcv](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/frewsxcv/32/1268_2.png) [@frewsxcv](https://internals.rust-lang.org/u/frewsxcv)\ **Post date:** [August 20, 2017, 8:47pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/19 "2017-08-20T20:47:41Z")
Just opened a PR that adds a `Builder` with the intention of removing the custom constructors (like `ThreadPool::with_name`) in the future. [https://github.com/rust-threadpool/rust-threadpool/pull/83](https://github.com/rust-threadpool/rust-threadpool/pull/83) I think the `PartialEq`, `Eq`, and `Clone` implementations on the `ThreadPool` are a bit strange and very niche. I’m thinking for a 2.0.0 release, I’ll remove them and if the user needs that functionality, all they need to do is wrap the `ThreadPool` in an `Arc`. ---
**Author:** ![vitalyd](https://avatars.discourse-cdn.com/v4/letter/v/edb3f5/32.png) [@vitalyd](https://internals.rust-lang.org/u/vitalyd)\ **Post date:** [August 21, 2017, 1:56am UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/20 "2017-08-21T01:56:26Z")
I’m not sure `Clone` should be replaced with `Arc`. I don’t think there’s anything niche about sending handles to a threadpool to threads. If you put in `Arc`, then any `&mut self` methods are uncallable. PartialEq and Eq, however, indeed seem odd. ---
**Author:** ![dns2utf8](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dns2utf8/32/1713_2.png) [@dns2utf8](https://internals.rust-lang.org/u/dns2utf8)\ **Post date:** [September 13, 2017, 6:09pm UTC](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495/21 "2017-09-13T18:09:11Z")
In my opinion `Clone` and `PartialEq` go hand in hand. Once you can have more than one entrypoint to a pool you will most likely want to compare it with other entrypoints to be able to tell them apart. An `Arc` is not an option here, because we use a `channel` internally, [see example](https://play.rust-lang.org/?gist=828d338bf3cb4214a8127786d89fe085&version=stable) [Next page](https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-07-06-threadpool/5495.md?page=2)