Improve dependency_format a bit #134514

bjorn3 · 2024-12-19T13:48:03Z

Make DependencyList an IndexVec rather than emulating one using a Vec (which was off-by-one as LOCAL_CRATE was intentionally skipped)
Update some comments for the fact that we now use #[global_allocator] rather than extern crate alloc_system;/extern crate alloc_jemalloc; for specifying which allocator to use. We still use a similar mechanism for the panic runtime, so refer to the panic runtime in those comments instead.
An unrelated refactor to create_and_enter_global_ctxt I forgot to include in Remove queries from the driver interface #134302. This refactor is too small to be worth it's own PR.

rustbot · 2024-12-19T13:48:12Z

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler/rustc_metadata/src/dependency_format.rs

jieyouxu

Nice cleanup, you can r=me after PR CI is green.

compiler/rustc_codegen_ssa/src/back/link.rs

compiler/rustc_codegen_ssa/src/back/linker.rs

rustbot · 2024-12-19T14:34:52Z

The Miri subtree was changed

cc @rust-lang/miri

src/tools/miri/src/helpers.rs

jieyouxu

Thanks, LGTM in general, I left some feedback.

compiler/rustc_codegen_ssa/src/back/link.rs

compiler/rustc_metadata/src/dependency_format.rs

jieyouxu · 2024-12-19T16:57:32Z

@rustbot author

bjorn3 · 2024-12-20T08:35:19Z

@rustbot ready

jieyouxu

Cool

jieyouxu · 2024-12-20T10:08:34Z

@bors r+ rollup

bors · 2024-12-20T10:09:15Z

📌 Commit 0daa921 has been approved by jieyouxu

It is now in the queue for this repository.

bjorn3 · 2024-12-20T12:47:32Z

I'm not sure if rolling up a linkage related PR is the best idea.

@bors rollup=iffy

Rollup of 5 pull requests Successful merges: - rust-lang#134366 (Fix logical error with what text is considered whitespace.) - rust-lang#134514 (Improve dependency_format a bit) - rust-lang#134519 (ci: use ubuntu `24` instead of `latest`) - rust-lang#134551 (coverage: Rename `basic_coverage_blocks` to just `graph`) - rust-lang#134553 (add member constraints comment) r? `@ghost` `@rustbot` modify labels: rollup

…eyouxu Improve dependency_format a bit * Make `DependencyList` an `IndexVec` rather than emulating one using a `Vec` (which was off-by-one as LOCAL_CRATE was intentionally skipped) * Update some comments for the fact that we now use `#[global_allocator]` rather than `extern crate alloc_system;`/`extern crate alloc_jemalloc;` for specifying which allocator to use. We still use a similar mechanism for the panic runtime, so refer to the panic runtime in those comments instead. * An unrelated refactor to `create_and_enter_global_ctxt` I forgot to include in rust-lang#134302. This refactor is too small to be worth it's own PR.

jieyouxu · 2024-12-20T17:40:22Z

@bors rollup=never (to make it easier to bisect if this causes issues)

Rollup merge of rust-lang#134514 - bjorn3:more_driver_refactors, r=jieyouxu Improve dependency_format a bit * Make `DependencyList` an `IndexVec` rather than emulating one using a `Vec` (which was off-by-one as LOCAL_CRATE was intentionally skipped) * Update some comments for the fact that we now use `#[global_allocator]` rather than `extern crate alloc_system;`/`extern crate alloc_jemalloc;` for specifying which allocator to use. We still use a similar mechanism for the panic runtime, so refer to the panic runtime in those comments instead. * An unrelated refactor to `create_and_enter_global_ctxt` I forgot to include in rust-lang#134302. This refactor is too small to be worth it's own PR.

DianQK · 2024-12-21T12:47:46Z

@bors rollup=never (to make it easier to bisect if this causes issues)

Oops..

DianQK · 2024-12-21T12:49:28Z

I believe that improvement come from this PR.
@rust-timer build 08f8f9d

jieyouxu · 2024-12-21T14:22:07Z

Oops..

My bad, I didn't notice the rollup backlink, not a big deal.

rust-timer · 2024-12-21T14:46:35Z

Finished benchmarking commit (08f8f9d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-1.4%, -0.1%]	16
Improvements ✅ (secondary)	-2.9%	[-4.3%, -1.3%]	3
All ❌✅ (primary)	-0.5%	[-1.4%, -0.1%]	16

Max RSS (memory usage)

