Skip to content

make rustdoc test follow the jobserver limit of threads #50134

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 25, 2018
Merged

make rustdoc test follow the jobserver limit of threads #50134

merged 1 commit into from
Apr 25, 2018

Conversation

andjo403
Copy link
Contributor

@andjo403 andjo403 commented Apr 21, 2018 •
edited
Loading

fix that to many threads is executing at the same time
when rustdoc test is executed.

@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 21, 2018
@GuillaumeGomez
Copy link
Member

👍

michaelwoerister
michaelwoerister reviewed Apr 23, 2018
View reviewed changes
src/librustc_trans/back/write.rs
// Pick a "reasonable maximum" if we don't otherwise have a jobserver in
// our environment, capping out at 32 so we don't take everything down
// by hogging the process run queue.
Client::new(32).expect("failed to create jobserver")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcrichton, do you remember why we picked 32 instead of num_cpus here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes that has to do with deterministic compilation. With num_cpus then you can have different results across machines from the sheer fact that the number of cpus were different. With 32, however, it's deterministic across all machines.

michaelwoerister
michaelwoerister reviewed Apr 23, 2018
View reviewed changes
src/librustc/session/mod.rs Outdated
static INIT: std::sync::Once = std::sync::ONCE_INIT;
INIT.call_once(|| {
GLOBAL_JOBSERVER = Box::into_raw(Box::new(Client::from_env()));
let client = Client::from_env().unwrap_or_else(|| {
Client::new(::num_cpus::get()).expect("failed to create jobserver")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andjo403, given @alexcrichton's comment below, could you change this 32? Most of the time there'll be a jobserver around anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelwoerister have updated now and added a comment about why there is a fixed value.
Thanks for the feedback

@andjo403
make rustdoc test follow the jobserver limit of threadsfix that to ma…
ed318dd 
…ny threads is executing at the same timewhen rustdoc test is executed.
@michaelwoerister
Copy link
Member

Is this finished? If so, how do the changes relate to rustdoc?

@andjo403
Copy link
Contributor Author

yes this was all that I think is needed, it works when I tested.
The problem was that when running "rustdoc test" the default is that number of cpus threads will start to compile the examples from the comments and due to there was no jobserver defined when it came to the start_async_translation it created one new jobserver for every test thread resulting in "number of cpu"*32 maximum threads created. And with this solution there is only one jobserver created due to the global variable for all compilations.
if you have a chrome browser you can see in this page https://andjo403.github.io/rs_tracing/rustdoc32.html how almost 300 threads is executing codegen on my 32 threads cpu

@michaelwoerister
Copy link
Member

Oh ok, that makes sense! Great find!

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 24, 2018

📌 Commit ed318dd has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2018
@bors
Copy link
Collaborator

bors commented Apr 25, 2018

⌛ Testing commit ed318dd with merge ba3707e1a2fa8c282a345f2f7f660268b7a9a15a...

@bors
Copy link
Collaborator

bors commented Apr 25, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2018
@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
[00:01:23]  Downloading serde_json v1.0.15
[00:01:23] error: unable to get packages from source
[00:01:23] 
[00:01:23] Caused by:
[00:01:23]   failed to get 200 response from `https://crates.io/api/v1/crates/serde_json/1.0.15/download`, got 404
[00:01:23] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:23] Build completed unsuccessfully in 0:00:36
[00:01:23] make: *** [prepare] Error 1
[00:01:23] Makefile:81: recipe for target 'prepare' failed
[00:01:24]  Downloading num_cpus v1.8.0
[00:01:24] error: unable to get packages from source
[00:01:24] 
[00:01:24] Caused by:
[00:01:24] Caused by:
[00:01:24]   failed to get 200 response from `https://crates.io/api/v1/crates/num_cpus/1.8.0/download`, got 404
[00:01:24] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:24] Build completed unsuccessfully in 0:00:00
[00:01:24] make: *** [prepare] Error 1
[00:01:24] Makefile:81: recipe for target 'prepare' failed
[00:01:25]  Downloading toml v0.4.6
[00:01:25] error: unable to get packages from source
[00:01:25] 
[00:01:25] Caused by:
[00:01:25] Caused by:
[00:01:25]   failed to get 200 response from `https://crates.io/api/v1/crates/toml/0.4.6/download`, got 404
[00:01:25] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:25] Build completed unsuccessfully in 0:00:00
[00:01:25] Makefile:81: recipe for target 'prepare' failed
[00:01:25] make: *** [prepare] Error 1
[00:01:25]  Downloading cc v1.0.10
[00:01:25] error: unable to get packages from source
[00:01:25] 
[00:01:25] Caused by:
[00:01:25] Caused by:
[00:01:25]   failed to get 200 response from `https://crates.io/api/v1/crates/cc/1.0.10/download`, got 404
[00:01:25] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:25] Build completed unsuccessfully in 0:00:00
[00:01:25] Makefile:81: recipe for target 'prepare' failed
[00:01:25] make: *** [prepare] Error 1
[00:01:26]  Downloading filetime v0.1.15
[00:01:26] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/filetime/0.1.15/download`, got 500
[00:01:26] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/filetime/0.1.15/download`, got 500
[00:01:26] error: unable to get packages from source
[00:01:26] error: unable to get packages from source
[00:01:26] 
[00:01:26] Caused by:
[00:01:26]   failed to get 200 response from `https://crates.io/api/v1/crates/filetime/0.1.15/download`, got 500
[00:01:26] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:26] Build completed unsuccessfully in 0:00:00
[00:01:26] make: *** [prepare] Error 1
[00:01:26] Makefile:81: recipe for target 'prepare' failed
[00:01:26] The command has failed after 5 attempts.

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
travis_time:start:213d22b8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton
Copy link
Member

alexcrichton commented Apr 25, 2018 via email

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2018
@bors
Copy link
Collaborator

bors commented Apr 25, 2018

⌛ Testing commit ed318dd with merge 81135c9...

bors added a commit that referenced this pull request Apr 25, 2018
@bors
Auto merge of #50134 - andjo403:jobserver, r=michaelwoerister
81135c9 
make rustdoc test follow the jobserver limit of threads

fix that to many threads is executing at the same time
when rustdoc test is executed.
@bors
Copy link
Collaborator

bors commented Apr 25, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 81135c9 to master...

@bors bors merged commit ed318dd into rust-lang:master Apr 25, 2018
@andjo403 andjo403 deleted the jobserver branch April 25, 2018 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

@michaelwoerister michaelwoerister michaelwoerister left review comments

Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@andjo403 @rust-highfive @GuillaumeGomez @michaelwoerister @bors @alexcrichton