Load all builtin targets at once instead of one by one in check-cfg #137072

Urgau · 2025-02-15T10:57:28Z

This PR adds a method on rustc_target::Target to load all the builtin targets at once, and then uses that method when constructing the target_* values in check-cfg instead of load loading each target one by one by their name, which requires a lookup and was more of a hack anyway.

This may give us some performance improvements as we won't need to do the lookup for the currently 287 targets we have.

rustbot · 2025-02-15T10:57:36Z

r? @Noratrieb

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

rustbot · 2025-02-15T10:57:37Z

These commits modify compiler targets.
(See the Target Tier Policy.)

Urgau · 2025-02-15T10:58:04Z

@bors try @rust-timer queue

…e, r=<try> Load all builtin targets at once instead of one by one in check-cfg This PR adds a method on `rustc_target::Target` to load all the builtin targets at once, and then uses that method when constructing the `target_*` values in check-cfg instead of load loading each target one by one by their name, which requires a lookup. This may give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

bors · 2025-02-15T11:00:30Z

⌛ Trying commit 5ce8335 with merge 20bc5f6...

bors · 2025-02-15T12:48:42Z

☀️ Try build successful - checks-actions
Build commit: 20bc5f6 (20bc5f66e48eb4d9e2c2668b389fa9a12efdaa1c)

rust-timer · 2025-02-15T17:24:10Z

Finished benchmarking commit (20bc5f6): comparison URL.

Overall result: ❌✅ regressions and 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.4%	[0.4%, 0.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.9%, secondary 3.1%)

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)	2.9%	[0.6%, 6.8%]	8
Regressions ❌ (secondary)	3.1%	[2.6%, 3.7%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.9%	[0.6%, 6.8%]	8

Cycles

Results (secondary 4.1%)

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)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: missing data
Artifact size: 347.22 MiB -> 347.39 MiB (0.05%)

This should give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

Urgau · 2025-02-15T17:52:00Z

@bors try @rust-timer queue

…e, r=<try> Load all builtin targets at once instead of one by one in check-cfg This PR adds a method on `rustc_target::Target` to load all the builtin targets at once, and then uses that method when constructing the `target_*` values in check-cfg instead of load loading each target one by one by their name, which requires a lookup and was more of a hack anyway. This may give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

bors · 2025-02-15T17:53:14Z

⌛ Trying commit 6ec3cf9 with merge 86f627b...

bors · 2025-02-15T19:40:32Z

☀️ Try build successful - checks-actions
Build commit: 86f627b (86f627b06d8f3bf88ae6d2f6f7c87ee1f2ccc4ae)

rust-timer · 2025-02-15T20:56:44Z

Finished benchmarking commit (86f627b): 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
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.1%]	3
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -2.1%)

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)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Cycles

Results (primary -3.0%)

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)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.0%	[-3.0%, -3.0%]	1

Binary size

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

Bootstrap: 789.995s -> 790.069s (0.01%)
Artifact size: 347.21 MiB -> 347.31 MiB (0.03%)

Noratrieb

Doesn't look like it helps performance, but I think it's in an improvement to the code nevertheless.

Noratrieb · 2025-02-16T10:00:18Z

@bors r+

bors · 2025-02-16T10:00:20Z

📌 Commit 6ec3cf9 has been approved by Noratrieb

It is now in the queue for this repository.

Noratrieb · 2025-02-16T10:01:00Z

@bors rollup

…nce, r=Noratrieb Load all builtin targets at once instead of one by one in check-cfg This PR adds a method on `rustc_target::Target` to load all the builtin targets at once, and then uses that method when constructing the `target_*` values in check-cfg instead of load loading each target one by one by their name, which requires a lookup and was more of a hack anyway. This may give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125087 (Optimize `Seek::stream_len` impl for `File`) - rust-lang#136986 (Apply unsafe_op_in_unsafe_fn to the standard library) - rust-lang#137012 (add docs and ut for bootstrap util cc-detect) - rust-lang#137072 (Load all builtin targets at once instead of one by one in check-cfg) - rust-lang#137102 (Rework `name_regions` to not rely on reverse scc graph for non-member-constrain usages) - rust-lang#137112 (Don't project into `NonNull` when dropping a `Box`) - rust-lang#137114 (Add an example for `std::error::Error`) - rust-lang#137117 (Fix test that relies on error language) - rust-lang#137119 (fix broken `x {doc, build} core`) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136986 (Apply unsafe_op_in_unsafe_fn to the standard library) - rust-lang#137012 (add docs and ut for bootstrap util cc-detect) - rust-lang#137072 (Load all builtin targets at once instead of one by one in check-cfg) - rust-lang#137102 (Rework `name_regions` to not rely on reverse scc graph for non-member-constrain usages) - rust-lang#137112 (Don't project into `NonNull` when dropping a `Box`) - rust-lang#137114 (Add an example for `std::error::Error`) - rust-lang#137117 (Fix test that relies on error language) - rust-lang#137119 (fix broken `x {doc, build} core`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137072 - Urgau:check-cfg-load-builtins-at-once, r=Noratrieb Load all builtin targets at once instead of one by one in check-cfg This PR adds a method on `rustc_target::Target` to load all the builtin targets at once, and then uses that method when constructing the `target_*` values in check-cfg instead of load loading each target one by one by their name, which requires a lookup and was more of a hack anyway. This may give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

rustbot assigned Noratrieb Feb 15, 2025

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 Feb 15, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 15, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 15, 2025

Load all builtin targets at once instead of one by one

6ec3cf9

This should give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have.

Urgau force-pushed the check-cfg-load-builtins-at-once branch from 5ce8335 to 6ec3cf9 Compare February 15, 2025 17:49

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 15, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 15, 2025

Noratrieb approved these changes Feb 16, 2025

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 Feb 16, 2025

matthiaskrgr mentioned this pull request Feb 16, 2025

Rollup of 9 pull requests #137124

Closed

GuillaumeGomez mentioned this pull request Feb 16, 2025

Rollup of 10 pull requests #137130

Closed

matthiaskrgr mentioned this pull request Feb 16, 2025

Rollup of 8 pull requests #137143

Merged

bors merged commit fc094a1 into rust-lang:master Feb 17, 2025
7 checks passed

rustbot added this to the 1.87.0 milestone Feb 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load all builtin targets at once instead of one by one in check-cfg #137072

Load all builtin targets at once instead of one by one in check-cfg #137072

Urgau commented Feb 15, 2025 •

edited

Loading

rustbot commented Feb 15, 2025

rustbot commented Feb 15, 2025

Urgau commented Feb 15, 2025

This comment has been minimized.

bors commented Feb 15, 2025

bors commented Feb 15, 2025

This comment has been minimized.

rust-timer commented Feb 15, 2025

Urgau commented Feb 15, 2025

This comment has been minimized.

bors commented Feb 15, 2025

bors commented Feb 15, 2025

This comment has been minimized.

rust-timer commented Feb 15, 2025

Noratrieb left a comment

Noratrieb commented Feb 16, 2025

bors commented Feb 16, 2025

Noratrieb commented Feb 16, 2025

Load all builtin targets at once instead of one by one in check-cfg #137072

Load all builtin targets at once instead of one by one in check-cfg #137072

Conversation

Urgau commented Feb 15, 2025 • edited Loading

rustbot commented Feb 15, 2025

rustbot commented Feb 15, 2025

Urgau commented Feb 15, 2025

This comment has been minimized.

bors commented Feb 15, 2025

bors commented Feb 15, 2025

This comment has been minimized.

rust-timer commented Feb 15, 2025

Overall result: ❌✅ regressions and improvements - no action needed

Urgau commented Feb 15, 2025

This comment has been minimized.

bors commented Feb 15, 2025

bors commented Feb 15, 2025

This comment has been minimized.

rust-timer commented Feb 15, 2025

Overall result: ✅ improvements - no action needed

Noratrieb left a comment

Choose a reason for hiding this comment

Noratrieb commented Feb 16, 2025

bors commented Feb 16, 2025

Noratrieb commented Feb 16, 2025

Urgau commented Feb 15, 2025 •

edited

Loading