Results (primary -1.5%, secondary -4.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.8%	[0.9%, 2.7%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-3.7%, -0.9%]	8
Improvements ✅ (secondary)	-4.9%	[-7.9%, -2.5%]	3
All ❌✅ (primary)	-1.5%	[-3.7%, 2.7%]	10

Cycles

Results (primary -1.7%, secondary -5.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.7%	[-1.9%, -1.4%]	2
Improvements ✅ (secondary)	-5.7%	[-6.4%, -5.0%]	2
All ❌✅ (primary)	-1.7%	[-1.9%, -1.4%]	2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 769.09s -> 768.925s (-0.02%)
Artifact size: 330.24 MiB -> 330.33 MiB (0.03%)

bjorn3 · 2024-12-21T15:45:06Z

I'm surprised that this PR improves performance and reduces memory usage.

rustbot assigned compiler-errors Dec 19, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 19, 2024

bjorn3 commented Dec 19, 2024

View reviewed changes

compiler/rustc_metadata/src/dependency_format.rs Show resolved Hide resolved

bjorn3 force-pushed the more_driver_refactors branch from 582e5d3 to c828f9b Compare December 19, 2024 13:51

jieyouxu approved these changes Dec 19, 2024

View reviewed changes

jieyouxu assigned jieyouxu and unassigned compiler-errors Dec 19, 2024

bjorn3 commented Dec 19, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/back/link.rs Show resolved Hide resolved

bjorn3 force-pushed the more_driver_refactors branch from c828f9b to 498fab3 Compare December 19, 2024 13:56

bjorn3 commented Dec 19, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/back/linker.rs Show resolved Hide resolved

jieyouxu self-requested a review December 19, 2024 14:25

bjorn3 force-pushed the more_driver_refactors branch from 498fab3 to 341d672 Compare December 19, 2024 14:34

RalfJung reviewed Dec 19, 2024

View reviewed changes

src/tools/miri/src/helpers.rs Outdated Show resolved Hide resolved

bjorn3 added 3 commits December 19, 2024 15:30

Minor cleanup

38bc902

Make DependencyList an IndexVec

7e6be13

Update comments

943f6a8

bjorn3 force-pushed the more_driver_refactors branch from 341d672 to 943f6a8 Compare December 19, 2024 15:30

jieyouxu approved these changes Dec 19, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/back/link.rs Show resolved Hide resolved

compiler/rustc_codegen_ssa/src/back/link.rs Outdated Show resolved Hide resolved

compiler/rustc_metadata/src/dependency_format.rs Show resolved Hide resolved

rustbot 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 Dec 19, 2024

Review comments

0daa921

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 20, 2024

jieyouxu approved these changes Dec 20, 2024

View reviewed changes

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 Dec 20, 2024

DianQK mentioned this pull request Dec 20, 2024

Rollup of 5 pull requests #134559

Merged

jieyouxu mentioned this pull request Dec 20, 2024

Rollup of 6 pull requests #134571

Closed

bors merged commit 350e7f8 into rust-lang:master Dec 20, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 20, 2024

bjorn3 deleted the more_driver_refactors branch December 21, 2024 11:55

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

jieyouxu mentioned this pull request Dec 21, 2024

Bump compiler cc to 1.2.5 #134505

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve dependency_format a bit #134514

Improve dependency_format a bit #134514

bjorn3 commented Dec 19, 2024

rustbot commented Dec 19, 2024

jieyouxu left a comment

rustbot commented Dec 19, 2024

jieyouxu left a comment

jieyouxu commented Dec 19, 2024

bjorn3 commented Dec 20, 2024

jieyouxu left a comment

jieyouxu commented Dec 20, 2024

bors commented Dec 20, 2024

bjorn3 commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

DianQK commented Dec 21, 2024

DianQK commented Dec 21, 2024

This comment has been minimized.

jieyouxu commented Dec 21, 2024

rust-timer commented Dec 21, 2024

bjorn3 commented Dec 21, 2024

This comment was marked as resolved.

Improve dependency_format a bit #134514

Improve dependency_format a bit #134514

Conversation

bjorn3 commented Dec 19, 2024

rustbot commented Dec 19, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

rustbot commented Dec 19, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Dec 19, 2024

bjorn3 commented Dec 20, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Dec 20, 2024

bors commented Dec 20, 2024

bjorn3 commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

DianQK commented Dec 21, 2024

DianQK commented Dec 21, 2024

This comment has been minimized.

jieyouxu commented Dec 21, 2024

rust-timer commented Dec 21, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

bjorn3 commented Dec 21, 2024

This comment was marked as resolved.