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.

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

Jobserver cleanup #11764

Merged
merged 4 commits into from
Mar 1, 2023
Merged

Jobserver cleanup #11764

merged 4 commits into from
Mar 1, 2023

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Feb 24, 2023 •
edited
Loading

What does this PR try to resolve?

-Zjobserver-per-rustc has been broken for a long while. My guess is that we never make -Zjobserver-token-request into rustc (rust-lang/rust#67398). To reduce the complexity of the current job queue implementation, I propose to remove this portion. Cargo doesn't really receive any jobserver_event1 from the official rustc.

We can always bring this back if needed.

How should we test and review this PR?

You can take the PR adding this feature as a reference #7731. This PR removes things below:

  • -Zjobserver-per-rustc itself.
  • Logic where rustc is waiting for tokens and how many tokens a rustc owns.
  • Timing info shows “Max (global) rustc threads concurrency:”, which was never correct.
    image

Additional information

Updated: See Zulip discussion. People are okay to remove this.

Still in draft because relevant documentations haven't yet got updated. I want to make sure that it is okay to remove, then I'll update docs accordingly.

Related PRs:

cc @Mark-Simulacrum to check if there is anyone still using this feature doing experiments.

Footnotes

  1. https://github.com/rust-lang/cargo/blob/65cab34dc75587eeaff68d3c19358c4d79041452/src/cargo/core/compiler/mod.rs#L1704-L1713 ↩

@rustbot
Copy link
Collaborator

rustbot commented Feb 24, 2023

r? @epage

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

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-timings Area: timings A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2023
@weihanglo weihanglo added the A-jobserver Area: jobserver, concurrency, parallelism label Feb 24, 2023
@weihanglo
Remove rustc threads concurrency info from timings
8737512 
Given `-Zjobserver-token-request` in rustc is never landed,
This is never correct either.
@weihanglo
Remove support of -Zjobserver-per-rustc
60e72cc 
This effectively remove fields and logic where rustc is waiting for
tokens and how many tokens a rustc owns.
@weihanglo
Remove the entry point triggering -Zjobserver-per-rustc
a880806
@weihanglo
Update doc due to removal of cargo-rustc jobserver interaction
d8fb0db
@weihanglo weihanglo marked this pull request as ready for review March 1, 2023 16:58
@weihanglo
Copy link
Member Author

This is ready for review. Thank you!

@epage
Copy link
Contributor

epage commented Mar 1, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Mar 1, 2023

📌 Commit d8fb0db has been approved by epage

It is now in the queue for this repository.

@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 Mar 1, 2023
@bors
Copy link
Contributor

bors commented Mar 1, 2023

⌛ Testing commit d8fb0db with merge 9580786...

@bors
Copy link
Contributor

bors commented Mar 1, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 9580786 to master...

@bors bors merged commit 9580786 into rust-lang:master Mar 1, 2023
@weihanglo weihanglo deleted the jobserver-cleanup branch March 1, 2023 18:41
@bors bors mentioned this pull request Mar 1, 2023
Merged
16 tasks
@weihanglo weihanglo mentioned this pull request Mar 2, 2023
Merged
weihanglo added a commit to weihanglo/rust that referenced this pull request Mar 7, 2023
@weihanglo
Update cargo
e52e040 
23 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..c1334b059c6dcceab3c10c81413f79bb832c8d9d
2023-02-28 19:39:39 +0000 to 2023-03-07 19:21:50 +0000

- Add `CARGO_PKG_README` (rust-lang/cargo#11645)
- path dependency: fix cargo-util version (rust-lang/cargo#11807)
- Adding display of which target failed to compile (rust-lang/cargo#11636)
- Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790)
- Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805)
- Enhance the doc of timing report with graphs (rust-lang/cargo#11798)
- Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791)
- Use sha2 to calculate SHA256 (rust-lang/cargo#11795)
- gitoxide progress bar fixes (rust-lang/cargo#11800)
- Check publish_to_alt_registry publish content (rust-lang/cargo#11799)
- chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797)
- chore: Update base64 (rust-lang/cargo#11796)
- Fix some doc typos (rust-lang/cargo#11794)
- chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761)
- Some cleanup for unstable docs (rust-lang/cargo#11793)
- gitoxide integration: fetch (rust-lang/cargo#11448)
- patch can conflict on not activated packages (rust-lang/cargo#11770)
- fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630)
- Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783)
- feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688)
- feat: Use test name for dir when running tests (rust-lang/cargo#11738)
- Jobserver cleanup (rust-lang/cargo#11764)
- Fix help string for  "--charset" option of "cargo tree" (rust-lang/cargo#11785)

Note that some 3rd-party licensing allowed list changed due to the
introducion of `gix` dependency
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2023
@bors
Auto merge of rust-lang#108665 - weihanglo:update-cargo, r=weihanglo
900c354 
Update cargo

25 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..7d3033d2e59383fd76193daf9423c3d141972a7d
2023-02-28 19:39:39 +0000 to 2023-03-08 17:05:08 +0000

- Revert "rust-lang/cargo#11738" - Use test name for dir when running tests (rust-lang/cargo#11812)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11810)
- Add `CARGO_PKG_README` (rust-lang/cargo#11645)
- path dependency: fix cargo-util version (rust-lang/cargo#11807)
- Adding display of which target failed to compile (rust-lang/cargo#11636)
- Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790)
- Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805)
- Enhance the doc of timing report with graphs (rust-lang/cargo#11798)
- Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791)
- Use sha2 to calculate SHA256 (rust-lang/cargo#11795)
- gitoxide progress bar fixes (rust-lang/cargo#11800)
- Check publish_to_alt_registry publish content (rust-lang/cargo#11799)
- chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797)
- chore: Update base64 (rust-lang/cargo#11796)
- Fix some doc typos (rust-lang/cargo#11794)
- chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761)
- Some cleanup for unstable docs (rust-lang/cargo#11793)
- gitoxide integration: fetch (rust-lang/cargo#11448)
- patch can conflict on not activated packages (rust-lang/cargo#11770)
- fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630)
- Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783)
- feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688)
- feat: Use test name for dir when running tests (rust-lang/cargo#11738)
- Jobserver cleanup (rust-lang/cargo#11764)
- Fix help string for  "--charset" option of "cargo tree" (rust-lang/cargo#11785)

---

~~This update is primarily for making rust-lang/cargo#11630 into 1.69~~ (will file a beta backport then). However, just look into the licenses and dependencies permitted list, it looks a bit unfortunate but inevitable I guess?

r? `@ehuss`
cc `@Muscraft`
@ehuss ehuss added this to the 1.70.0 milestone Mar 9, 2023
saethlin pushed a commit to saethlin/miri that referenced this pull request Mar 11, 2023
@bors
Auto merge of #108665 - weihanglo:update-cargo, r=weihanglo
5894753 
Update cargo

25 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..7d3033d2e59383fd76193daf9423c3d141972a7d
2023-02-28 19:39:39 +0000 to 2023-03-08 17:05:08 +0000

- Revert "rust-lang/cargo#11738" - Use test name for dir when running tests (rust-lang/cargo#11812)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11810)
- Add `CARGO_PKG_README` (rust-lang/cargo#11645)
- path dependency: fix cargo-util version (rust-lang/cargo#11807)
- Adding display of which target failed to compile (rust-lang/cargo#11636)
- Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790)
- Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805)
- Enhance the doc of timing report with graphs (rust-lang/cargo#11798)
- Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791)
- Use sha2 to calculate SHA256 (rust-lang/cargo#11795)
- gitoxide progress bar fixes (rust-lang/cargo#11800)
- Check publish_to_alt_registry publish content (rust-lang/cargo#11799)
- chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797)
- chore: Update base64 (rust-lang/cargo#11796)
- Fix some doc typos (rust-lang/cargo#11794)
- chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761)
- Some cleanup for unstable docs (rust-lang/cargo#11793)
- gitoxide integration: fetch (rust-lang/cargo#11448)
- patch can conflict on not activated packages (rust-lang/cargo#11770)
- fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630)
- Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783)
- feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688)
- feat: Use test name for dir when running tests (rust-lang/cargo#11738)
- Jobserver cleanup (rust-lang/cargo#11764)
- Fix help string for  "--charset" option of "cargo tree" (rust-lang/cargo#11785)

---

~~This update is primarily for making rust-lang/cargo#11630 into 1.69~~ (will file a beta backport then). However, just look into the licenses and dependencies permitted list, it looks a bit unfortunate but inevitable I guess?

r? `@ehuss`
cc `@Muscraft`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@epage epage

Labels
A-build-execution Area: anything dealing with executing the compiler A-jobserver Area: jobserver, concurrency, parallelism A-timings Area: timings A-unstable Area: nightly unstable support S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.
5 participants
@weihanglo @rustbot @epage @bors @ehuss