-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assembly for Rc::<str>::default() looks quite inefficient #135784
Labels
C-optimization
Category: An issue highlighting optimization opportunities or PRs implementing such
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
pub fn rc_str_from_bytes() -> Rc<str> {
let rc = Rc::<[u8]>::default();
unsafe { Rc::from_raw(Rc::into_raw(rc) as *const [u8] as *const str) }
} It produces only one call (for the non-OOM case) instead of two: https://godbolt.org/z/aMGr4eqxT example::rc_str_from_bytes::hba4560ddf8c19a90:
push rax
mov rax, qword ptr [rip + __rust_no_alloc_shim_is_unstable@GOTPCREL]
movzx eax, byte ptr [rax]
mov edi, 16
mov esi, 8
call qword ptr [rip + __rust_alloc@GOTPCREL]
test rax, rax
je .LBB1_2
mov qword ptr [rax], 1
mov qword ptr [rax + 8], 1
xor edx, edx
pop rcx
ret
.LBB1_2:
mov edi, 8
mov esi, 16
call qword ptr [rip + alloc::alloc::handle_alloc_error::ha0547c441587f574@GOTPCREL] |
Kijewski
added a commit
to Kijewski/rust
that referenced
this issue
Jan 26, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. Resolves <rust-lang#135784>.
Kijewski
added a commit
to Kijewski/rust
that referenced
this issue
Jan 26, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Feb 8, 2025
…heemdev Optimize `Rc::<str>::default()` implementation This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>. Cc `@Billy-Sheppard.`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Feb 8, 2025
…heemdev Optimize `Rc::<str>::default()` implementation This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>. Cc ``@Billy-Sheppard.``
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Feb 8, 2025
…heemdev Optimize `Rc::<str>::default()` implementation This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>. Cc `@Billy-Sheppard.`
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 8, 2025
Rollup merge of rust-lang#136099 - Kijewski:pr-rc-str-default, r=ibraheemdev Optimize `Rc::<str>::default()` implementation This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>. Cc `@Billy-Sheppard.`
github-actions bot
pushed a commit
to tautschnig/verify-rust-std
that referenced
this issue
Feb 20, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to tautschnig/verify-rust-std
that referenced
this issue
Feb 20, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to carolynzech/rust
that referenced
this issue
Feb 20, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to model-checking/verify-rust-std
that referenced
this issue
Feb 20, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Feb 21, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Feb 21, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to model-checking/verify-rust-std
that referenced
this issue
Feb 22, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to carolynzech/rust
that referenced
this issue
Feb 22, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to tautschnig/verify-rust-std
that referenced
this issue
Feb 22, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Feb 22, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Mar 3, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to carolynzech/rust
that referenced
this issue
Mar 4, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to carolynzech/rust
that referenced
this issue
Mar 4, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Mar 4, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to tautschnig/verify-rust-std
that referenced
this issue
Mar 6, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
github-actions bot
pushed a commit
to thanhnguyen-aws/verify-rust-std
that referenced
this issue
Mar 6, 2025
This PR lets `impl Default for Rc<str>` re-use the implementation for `Rc::<[u8]>::default()`. The previous version only calculted the memory layout at runtime, even though it should be known at compile time, resulting in an additional function call. The same optimization is done for `Rc<CStr>`. Generated byte code: <https://godbolt.org/z/dfq73jsoP>. Resolves <rust-lang#135784>.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
C-optimization
Category: An issue highlighting optimization opportunities or PRs implementing such
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
I tried this code:
I expected this to allocate with a known layout and and handle potential allocation errors.
Instead, before the allocation, the assembly contains not one, but two calls to
rc_inner_layout_for_value_layout
(indirect viacall r15
) (compiler explorer):The two calls seem to correspond to the source code (line 257 and 2076), but I would have expected the compiler to be smart enough to see those should be the same, and even to compute that value at compile time.
Meta
rustc --version --verbose
:The text was updated successfully, but these errors were encountered: