Stop emitting drop loops for every array length by scottmcm · Pull Request #134297 · rust-lang/rust

scottmcm · 2024-12-14T04:25:40Z

We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length.

As part of this (and how I originally noticed this), use PtrMetadata instead of Len for the slice length, for another step closer to being able to remove Rvalue::Len.

Demonstration that yes, every slice length gets its own loop today: https://rust.godbolt.org/z/5EsPjPWv4

rustbot · 2024-12-14T04:25:48Z

r? @BoxyUwU

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

scottmcm · 2024-12-14T04:36:29Z

@bors try @rust-timer queue

rust-timer · 2024-12-14T04:36:32Z

Awaiting bors try build completion.

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

…<try> Stop emitting drop loops for every array length We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length. As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`. Demonstration that yes, every slice length gets its own loop today: <https://rust.godbolt.org/z/5EsPjPWv4>

bors · 2024-12-14T04:37:40Z

⌛ Trying commit f6cd47a with merge 41701b3...

bors · 2024-12-14T05:02:07Z

💔 Test failed - checks-actions

We can just unsize the array to a slice and drop that instead, reusing the same loop. As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`.

scottmcm · 2024-12-14T07:39:24Z

@bors try @rust-timer queue

rust-timer · 2024-12-14T07:39:27Z

Awaiting bors try build completion.

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

bors · 2024-12-14T07:40:35Z

⌛ Trying commit aac57cf with merge 4ce3fa5...

