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

Update some rustc dependencies to deduplicate them #92896

Merged
merged 3 commits into from
Jan 21, 2022

Conversation

lqd
Copy link
Member

@lqd lqd commented Jan 14, 2022

This PR updates rand and itertools in rustc (not the whole workspace) in order to deduplicate them (and hopefully slightly improve compile times).

Currently, object is still duplicated, but rust-lang/thorin#15 and updating thorin in the future will remove the use of version 0.27. Update: Thorin 0.2 has now been released, and this PR updates rustc_codegen_ssa to use it and deduplicate the object crate.

There's a final tiny rustc dependency, cfg-if, which will be left: as both versions 0.1.x and 1.0 looked to be heavily depended on, they will require a few cascading updates to be removed.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 14, 2022
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Jan 14, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

This seems good, and a follow-up PR can bump thorin once that gets released.

@bors
Copy link
Contributor

bors commented Jan 17, 2022

📌 Commit 3713562 has been approved by Mark-Simulacrum

@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 Jan 17, 2022
@lqd
Copy link
Member Author

lqd commented Jan 20, 2022

Thanks to @davidtwco for releasing thorin 0.2!

As discussed with Mark, this PR is still far from the top of the queue, and we can add the thorin-dwp update to deduplicate object at the same time.

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 20, 2022

📌 Commit 820fd05 has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 20, 2022

🌲 The tree is currently closed for pull requests below priority 600. This pull request will be tested once the tree is reopened.

@bors
Copy link
Contributor

bors commented Jan 21, 2022

⌛ Testing commit 820fd05 with merge 0367442179f1b7b7faa02ac14f827753f462e7eb...

@bors
Copy link
Contributor

bors commented Jan 21, 2022

💔 Test failed - checks-actions

@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 Jan 21, 2022
@lqd
Copy link
Member Author

lqd commented Jan 21, 2022

@bors retry osx failure while building stage0 std

error: /Applications/Xcode_13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: can't open file: /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/profiler_builtins-e02f38cb938c3431/out/libprofiler-rt.a (No such file or directory)

I wouldn't expect changes to rustc dependencies to affect building libstd with the beta compiler

@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 Jan 21, 2022
@bors
Copy link
Contributor

bors commented Jan 21, 2022

⌛ Testing commit 820fd05 with merge 84e9189...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] build_script_build test:false 0.473
[RUSTC-TIMING] build_script_build test:false 1.462
The following warnings were emitted during compilation:

warning: error: /Applications/Xcode_13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: can't open file: /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/profiler_builtins-e02f38cb938c3431/out/libprofiler-rt.a (No such file or directory)
warning: /Applications/Xcode_13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ar: internal ranlib command failed
error: failed to run custom build command for `profiler_builtins v0.0.0 (/Users/runner/work/rust/rust/library/profiler_builtins)`

Caused by:
  process didn't exit successfully: `/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/release/build/profiler_builtins-47d7f900160d7f14/build-script-build` (exit status: 1)
  process didn't exit successfully: `/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/release/build/profiler_builtins-47d7f900160d7f14/build-script-build` (exit status: 1)
  --- stdout
  TARGET = Some("x86_64-apple-darwin")
  HOST = Some("x86_64-apple-darwin")
  AR_x86_64-apple-darwin = Some("ar")
  running: "ar" "s" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/profiler_builtins-e02f38cb938c3431/out/libprofiler-rt.a"
  cargo:warning=error: /Applications/Xcode_13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: can't open file: /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/profiler_builtins-e02f38cb938c3431/out/libprofiler-rt.a (No such file or directory)
  cargo:warning=/Applications/Xcode_13.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ar: internal ranlib command failed

  --- stderr



  error occurred: Command "ar" "s" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/profiler_builtins-e02f38cb938c3431/out/libprofiler-rt.a" with args "ar" did not execute successfully (status code exit status: 1).

warning: build failed, waiting for other jobs to finish...
[RUSTC-TIMING] core test:false 33.033
error: build failed

@bors
Copy link
Contributor

bors commented Jan 21, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 84e9189 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 21, 2022
@bors bors merged commit 84e9189 into rust-lang:master Jan 21, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 21, 2022
@lqd lqd deleted the update-deps branch January 21, 2022 13:39
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (84e9189): comparison url.

Summary: This change led to moderate relevant regressions 😿 in compiler performance.

  • Moderate regression in instruction counts (up to 0.6% on full builds of match-stress-enum check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Jan 21, 2022
@lqd
Copy link
Member Author

lqd commented Jan 21, 2022

Nothing immediately jumps at me here. The changes in the match-stress-enum benchmarks for example don't seem to directly use the updated crates, so my guess was that it could be noise related to the hashbrown 0.11.0 to 0.11.2 update via thorin 0.2 ?

@pnkfelix
Copy link
Member

Hmm. Should we be doing perf runs even on PR's like this, so that we have a chance of seeing this kind of fallout ahead of time?

@lqd
Copy link
Member Author

lqd commented Jan 26, 2022

That could be nice, yeah. It could also be nice to do perf runs on all PRs before they landed, I presume.

On the other hand, most of these changes are to stress tests and not real world crates (also: small), compared to the improvements to bootstrap times (variance could be involved here), although I assume perf.rlo shows the deduplications as improvements to the build time of rustc's dependencies.

If we knew of the fallout ahead of time, we could triage the results earlier and that would make it easier for the weekly perf report process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants