Skip to content

Conversation

@bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Dec 15, 2025

Couple of cleanups I found while working on #149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.

Perhaps the old name used to be accurate in the past, but nowadays most
injected dependencies are unconditionally linked too.
It is always identical to the list of values in cnum_map.
@rustbot 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 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, +1 on the rename

View changes since this review

tcx,
sym::core,
CrateDepKind::Explicit,
CrateDepKind::Unconditional,
Copy link
Member

Choose a reason for hiding this comment

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

Remark: okay I was about to ask what's the distinction between Explicit/Implicit, but this commit answers my question 😆

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 15, 2025

📌 Commit 0e1e72a has been approved by jieyouxu

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 Dec 15, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2025
…=jieyouxu

Metadata loader cleanups

Couple of cleanups I found while working on rust-lang#149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.
bors added a commit that referenced this pull request Dec 15, 2025
Rollup of 9 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149354 (Bootstrap config: libgccjit libs dir)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser )
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2025
…=jieyouxu

Metadata loader cleanups

Couple of cleanups I found while working on rust-lang#149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.
bors added a commit that referenced this pull request Dec 15, 2025
Rollup of 8 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser )
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 16, 2025
…=jieyouxu

Metadata loader cleanups

Couple of cleanups I found while working on rust-lang#149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.
bors added a commit that referenced this pull request Dec 16, 2025
Rollup of 14 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149771 (bootstrap readme: make easy to read when editor wrapping is not enabled)
 - #149856 (Provide an extended framework for type visit, for use in rust-analyzer)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)
 - #150022 (Generate macro expansion for rust compiler crates docs)
 - #150029 (Update books)
 - #150031 (assert impossible branch is impossible)
 - #150034 (do not add `I-prioritize` when `F-*` labels are present)
 - #150036 (Use the embeddable filename for coverage artifacts)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Dec 16, 2025
Rollup of 13 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149771 (bootstrap readme: make easy to read when editor wrapping is not enabled)
 - #149856 (Provide an extended framework for type visit, for use in rust-analyzer)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)
 - #150029 (Update books)
 - #150031 (assert impossible branch is impossible)
 - #150034 (do not add `I-prioritize` when `F-*` labels are present)
 - #150036 (Use the embeddable filename for coverage artifacts)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0d0f136 into rust-lang:main Dec 16, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 16, 2025
rust-timer added a commit that referenced this pull request Dec 16, 2025
Rollup merge of #150014 - bjorn3:metadata_loader_cleanups, r=jieyouxu

Metadata loader cleanups

Couple of cleanups I found while working on #149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.
@bjorn3 bjorn3 deleted the metadata_loader_cleanups branch December 16, 2025 10:46
@bjorn3
Copy link
Member Author

bjorn3 commented Dec 16, 2025

There is a minor regression overall regression in the rollup. Lets check if this is the cause:

@rust-timer build 434f473

@rust-timer

This comment has been minimized.

@rust-timer

This comment was marked as resolved.

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 16, 2025

I was too quick. CI for the unrolled commit hadn't finished yet.

@rust-timer build 434f473

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (434f473): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.3%, -0.2%] 4
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -1.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.2% [3.2%, 3.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.2% [-3.9%, -2.8%] 4
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.8% [2.7%, 2.9%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 479.175s -> 479.501s (0.07%)
Artifact size: 390.21 MiB -> 390.22 MiB (0.00%)

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 16, 2025

Looks like this was indeed the cause. It is going to be either aff1f2a or a0f8dff. I don't see how the others could regress performance. 0e1e72a is the only other commit that is not purely a rename, but that one should only be able to improve performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

5 participants