Make `DefId` `repr(C)`, optimize big-endian field order by llogiq · Pull Request #92012 · rust-lang/rust

llogiq · 2021-12-16T23:46:58Z

r? @michaelwoerister

jackh726 · 2021-12-17T01:50:21Z

@bors try @rust-timer queue

rust-timer · 2021-12-17T01:50:22Z

Awaiting bors try build completion.

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

bors · 2021-12-17T01:50:29Z

⌛ Trying commit 95a3e6adc5e724e93a62915b22380b6137be08db with merge 6de93a5f3c58289159da3d736b0361ca414bd34f...

bors · 2021-12-17T03:11:21Z

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

rust-timer · 2021-12-17T03:11:22Z

Queued 6de93a5f3c58289159da3d736b0361ca414bd34f with parent 5531927, future comparison URL.

rust-timer · 2021-12-17T05:35:01Z

Finished benchmarking commit (6de93a5f3c58289159da3d736b0361ca414bd34f): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

Very large improvement in instruction counts (up to -6.8% on incr-unchanged builds of ctfe-stress-4)
Large regression in instruction counts (up to 3.5% on incr-unchanged builds of unicode_normalization)

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

llogiq · 2021-12-17T08:49:29Z

That's weird. Even if I did something wrong, it shouldn't change the outcome by that much.

michaelwoerister · 2021-12-17T12:33:05Z

Yeah, that is weird. We'd expect the outcome to not change at all...

jackh726 · 2021-12-17T14:13:07Z

I wonder if something is going wrong with perf: #91672 (comment)

llogiq · 2021-12-17T21:17:59Z

Yeah, that looks really fishy. Perhaps someone with access to perf can have a look?

llogiq · 2021-12-17T22:48:25Z

Ah it's likely because of rust-lang/rustc-perf#1123

Mark-Simulacrum · 2021-12-17T23:20:19Z

@bors retry @rust-timer queue

rust-timer · 2021-12-17T23:20:20Z

Awaiting bors try build completion.

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

bors · 2021-12-17T23:20:31Z

⌛ Trying commit 95a3e6adc5e724e93a62915b22380b6137be08db with merge 6a6e67427da0929979dc434855e205efe90bd2a3...

bors · 2021-12-18T00:51:11Z

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

rust-timer · 2021-12-18T00:51:12Z

Queued 6a6e67427da0929979dc434855e205efe90bd2a3 with parent 7abab1e, future comparison URL.

rust-timer · 2021-12-18T05:01:12Z

Finished benchmarking commit (6a6e67427da0929979dc434855e205efe90bd2a3): comparison url.

Summary: This change led to moderate relevant regressions 😿 in compiler performance.

Moderate regression in instruction counts (up to 3.2% on incr-patched: println builds of regression-31157)

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

llogiq · 2021-12-18T09:47:28Z

So there is something wrong. I'll have a look.

llogiq · 2021-12-20T11:58:30Z

Can we get another perf run? I wonder if removing the repr(C) fixes the regression. Otherwise it must be because of the #[cfg(..)]s. I also wonder if at this point it would make sense to change DefId to a single u64 and make index and krate methods instead, but that change would be far larger. The upside would be that we could freely change the internal representation should we ever desire it.

michaelwoerister · 2022-01-03T10:05:32Z

perf.rlo seems to have problems with increased noise at the moment: rust-lang/rustc-perf#1126

Let's wait for that to be fixed and then revisit this.

rust-timer · 2022-01-03T23:33:27Z

Queued 586b3d2e1d80845457c6700c7e3deee19006521b with parent ddabe07, future comparison URL.

rust-timer · 2022-01-04T01:29:00Z

Finished benchmarking commit (586b3d2e1d80845457c6700c7e3deee19006521b): comparison url.

Summary: This benchmark run did not return any relevant 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. 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.

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

llogiq · 2022-01-04T07:08:33Z

Ok, that's what we were expecting. Now I'll add the #[repr(C)] back.

Mark-Simulacrum · 2022-01-04T09:33:16Z

@bors try @rust-timer queue

rust-timer · 2022-01-04T09:33:17Z

Awaiting bors try build completion.

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

bors · 2022-01-04T09:33:53Z

⌛ Trying commit 7f7b551 with merge a5535ef48e9bb30104929abcdb9f1a6eec3d532f...

bors · 2022-01-04T11:07:08Z

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

rust-timer · 2022-01-04T11:07:09Z

Queued a5535ef48e9bb30104929abcdb9f1a6eec3d532f with parent 2b681ac, future comparison URL.

rust-timer · 2022-01-04T12:37:15Z

Finished benchmarking commit (a5535ef48e9bb30104929abcdb9f1a6eec3d532f): comparison url.

Summary: This benchmark run did not return any relevant 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. 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.

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

llogiq · 2022-01-04T20:34:07Z

Cool. This means we haven't lost any perf on any tier-1 systems and may have gained a little on big-endian 64-bit machines, not that there would be a lot of them around.

michaelwoerister · 2022-01-10T09:20:48Z

Looks good now!
@bors r+

bors · 2022-01-10T09:20:49Z

📌 Commit 7f7b551 has been approved by michaelwoerister

bors · 2022-01-11T08:09:09Z

⌛ Testing commit 7f7b551 with merge e4b1d58...

bors · 2022-01-11T11:17:53Z

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing e4b1d58 to master...

rust-timer · 2022-01-11T12:35:08Z

Finished benchmarking commit (e4b1d58): comparison url.

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

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

@rustbot label: -perf-regression

rust-highfive assigned michaelwoerister Dec 16, 2021

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 16, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 16, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 17, 2021

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 17, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 17, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 18, 2021

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. perf-regression Performance regression. labels Jan 4, 2022

Make DefIdrepr(C), optimize big-endian field order
7f7b551

llogiq force-pushed the repr-c-def-id branch from f37a6fe to 7f7b551Compare January 4, 2022 07:17

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

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 4, 2022

bors mentioned this pull request Jan 11, 2022
Add missing suffix for sidebar-items script path #92742
Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 11, 2022

bors merged commit e4b1d58 into rust-lang:masterJan 11, 2022

rustbot added this to the 1.60.0 milestone Jan 11, 2022

Uh oh!

Uh oh!

Conversation

llogiq commented Dec 16, 2021

Uh oh!

jackh726 commented Dec 17, 2021

Uh oh!

rust-timer commented Dec 17, 2021

Uh oh!

bors commented Dec 17, 2021

Uh oh!

bors commented Dec 17, 2021

Uh oh!

rust-timer commented Dec 17, 2021

Uh oh!

rust-timer commented Dec 17, 2021

Uh oh!

llogiq commented Dec 17, 2021

Uh oh!

michaelwoerister commented Dec 17, 2021

Uh oh!

jackh726 commented Dec 17, 2021

Uh oh!

llogiq commented Dec 17, 2021

Uh oh!

llogiq commented Dec 17, 2021

Uh oh!

Mark-Simulacrum commented Dec 17, 2021

Uh oh!

rust-timer commented Dec 17, 2021

Uh oh!

bors commented Dec 17, 2021

Uh oh!

bors commented Dec 18, 2021

Uh oh!

rust-timer commented Dec 18, 2021

Uh oh!

rust-timer commented Dec 18, 2021

Uh oh!

llogiq commented Dec 18, 2021

Uh oh!

llogiq commented Dec 20, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Jan 3, 2022

Uh oh!

rust-timer commented Jan 3, 2022

Uh oh!

rust-timer commented Jan 4, 2022

Uh oh!

llogiq commented Jan 4, 2022

Uh oh!

Mark-Simulacrum commented Jan 4, 2022

Uh oh!

rust-timer commented Jan 4, 2022

Uh oh!

bors commented Jan 4, 2022

Uh oh!

bors commented Jan 4, 2022

Uh oh!

rust-timer commented Jan 4, 2022

Uh oh!

rust-timer commented Jan 4, 2022

Uh oh!

llogiq commented Jan 4, 2022

Uh oh!

michaelwoerister commented Jan 10, 2022

Uh oh!

bors commented Jan 10, 2022

Uh oh!

bors commented Jan 11, 2022

Uh oh!

bors commented Jan 11, 2022

Uh oh!

rust-timer commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

llogiq commented Dec 20, 2021•
edited
Loading