Skip to content

Remove unused dependencies from compiler crates #86617

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

Merged
merged 15 commits into from
Jul 1, 2021

Conversation

joshtriplett
Copy link
Member

Various compiler crates have dependencies that they don't appear to use. I used some scripting to detect such dependencies, filtered them based on some manual review, and removed those that do indeed appear to be entirely unused.

@rust-highfive
Copy link
Contributor

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 Jun 25, 2021
@rust-log-analyzer

This comment has been minimized.

rustc_data_structures has a dependency on crossbeam-utils but never uses
it. It appears to have originally had this dependency in order to set
the "nightly" feature; however, its other dependencies use a different
version of crossbeam-utils, so this doesn't actually affect anything.
Furthermore, in current crossbeam-utils, the "nightly" feature has
become a no-op.
@joshtriplett
Copy link
Member Author

Updated Cargo.lock in each commit.

Useful command:

git rebase -i origin --exec 'cargo tree > /dev/null && git add Cargo.lock && git commit --amend --no-edit'

@rust-log-analyzer

This comment has been minimized.

Unused since commit 622c48e
("Allow making `RUSTC_BOOTSTRAP` conditional on the crate name").
Unused since commit f141acf
("Move finalize_session_directory call out of cg_llvm").
…peck

Unused since commit dc3eabd
("Store THIR in `IndexVec`s instead of an `Arena`").
Unused since commit 7d73e4c
("Remove ReScope").
Unused since commit 50e1ae1
("Use ty::{IntTy,UintTy,FloatTy} in rustc").
Unused since commit cd7a011
("Don't duplicate the extern providers once for each crate").
Unused since commit 8331dbe
("Add an Mmap wrapper to rustc_data_structures").
Unused since commit 4581d16
("Move the query system to rustc_query_impl.").
Unused since commit 530a629
("Remove pretty-print/reparse hack, and add derive-specific hack").
Unused since commit 6a32e79
("stabilize union with 'ManuallyDrop' fields and 'impl Drop for Union'").
Unused since commit 64af7ea
("Move SanitizerSet to rustc_target").
Unused since commit 50e1ae1
("Use ty::{IntTy,UintTy,FloatTy} in rustc").
@joshtriplett
Copy link
Member Author

Dropped the removal of rustc_data_structures from rustc_attr, as it's used via derive(HashStable_Generic).

@joshtriplett
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 25, 2021
@bors
Copy link
Collaborator

bors commented Jun 25, 2021

⌛ Trying commit 7a9d419 with merge 83f01bd5cc130c2f354a61775a6bb8dc2cf42b8f...

@bors
Copy link
Collaborator

bors commented Jun 25, 2021

☀️ Try build successful - checks-actions
Build commit: 83f01bd5cc130c2f354a61775a6bb8dc2cf42b8f (83f01bd5cc130c2f354a61775a6bb8dc2cf42b8f)

@rust-timer
Copy link
Collaborator

Queued 83f01bd5cc130c2f354a61775a6bb8dc2cf42b8f with parent 50a9081, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (83f01bd5cc130c2f354a61775a6bb8dc2cf42b8f): comparison url.

Summary: This benchmark run did not return any significant changes.

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2021

Error: Label perf-regression can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@JohnTitor JohnTitor removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 25, 2021
@JohnTitor
Copy link
Member

Label perf-regression can only be set by Rust team members

I just created the perf-regression label as it was missing.
@Mark-Simulacrum Is the perfbot considered as a team member? If not we should tweak the triagebot config.

@JohnTitor
Copy link
Member

Although the bot said the result didn't have any significant changes, there're a lot of small regressions it seems.

@joshtriplett
Copy link
Member Author

The benchmark results appear to be entirely noise/neutral, which is what I expected. I was primarily doing the perf run for the bootstrap timings; however, it looks like those timings are a simple sum of individual crate timings, which won't reflect any additional parallelism potentially made available by the looser dependencies.

@Mark-Simulacrum
Copy link
Member

The benchmark results appear to be entirely noise/neutral, which is what I expected. I was primarily doing the perf run for the bootstrap timings; however, it looks like those timings are a simple sum of individual crate timings, which won't reflect any additional parallelism potentially made available by the looser dependencies.

Yes, we currently use -j1 to reduce noise in the bootstrap measurements.

@joshtriplett is there a reason this is still marked as a draft PR?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 28, 2021
@Mark-Simulacrum
Copy link
Member

(r=me if it's ready to go on your end)

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 28, 2021
…Mark-Simulacrum

Allow anyone to set `perf-regression` label

The main purpose is to allow the triage bot to set the label: rust-lang#86617 (comment)

r? `@Mark-Simulacrum`
@joshtriplett joshtriplett marked this pull request as ready for review June 28, 2021 19:31
@joshtriplett
Copy link
Member Author

Yes, we currently use -j1 to reduce noise in the bootstrap measurements.

I absolutely think that's the right thing to do in general. Is there some way to get perf measurements of a normal parallel build (and ideally the graph timings of "how early can things start"), without having to run them locally?

is there a reason this is still marked as a draft PR?

Just hadn't changed that yet. It started out as a draft because I wasn't sure if there were subtle dependencies I missed. I meant to change it to non-draft as soon as it built and showed a build performance improvement, and got distracted by the perf results not actually providing that information.

@Mark-Simulacrum
Copy link
Member

I absolutely think that's the right thing to do in general. Is there some way to get perf measurements of a normal parallel build (and ideally the graph timings of "how early can things start"), without having to run them locally?

No, not really. We in theory could add support for one-off collection on perf, but it's not trivial and doesn't seem particularly warranted.

@joshtriplett
Copy link
Member Author

@bors r=Mark-Simulacrum

@bors
Copy link
Collaborator

bors commented Jun 30, 2021

📌 Commit 7a9d419 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 30, 2021
@bors
Copy link
Collaborator

bors commented Jul 1, 2021

⌛ Testing commit 7a9d419 with merge 1034282...

@bors
Copy link
Collaborator

bors commented Jul 1, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 1, 2021
@bors bors merged commit 1034282 into rust-lang:master Jul 1, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 1, 2021
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. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants