Reduce the cost of loading all built-ins targets #95202

Urgau · 2022-03-22T10:47:35Z

This PR started by measuring the exact slowdown of checking of well known conditional values.
Than this PR implemented some technics to reduce the cost of loading all built-ins targets.

cf. #82450 (comment)

lqd · 2022-03-22T11:12:40Z

@bors try @rust-timer queue

rust-timer · 2022-03-22T11:12:42Z

Awaiting bors try build completion.

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

bors · 2022-03-22T11:12:52Z

⌛ Trying commit aeadc42389613ae25e868809e26100fa960da068 with merge f1bad32f32f5ed8240872cb387d09a3635f62297...

bors · 2022-03-22T12:47:43Z

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

rust-timer · 2022-03-22T12:47:45Z

Queued f1bad32f32f5ed8240872cb387d09a3635f62297 with parent b9c4067, future comparison URL.

rust-timer · 2022-03-22T14:21:45Z

Finished benchmarking commit (f1bad32f32f5ed8240872cb387d09a3635f62297): comparison url.

Summary: This benchmark run shows 68 relevant regressions 😿 to instruction counts.

Arithmetic mean of relevant regressions: 1.4%
Largest regression in instruction counts: 4.0% on full builds of helloworld check

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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Urgau · 2022-03-22T15:46:23Z

@petrochenkov Do you want me to try with Cow<String> instead of String for the targets ? or something else ?

petrochenkov · 2022-03-25T14:22:12Z

@Urgau
Yes, non-empty Strings are the main blocker for constification of target specs, but there may be other issues.

Urgau · 2022-03-25T15:50:48Z

Before measuring the cost (hopefully negative) of replacing every String by Cow<String>, I would be good to check whenever it's actually significative. I just push a commit who disable the checking but NOT the loading of values (targets, ...).

bjorn3 · 2022-03-25T15:53:03Z

@bors try @rust-timer queue

rust-timer · 2022-03-25T15:53:04Z

Awaiting bors try build completion.

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

bors · 2022-03-25T15:53:18Z

⌛ Trying commit 93a4d854318dcc9c758403213ebb77be98eaa02c with merge f2bca08ebf42b573bcacc3ae5a8dacabb8f03268...

bors · 2022-03-25T17:12:45Z

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

rust-timer · 2022-03-25T17:12:46Z

Queued f2bca08ebf42b573bcacc3ae5a8dacabb8f03268 with parent 903427b, future comparison URL.

rust-timer · 2022-03-26T23:58:52Z

Finished benchmarking commit (f2bca08ebf42b573bcacc3ae5a8dacabb8f03268): comparison url.

Summary: This benchmark run shows 68 relevant regressions 😿 to instruction counts.

Arithmetic mean of relevant regressions: 1.5%
Largest regression in instruction counts: 4.2% on full builds of helloworld check

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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

petrochenkov · 2022-04-03T14:52:51Z

The PR's title and description need to be updated.
Also CI is failing.
@bors rollup=maybe

petrochenkov · 2022-04-03T18:13:37Z

Unit tests in rustc_target are failing.
@rustbot author

bjorn3 · 2022-04-03T18:20:22Z

compiler/rustc_target/src/spec/mod.rs

@@ -1107,6 +1104,8 @@ impl HasTargetSpec for Target {
    }
 }

+type StaticCow<T> = Cow<'static, T>;


I don't think this adds much. It only saves 2 characters.

I asked to add it in #95202 (comment) because it looks less noisy due to removing punctuation like ' and , even if it's only slightly shorter. (Not a strong opinion.)

Urgau · 2022-04-03T19:33:28Z

Review comments addressed or responded. Commits squashed to only 4, but can squash to only 1 if you prefer.
Title and PR description also addressed. Also fix all of the failing unit tests (incorrectly removed to_string() when serializing pointer_width).

@rustbot ready

petrochenkov · 2022-04-03T21:00:21Z

@bors r+

bors · 2022-04-03T21:00:22Z

📌 Commit 1a1f5b8 has been approved by petrochenkov

bors · 2022-04-03T21:18:27Z

⌛ Testing commit 1a1f5b8 with merge 8a8b80634e8c439c8b36e3b3729a5094606ae9cd...

Dylan-DPC · 2022-04-03T21:22:44Z

@bors retry (yield to rollup containing this pr)

Rollup of 5 pull requests Successful merges: - rust-lang#95202 (Reduce the cost of loading all built-ins targets) - rust-lang#95553 (Don't emit non-asm contents error for naked function composed of errors) - rust-lang#95613 (Fix rustdoc attribute display) - rust-lang#95617 (Fix &mut invalidation in ptr::swap doctest) - rust-lang#95618 (core: document that the align_of* functions return the alignment in bytes) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-log-analyzer · 2022-04-03T21:25:55Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 22, 2022

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 Mar 22, 2022

rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 22, 2022

petrochenkov self-assigned this Mar 25, 2022

petrochenkov 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 Mar 25, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 25, 2022

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 26, 2022

Urgau force-pushed the check-cfg-perf-well-known-values branch from 1424502 to 2191d2a Compare April 3, 2022 17:11

Urgau changed the title ~~[PERF] Always enable checking of well known conditional values~~ Reduce cost of creating built-ins targets Apr 3, 2022

Urgau changed the title ~~Reduce cost of creating built-ins targets~~ Reduce the cost of loading all built-ins targets Apr 3, 2022

Urgau force-pushed the check-cfg-perf-well-known-values branch from 2191d2a to 303611c Compare April 3, 2022 17:35

bjorn3 reviewed Apr 3, 2022

View reviewed changes

Urgau added 4 commits April 3, 2022 21:29

Replace every String in Target(Options) with Cow<'static, str>

ccff48f

Replace every Vec in Target(Options) with it's Cow equivalent

ce61d40

Replace LinkArgs with Cow<'static, str>

c16a558

Cleanup after some refactoring in rustc_target

1a1f5b8

Urgau force-pushed the check-cfg-perf-well-known-values branch from 303611c to 1a1f5b8 Compare April 3, 2022 19:33

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 Apr 3, 2022

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 Apr 3, 2022

Dylan-DPC mentioned this pull request Apr 3, 2022

Rollup of 5 pull requests #95624

Merged

bors merged commit 796bc7e into rust-lang:master Apr 3, 2022

rustbot added this to the 1.61.0 milestone Apr 3, 2022

a1phyr mentioned this pull request Apr 27, 2022

Codegen features are no longer recognized by rustc #96472

Open

Urgau mentioned this pull request May 20, 2022

Tracking Issue for RFC 3013: Checking conditional compilation at compile time #82450

Closed

3 tasks

petrochenkov mentioned this pull request Aug 30, 2022

rustc_target: Add a compatibility layer to separate internal and user-facing linker flavors #100552

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce the cost of loading all built-ins targets #95202

Reduce the cost of loading all built-ins targets #95202

Urgau commented Mar 22, 2022 •

edited

Loading

This comment has been minimized.

lqd commented Mar 22, 2022

rust-timer commented Mar 22, 2022

bors commented Mar 22, 2022

bors commented Mar 22, 2022

rust-timer commented Mar 22, 2022

rust-timer commented Mar 22, 2022

Urgau commented Mar 22, 2022

petrochenkov commented Mar 25, 2022

Urgau commented Mar 25, 2022

bjorn3 commented Mar 25, 2022

rust-timer commented Mar 25, 2022

bors commented Mar 25, 2022

This comment has been minimized.

bors commented Mar 25, 2022

rust-timer commented Mar 25, 2022

rust-timer commented Mar 26, 2022

petrochenkov commented Apr 3, 2022

petrochenkov commented Apr 3, 2022

bjorn3 Apr 3, 2022

petrochenkov Apr 3, 2022

Urgau commented Apr 3, 2022 •

edited

Loading

petrochenkov commented Apr 3, 2022

bors commented Apr 3, 2022

bors commented Apr 3, 2022

Dylan-DPC commented Apr 3, 2022

rust-log-analyzer commented Apr 3, 2022

Reduce the cost of loading all built-ins targets #95202

Reduce the cost of loading all built-ins targets #95202

Conversation

Urgau commented Mar 22, 2022 • edited Loading

This comment has been minimized.

lqd commented Mar 22, 2022

rust-timer commented Mar 22, 2022

bors commented Mar 22, 2022

bors commented Mar 22, 2022

rust-timer commented Mar 22, 2022

rust-timer commented Mar 22, 2022

Urgau commented Mar 22, 2022

petrochenkov commented Mar 25, 2022

Urgau commented Mar 25, 2022

bjorn3 commented Mar 25, 2022

rust-timer commented Mar 25, 2022

bors commented Mar 25, 2022

This comment has been minimized.

bors commented Mar 25, 2022

rust-timer commented Mar 25, 2022

rust-timer commented Mar 26, 2022

petrochenkov commented Apr 3, 2022

petrochenkov commented Apr 3, 2022

bjorn3 Apr 3, 2022

Choose a reason for hiding this comment

petrochenkov Apr 3, 2022

Choose a reason for hiding this comment

Urgau commented Apr 3, 2022 • edited Loading

petrochenkov commented Apr 3, 2022

bors commented Apr 3, 2022

bors commented Apr 3, 2022

Dylan-DPC commented Apr 3, 2022

rust-log-analyzer commented Apr 3, 2022

Urgau commented Mar 22, 2022 •

edited

Loading

Urgau commented Apr 3, 2022 •

edited

Loading