…<try> Stop emitting drop loops for every array length We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length. As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`. Demonstration that yes, every slice length gets its own loop today: <https://rust.godbolt.org/z/5EsPjPWv4>

bors · 2024-12-14T08:04:47Z

💔 Test failed - checks-actions

lqd · 2024-12-14T10:43:41Z

(These failures within opt-dist running rustc-perf aren't super clear... I'll try to see how to improve that.)

rust-log-analyzer · 2024-12-15T04:22:08Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling mdbook-trpl v0.1.0 (/checkout/src/doc/book/packages/mdbook-trpl)
   Compiling rustbook v0.1.0 (/checkout/src/tools/rustbook)
    Finished `release` profile [optimized] target(s) in 25.92s
##[endgroup]
error: process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc /checkout/obj/build/bootstrap/debug/rustc -vV` (exit status: 101)
thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
Unable to install ctrlc handler: System(Custom { kind: Other, error: EBADF })
   0:     0x7f5569025131 - <<std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print::DisplayBacktrace as core[390652ced2a3c436]::fmt::Display>::fmt
   1:     0x7f55690840b3 - core[390652ced2a3c436]::fmt::write
   2:     0x7f5569018009 - <std[dfe8e69f7cd2cb9c]::sys::pal::unix::stdio::Stderr as std[dfe8e69f7cd2cb9c]::io::Write>::write_fmt
   3:     0x7f5569024fd2 - <std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print
   3:     0x7f5569024fd2 - <std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print
   4:     0x7f5569027a31 - std[dfe8e69f7cd2cb9c]::panicking::default_hook::{closure#1}
   5:     0x7f55690277af - std[dfe8e69f7cd2cb9c]::panicking::default_hook
   6:     0x7f5564773518 - <alloc[c0599e64d5b5f875]::boxed::Box<rustc_driver_impl[efef9de6942c42c2]::install_ice_hook::{closure#0}> as core[390652ced2a3c436]::ops::function::Fn<(&dyn for<'a, 'b> core[390652ced2a3c436]::ops::function::Fn<(&'a std[dfe8e69f7cd2cb9c]::panic::PanicHookInfo<'b>,), Output = ()> + core[390652ced2a3c436]::marker::Sync + core[390652ced2a3c436]::marker::Send, &std[dfe8e69f7cd2cb9c]::panic::PanicHookInfo)>>::call
   7:     0x7f5569028448 - std[dfe8e69f7cd2cb9c]::panicking::rust_panic_with_hook
   8:     0x7f556902803e - std[dfe8e69f7cd2cb9c]::panicking::begin_panic_handler::{closure#0}
   9:     0x7f5569025759 - std[dfe8e69f7cd2cb9c]::sys::backtrace::__rust_end_short_backtrace::<std[dfe8e69f7cd2cb9c]::panicking::begin_panic_handler::{closure#0}, !>
  11:     0x7f556907fb00 - core[390652ced2a3c436]::panicking::panic_fmt
  12:     0x7f55690800f6 - core[390652ced2a3c436]::result::unwrap_failed
  13:     0x7f55646a1995 - rustc_driver_impl[efef9de6942c42c2]::main
  14:     0x55a786bda8a7 - rustc_main[22b90118f880c90b]::main
---
warning: the ICE couldn't be written to `/checkout/rustc-ice-2024-12-15T04_21_58-26907.txt`: Read-only file system (os error 30)

note: rustc 1.85.0-nightly (dd429d1e6 2024-12-15) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z on-broken-pipe=kill -Z unstable-options
query stack during panic:
end of query stack

Build completed unsuccessfully in 0:00:30
---
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
WARN: currently no CI rustc builds have rustc debug assertions enabled. Please either set `rust.debug-assertions` to `false` if you want to use download CI rustc or set `rust.download-rustc` to `false`.
ERROR: Tool `book` was not recorded in tool state.
ERROR: Tool `nomicon` was not recorded in tool state.
ERROR: Tool `reference` was not recorded in tool state.
ERROR: Tool `rust-by-example` was not recorded in tool state.
ERROR: Tool `edition-guide` was not recorded in tool state.
ERROR: Tool `embedded-book` was not recorded in tool state.
  local time: Sun Dec 15 04:21:58 UTC 2024
  network time: Sun, 15 Dec 2024 04:21:59 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

scottmcm · 2024-12-15T04:39:39Z

I have no idea what's happening here :/

Replaced with #134326

lqd · 2024-12-15T17:53:24Z

@scottmcm The collector failed with collector error: Cannot read lib dir to find components (the error handling is buggy there, I'll fix it in rust-lang/rustc-perf#2021). It failed because it was trying to read the "lib" directory in the sysroot but couldn't: stage2 rustc panicked when asked to print the sysroot:

collector error: Cannot find libdir for rustc

Caused by:
    rustc failed to provide sysroot, error: exit status: 101, stderr:
    thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
    Unable to install ctrlc handler: System(Custom { kind: Other, error: EBADF })
    stack backtrace:
       0: rust_begin_unwind
       1: core::panicking::panic_fmt
       2: core::result::unwrap_failed
       3: rustc_driver_impl::install_ctrlc_handler
       4: rustc_driver_impl::main
       5: rustc_main::main
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

    error: the compiler unexpectedly panicked. this is a bug.

    note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

    note: please make sure that you have updated to the latest nightly

    note: please attach the file at `/tmp/tmp-multistage/opt-artifacts/rustc-perf/rustc-ice-2024-12-15T17_44_07-54539.txt` to your bug report

    query stack during panic:
    end of query stack

(I've launched a try build with the other PR, to see if these errors reproduce)

…, r=<try> Use `PtrMetadata` instead of `Len` in slice drop shims I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`. Reusing the same reviewer from the last one: r? BoxyUwU

scottmcm · 2024-12-16T03:57:09Z

Thanks for digging in, @lqd ! I think I'm not monoing enough here, or something :/

lqd · 2024-12-16T06:06:21Z

The good news is this behavior doesn’t seem to reproduce in the #134326 subset 🎉

…, r=saethlin Use `PtrMetadata` instead of `Len` in slice drop shims I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`. Split into two commits where the first just adds a test, so you can look at the second commit to see how the drop shim for an array changes with this PR. Reusing the same reviewer from the last one: r? BoxyUwU

rustbot assigned BoxyUwU Dec 14, 2024

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 Dec 14, 2024

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

This comment has been minimized.

Sign in to view

bors 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 Dec 14, 2024

scottmcm force-pushed the drop-arrays-by-unsizing branch from f6cd47a to aac57cf Compare December 14, 2024 07:03

This comment has been minimized.

Sign in to view

Delegate via a Call instead

a5f9505

scottmcm force-pushed the drop-arrays-by-unsizing branch from f1c87ea to a5f9505 Compare December 15, 2024 04:03

scottmcm closed this Dec 15, 2024

scottmcm deleted the drop-arrays-by-unsizing branch December 15, 2024 05:43

scottmcm mentioned this pull request Dec 15, 2024

Use PtrMetadata instead of Len in slice drop shims #134326

Merged

lqd mentioned this pull request Dec 15, 2024

Fix error handling when looking for toolchain components rust-lang/rustc-perf#2021

Merged

Uh oh!

Conversation

scottmcm commented Dec 14, 2024

Uh oh!

rustbot commented Dec 14, 2024

Uh oh!

scottmcm commented Dec 14, 2024

Uh oh!

rust-timer commented Dec 14, 2024

Uh oh!

bors commented Dec 14, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Dec 14, 2024

Uh oh!

This comment has been minimized.

scottmcm commented Dec 14, 2024

Uh oh!

rust-timer commented Dec 14, 2024

Uh oh!

bors commented Dec 14, 2024

Uh oh!

This comment has been minimized.

bors commented Dec 14, 2024

Uh oh!

lqd commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rust-log-analyzer commented Dec 15, 2024

Uh oh!

scottmcm commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lqd commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottmcm commented Dec 16, 2024

Uh oh!

lqd commented Dec 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lqd commented Dec 14, 2024 •

edited

Loading

scottmcm commented Dec 15, 2024 •

edited

Loading

lqd commented Dec 15, 2024 •

edited

Loading