Skip to content

Reduce AttributeKind from 40 to 32 bytes#155788

Draft
mejrs wants to merge 2 commits intorust-lang:mainfrom
mejrs:reduce_attr_size
Draft

Reduce AttributeKind from 40 to 32 bytes#155788
mejrs wants to merge 2 commits intorust-lang:mainfrom
mejrs:reduce_attr_size

Conversation

@mejrs
Copy link
Copy Markdown
Contributor

@mejrs mejrs commented Apr 25, 2026

@bors try @rust-timer queue

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 25, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
Reduce AttributeKind from 40 to 32 bytes
@rust-log-analyzer

This comment has been minimized.

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

JonathanBrouwer commented Apr 25, 2026

Fyi I tried this a while ago both with size 24 and 32, and both were perf regressions, which really surprised me.
But it's always a good idea to re-check :)

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

See #151507

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

Hmm your diff is a lot smaller so this might have some potential

@mejrs
Copy link
Copy Markdown
Contributor Author

mejrs commented Apr 25, 2026

I have another commit lined up that does it without boxing anything, by removing deprecated's Span.

The only user facing place where we need its span is in check_attr, where it lints for USELESS_DEPRECATED. Maybe we can move that to parsing. The other uses are in this lint pass with the stability attributes, where I don't care about nice spans.

We should probably do a pass and see which spans are actually still used. They won't matter for AttributeKind's size though.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 25, 2026

☀️ Try build successful (CI)
Build commit: 7f7461f (7f7461f22d1b2f465da5284d319e2a32dbd1b4a5, parent: fb76025f2fe6a1c8ce03fe0931f941c8077a2c34)

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@rust-timer build 7f7461f

It doesn't listen to pr descriptions :)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (7f7461f): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@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.2% [0.1%, 0.2%] 3
Regressions ❌
(secondary)
0.2% [0.2%, 0.3%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.1%, 0.2%] 3

Max RSS (memory usage)

Results (primary -0.6%, secondary 1.7%)

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

mean range count
Regressions ❌
(primary)
0.8% [0.8%, 0.8%] 1
Regressions ❌
(secondary)
4.4% [0.7%, 6.5%] 4
Improvements ✅
(primary)
-2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)
-1.9% [-2.7%, -1.0%] 3
All ❌✅ (primary) -0.6% [-2.0%, 0.8%] 2

Cycles

Results (secondary 6.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)
6.8% [3.3%, 10.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (secondary 0.1%)

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

Bootstrap: 487.131s -> 488.874s (0.36%)
Artifact size: 394.07 MiB -> 394.17 MiB (0.03%)

@rustbot rustbot added the perf-regression Performance regression. label Apr 25, 2026
@mejrs
Copy link
Copy Markdown
Contributor Author

mejrs commented Apr 26, 2026

@bors try @rust-timer queue

@rust-timer
Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 26, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 26, 2026

⌛ Trying commit 2ba2010 with merge 7d66ae7

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/24953850800

rust-bors Bot pushed a commit that referenced this pull request Apr 26, 2026
Reduce AttributeKind from 40 to 32 bytes
@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
 
Diff in /checkout/compiler/rustc_passes/src/stability.rs:217:
     }
 
     let const_stable_indirect = find_attr!(tcx, def_id, RustcConstStableIndirect);
-    let const_stab =
-        find_attr!(tcx, def_id, RustcConstStability { stability } => *stability);
+    let const_stab = find_attr!(tcx, def_id, RustcConstStability { stability } => *stability);
 
     // After checking the immediate attributes, get rid of the span and compute implied
     // const stability: inherit feature gate from regular stability.
Diff in /checkout/compiler/rustc_passes/src/stability.rs:387:
         if let Some(fn_sig) = fn_sig
             && !fn_sig.header.is_const()
             && const_stab.is_some()
-            && find_attr!(attrs, RustcConstStability{..})
+            && find_attr!(attrs, RustcConstStability { .. })
         {
             self.tcx.dcx().emit_err(errors::MissingConstErr { fn_sig_span: fn_sig.span });
         }
Diff in /checkout/compiler/rustc_passes/src/stability.rs:397:
             && let Some(fn_sig) = fn_sig
             && const_stab.is_const_stable()
             && !stab.is_some_and(|s| s.is_stable())
-            && find_attr!( attrs, RustcConstStability {..})
+            && find_attr!(attrs, RustcConstStability { .. })
         {
-            self.tcx
-                .dcx()
-                .emit_err(errors::ConstStableNotStable { fn_sig_span: fn_sig.span });
+            self.tcx.dcx().emit_err(errors::ConstStableNotStable { fn_sig_span: fn_sig.span });
         }
 
         if let Some(stab) = &const_stab
Diff in /checkout/compiler/rustc_passes/src/stability.rs:408:
             && stab.is_const_stable()
             && stab.const_stable_indirect
-            && find_attr!(attrs, RustcConstStability {..})
+            && find_attr!(attrs, RustcConstStability { .. })
         {
-            self.tcx.dcx().emit_err(errors::RustcConstStableIndirectPairing { span: rustc_span::DUMMY_SP });
+            self.tcx
+                .dcx()
+                .emit_err(errors::RustcConstStableIndirectPairing { span: rustc_span::DUMMY_SP });
         }
     }
 
fmt: checked 6821 files
Bootstrap failed while executing `test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`

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

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed. 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