Skip to content

Conversation

xizheyin
Copy link
Member

@xizheyin xizheyin commented Jul 24, 2025

I inline some small method in rustc_span/src/hygiene.rs and so on.

@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 Jul 24, 2025
@compiler-errors
Copy link
Member

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jul 24, 2025

⌛ Trying commit e233072 with merge 70a86f7

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

rust-bors bot added a commit that referenced this pull request Jul 24, 2025
Optimize performance in macro hygiene system
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 24, 2025
@rust-bors
Copy link

rust-bors bot commented Jul 24, 2025

☀️ Try build successful (CI)
Build commit: 70a86f7 (70a86f741181f85bf71587a6464a6364479c2806, parent: efd420c770bb179537c01063e98cb6990c439654)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (70a86f7): 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.1% [-0.1%, -0.1%] 5
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 5.1%, secondary 0.2%)

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

mean range count
Regressions ❌
(primary)
5.1% [5.1%, 5.1%] 1
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 5.1% [5.1%, 5.1%] 1

Cycles

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

Binary size

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

Bootstrap: 468.582s -> 468.539s (-0.01%)
Artifact size: 374.67 MiB -> 374.67 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 24, 2025
@petrochenkov
Copy link
Contributor

Did you check that the #[inline] annotations actually have effect?
I.e. without them the functions are not inlined on some hot path, and with them they are inlined.
Most of the marked functions are crate-private so LLVM already can inline them if it thinks it makes sense.

@xizheyin
Copy link
Member Author

I tested the execution efficiency of the first commit locally, and it does show a slight improvement in execution time, but I'm not sure if all the functions marked by #[inline] are actually inlined. do you have a good idea?

@petrochenkov 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 Jul 24, 2025
@xizheyin
Copy link
Member Author

@rustbot ready
The code in the master branch now works just fine. I removed the second commit, which did not result in a performance improvement. It seems that the improvement may only come from #[inline]. Do we need to run the bot again to test performance?

@rustbot 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 Jul 24, 2025
@petrochenkov
Copy link
Contributor

Since we are adding inline to small functions, it probably makes sense to add it to HygieneData::with and with_session_globals as well.
We can run benchmarks after that.

@petrochenkov
Copy link
Contributor

The PR description also needs to be updated.
@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 24, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 24, 2025
@xizheyin xizheyin changed the title Optimize performance in macro hygiene system Optimize performance by inline in macro hygiene system Jul 24, 2025
@xizheyin
Copy link
Member Author

@rustbot ready

@petrochenkov
Copy link
Contributor

@bors2 try @rust-timer queue

@rustbot 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 Jul 24, 2025
@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jul 24, 2025

⌛ Trying commit 60d6980 with merge dc79eb3

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

rust-bors bot added a commit that referenced this pull request Jul 24, 2025
Optimize performance by inline in macro hygiene system
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 24, 2025
@rust-bors
Copy link

rust-bors bot commented Jul 24, 2025

☀️ Try build successful (CI)
Build commit: dc79eb3 (dc79eb38141d9436db26729956e443911d304dad, parent: fc5af1813307d25a84d633f21e2e53c9376eb547)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dc79eb3): 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.1% [-0.1%, -0.1%] 4
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -1.0%)

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

Cycles

Results (secondary -3.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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.9% [-4.6%, -3.2%] 2
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 468.258s -> 469.253s (0.21%)
Artifact size: 374.68 MiB -> 374.65 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 24, 2025
@xizheyin
Copy link
Member Author

The results look good, all of them are improved.

@petrochenkov
Copy link
Contributor

@bors r+ rollup=maybe

@bors
Copy link
Collaborator

bors commented Jul 25, 2025

📌 Commit 60d6980 has been approved by petrochenkov

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 Jul 25, 2025
Kobzol added a commit to Kobzol/rust that referenced this pull request Jul 25, 2025
…nkov

Optimize performance by inline in macro hygiene system

I inline some small method in `rustc_span/src/hygiene.rs` and so on.
bors added a commit that referenced this pull request Jul 27, 2025
Rollup of 4 pull requests

Successful merges:

 - #144226 (Do not assert layout in KnownPanicsLint.)
 - #144385 (Optimize performance by inline in macro hygiene system)
 - #144454 (move uefi test to run-make)
 - #144455 (Unify LLVM ctlz/cttz intrinsic generation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e0a4299 into rust-lang:master Jul 27, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 27, 2025
rust-timer added a commit that referenced this pull request Jul 27, 2025
Rollup merge of #144385 - xizheyin:macro-hygiene, r=petrochenkov

Optimize performance by inline in macro hygiene system

I inline some small method in `rustc_span/src/hygiene.rs` and so on.
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.

6 participants