Skip to content

Mark <mem::DropGuard as Drop>::drop as #[inline] - #162064

Open
GrigorenkoPV wants to merge 1 commit into
rust-lang:mainfrom
GrigorenkoPV:inline-drop-guard-drop
Open

Mark <mem::DropGuard as Drop>::drop as #[inline]#162064
GrigorenkoPV wants to merge 1 commit into
rust-lang:mainfrom
GrigorenkoPV:inline-drop-guard-drop

Conversation

@GrigorenkoPV

@GrigorenkoPV GrigorenkoPV commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

#161702 has caused a perf regression. As suggested in #161702 (review), this might help.

@bors try @rust-timer queue
(I probably don't have permissions for those)

@rust-bors

rust-bors Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

@GrigorenkoPV: 🔑 Insufficient privileges: not in try users

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 31, 2026
@rustbot

rustbot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

r? @JohnTitor

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, joboet, nia-e

@lqd

lqd commented Aug 31, 2026

Copy link
Copy Markdown
Member

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 31, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 31, 2026
Mark `<mem::DropGuard as Drop>::drop` as `#[inline]`
@clarfonthey

Copy link
Copy Markdown
Contributor

r? clarfonthey since I reviewed the other change

@rustbot rustbot assigned clarfonthey and unassigned JohnTitor Aug 31, 2026
@rust-bors

rust-bors Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 4606595 (4606595922d66540e5fa94815f9717898f3ffc1d)
Base parent: 45f215f (45f215f136e00d8a74c69afde2f71be3f16837cf)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (4606595): comparison URL.

Overall result: ❌✅ regressions and improvements - 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 rustc-perf
@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.9% [0.3%, 1.5%] 2
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
-0.4% [-0.7%, -0.2%] 5
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.7%, 1.5%] 7

Max RSS (memory usage)

Results (primary 2.6%, secondary -2.9%)

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

mean range count
Regressions ❌
(primary)
2.6% [1.1%, 4.0%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) 2.6% [1.1%, 4.0%] 2

Cycles

Results (secondary -3.2%)

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.2% [2.0%, 2.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.4% [-7.1%, -2.4%] 9
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.5%, 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)
1.3% [1.3%, 1.3%] 1
Regressions ❌
(secondary)
0.7% [0.7%, 0.7%] 1
Improvements ✅
(primary)
-0.6% [-11.1%, -0.0%] 49
Improvements ✅
(secondary)
-1.3% [-11.1%, -0.0%] 11
All ❌✅ (primary) -0.5% [-11.1%, 1.3%] 50

Bootstrap: 475.395s -> 473.906s (-0.31%)
Artifact size: 401.38 MiB -> 400.48 MiB (-0.22%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 31, 2026
@clarfonthey

Copy link
Copy Markdown
Contributor

Tempted to say these are a net positive but will take another look later.

@panstromek

panstromek commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This is positive, yes, especially if you look at binary sizes - in some benchmarks, like html5ever-0.31.0 it more then offsets the regression from the original PR.

This doesn't address the original regression so we have to be a bit careful to asses the impact overall. It's possible (but unlikely in my opinion) that this is only positive because #161702 was negative and the benefit of this PR largely disappears when we address the root cause. Not a blocking concern, but something to keep in mind for example if later results come back unexpectedly red.

@clarfonthey

clarfonthey commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Right, this does not address the main regression, but still is a net positive, IMO. I guess I'll hold off on merging in case we'd rather revert the original before implementing a proper fix.

@clarfonthey

Copy link
Copy Markdown
Contributor

Side note: with the full revert in #162128, it's up to you if you want to reuse this PR or open a new one for whatever fix you come up with. I'm not sure I can delegate perf runs, but will try and run them when you ask.

@clarfonthey

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbot rustbot 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 Sep 2, 2026
@rustbot

rustbot commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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

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

Labels

perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants