Skip to content

Have arrays' drop_glue just unsize and call the slice version#155184

Open
scottmcm wants to merge 1 commit intorust-lang:mainfrom
scottmcm:intercept-array-drop-shim
Open

Have arrays' drop_glue just unsize and call the slice version#155184
scottmcm wants to merge 1 commit intorust-lang:mainfrom
scottmcm:intercept-array-drop-shim

Conversation

@scottmcm
Copy link
Copy Markdown
Member

@scottmcm scottmcm commented Apr 12, 2026

It's silly to emit two loops (because of the drop ladder -- just one in panic=abort) for every array length that's dropped when we can just polymorphize to the slice version.

Built atop #154327 to avoid conflicts later, so draft for now.

r? @WaffleLapkin

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` PG-exploit-mitigations Project group: Exploit mitigations 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 12, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

WaffleLapkin is not on the review rotation at the moment.
They may take a while to respond.

@scottmcm scottmcm added I-heavy Issue: Problems and improvements with respect to binary size of generated code. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. PG-exploit-mitigations Project group: Exploit mitigations A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` labels Apr 12, 2026

// RAW: ; core::ptr::drop_glue::<[array_drop_glue::NeedsDrop; [[N:7|13|42]]]>
// RAW-NEXT: inlinehint
// RAW: call core::ptr::drop_glue::<[array_drop_glue::NeedsDrop]>
Copy link
Copy Markdown
Member Author

@scottmcm scottmcm Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View changes since the review

Compare nightly https://rust.godbolt.org/z/5Wv1q86ja with a loop in each of the three.

@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin
Copy link
Copy Markdown
Member

💭 does it make sense to keep the array case separate for N <= 1 or something?

@scottmcm
Copy link
Copy Markdown
Member Author

My understanding is that LLVM will very effectively inline those -- just like how we don't special case the array iterator for N ≤ 1 and still use the polymorphic version.

Does make me ponder if we should have a separate general "hey, there's no Drop so we can delegate to the one for the one field" kind of check, but I'd leave all these for a different PR if that's ok.

@scottmcm scottmcm force-pushed the intercept-array-drop-shim branch from d1bdee8 to 2ae092a Compare April 15, 2026 01:23
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` PG-exploit-mitigations Project group: Exploit mitigations labels Apr 15, 2026
@scottmcm scottmcm removed PG-exploit-mitigations Project group: Exploit mitigations A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` labels Apr 15, 2026
@rust-bors

This comment has been minimized.

@scottmcm scottmcm added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Apr 28, 2026
@scottmcm scottmcm force-pushed the intercept-array-drop-shim branch 2 times, most recently from 8a17db9 to bf354cd Compare May 7, 2026 03:22
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label May 7, 2026
@rustbot rustbot added A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` PG-exploit-mitigations Project group: Exploit mitigations labels May 7, 2026
@scottmcm scottmcm removed PG-exploit-mitigations Project group: Exploit mitigations A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` labels May 7, 2026
@scottmcm scottmcm force-pushed the intercept-array-drop-shim branch from bf354cd to 1f28062 Compare May 7, 2026 04:06
@scottmcm scottmcm removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label May 7, 2026
@scottmcm scottmcm marked this pull request as ready for review May 7, 2026 04:14
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 7, 2026

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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 May 7, 2026
@scottmcm
Copy link
Copy Markdown
Member Author

scottmcm commented May 7, 2026

@bors try @rust-timer queue

@scottmcm
Copy link
Copy Markdown
Member Author

scottmcm commented May 7, 2026

does it make sense to keep the array case separate for N <= 1 or something?

Oh, also for N == 0 it's detected as !needs_drop, so it gets a fully empty body.

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request May 7, 2026
 Have arrays' `drop_glue` just unsize and call the slice version
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 7, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 7, 2026

☀️ Try build successful (CI)
Build commit: 69592c2 (69592c2a37914f5bee8cabac44cc5f28707bd578, parent: 4ddd4538a881317c622ed674b08300b8fc8dabdd)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (69592c2): 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
@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.1% [0.1%, 0.1%] 2
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
-0.2% [-0.4%, -0.0%] 5
All ❌✅ (primary) -0.4% [-0.4%, -0.4%] 1

Max RSS (memory usage)

Results (primary -1.3%, secondary -0.6%)

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

mean range count
Regressions ❌
(primary)
2.0% [2.0%, 2.0%] 1
Regressions ❌
(secondary)
1.5% [0.5%, 2.4%] 6
Improvements ✅
(primary)
-2.9% [-4.0%, -1.9%] 2
Improvements ✅
(secondary)
-2.2% [-3.9%, -0.9%] 8
All ❌✅ (primary) -1.3% [-4.0%, 2.0%] 3

Cycles

Results (primary -2.3%, 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)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
1.3% [0.6%, 4.6%] 7
Improvements ✅
(primary)
-7.7% [-7.7%, -7.7%] 1
Improvements ✅
(secondary)
-0.5% [-0.5%, -0.5%] 1
All ❌✅ (primary) -2.3% [-7.7%, 3.1%] 2

Binary size

Results (primary 0.3%)

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

mean range count
Regressions ❌
(primary)
0.6% [0.2%, 1.1%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.6% [-0.6%, -0.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [-0.6%, 1.1%] 4

Bootstrap: 499.861s -> 496.397s (-0.69%)
Artifact size: 394.98 MiB -> 397.13 MiB (0.54%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I-heavy Issue: Problems and improvements with respect to binary size of generated code. perf-regression Performance regression. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants