rust-lang / rust-lang/rust

Performance regression with `get_many_mut()`

Open
#128,214 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-optimization I-heavy I-slow P-medium perf-regression regression-from-stable-to-stable
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

In the process of trying to stabilize get_many_mut(), and while trying to check what will be the costs of a more detailed error type, I came across the following perf regression from stable to beta.

Code

I tried this code:

#![feature(get_many_mut)]

#[inline]
fn get_many_check_valid(indices: &[usize; 2], len: usize) -> Result<(), GetManyMutError> {
    for (i, &idx) in indices.iter().enumerate() {
        if idx >= len {
            return Err(GetManyMutError::OutOfBounds);
        }
        for &idx2 in &indices[..i] {
            if idx == idx2 {
                return Err(GetManyMutError::Overlapping);
            }
        }
    }
    Ok(())
}

#[inline]
pub fn after_all<'a>(
    slice: &'a mut [i32],
    indices: [usize; 2],
) -> Result<[&mut i32; 2], GetManyMutError> {
    get_many_check_valid(&indices, slice.len())?;
    unsafe { Ok(slice.get_many_unchecked_mut(indices)) }
}

#[derive(Debug)]
pub enum GetManyMutError {
    OutOfBounds,
    Overlapping,
}

#[no_mangle]
pub fn after_all_unwrap(data: &mut [i32], indices: [usize; 2]) -> [&mut i32; 2] {
    after_all(data, indices).unwrap()
}

On stable (with RUSTC_BOOTSTRAP=1) this optimizes nicely to the following code (https://godbolt.org/z/zq8EG7df7):

after_all_unwrap:
        mov     r8, qword ptr [rcx]
        xor     eax, eax
        cmp     r8, rdx
        jae     .LBB0_4
        mov     rcx, qword ptr [rcx + 8]
        cmp     rcx, rdx
        jae     .LBB0_4
        cmp     rcx, r8
        je      .LBB0_3
        lea     rax, [rsi + 4*r8]
        lea     rcx, [rsi + 4*rcx]
        mov     qword ptr [rdi], rax
        mov     qword ptr [rdi + 8], rcx
        mov     rax, rdi
        ret
.LBB0_3:
        mov     al, 1
.LBB0_4:
        push    rax
        mov     byte ptr [rsp + 7], al
        lea     rdi, [rip + .L__unnamed_1]
        lea     rcx, [rip + .L__unnamed_2]
        lea     r8, [rip + .L__unnamed_3]
        lea     rdx, [rsp + 7]
        mov     esi, 43
        call    qword ptr [rip + core::result::unwrap_failed::haccb9aaa604e1e21@GOTPCREL]

However, on beta (and nightly) this adds a whole bunch of unnecessary instructions:

after_all_unwrap:
        mov     r8, qword ptr [rcx]
        xor     eax, eax
        cmp     r8, rdx
        jae     .LBB0_4
        mov     rcx, qword ptr [rcx + 8]
        cmp     rcx, rdx
        jae     .LBB0_4
        cmp     rcx, r8
        je      .LBB0_3
        lea     rax, [rsi + 4*r8]
        lea     rcx, [rsi + 4*rcx]
        mov     rdx, rcx
        shr     rdx, 8
        mov     qword ptr [rdi], rax
        mov     byte ptr [rdi + 8], cl
        mov     rax, rcx
        shr     rax, 56
        mov     byte ptr [rdi + 15], al
        shr     rcx, 40
        mov     word ptr [rdi + 13], cx
        mov     dword ptr [rdi + 9], edx
        mov     rax, rdi
        ret
.LBB0_3:
        mov     al, 1
.LBB0_4:
        push    rax
        mov     byte ptr [rsp + 7], al
        lea     rdi, [rip + .L__unnamed_1]
        lea     rcx, [rip + .L__unnamed_2]
        lea     r8, [rip + .L__unnamed_3]
        lea     rdx, [rsp + 7]
        mov     esi, 43
        call    qword ptr [rip + core::result::unwrap_failed::h803c538fb296a320@GOTPCREL]

With cargo-bisect-rustc I bisected it to https://github.com/rust-lang/rust/pull/126578. CC @scottmcm.

@rustbot label +regression-from-stable-to-beta -regression-untriaged -C-bug +perf-regression +I-heavy +I-slow

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the get_many_mut() example and compare the stable, beta, and nightly assembly linked in the report. Review the regression bisect pointing to rust-lang/rust#126578 and investigate the affected compiler behavior. Done means the unnecessary result-layout instructions no longer appear in the generated after_all_unwrap code.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.