Async performance regression

[goffrie]

See #69033 (comment); @kpp reports a 200%+ regression on some futures microbenchmarks, e.g.:

future::ready/async_combinators                                                                             
                        time:   [1.5699 ns 1.5743 ns 1.5793 ns]
                        change: [+279.32% +282.34% +285.20%] (p = 0.00 < 0.05)
                        Performance has regressed.

The benchmarks are at https://github.com/kpp/futures-async-combinators/.

I bisected this to 57e1da5, which most likely implicates #69837.

[jonas-schievink]

Can reproduce. The regression comes from changing the discriminant type from u32 to u8 (or whatever fits best, which almost always is u8), exposing the niche does not regress performance much.

Isn't x86 generally less efficient when dealing with <32 bit words? In that case we might want to consider doing this optimization conditionally, since it can be a somewhat significant RAM saving on smaller microcontroller targets. I could imagine using -C opt-level=s to decide this, but I don't think there's any precedence for doing something like that at the moment.

[Mark-Simulacrum]

I think the slowdown is likely some sort of bug in our codegen or LLVM perhaps?

This is a single iteration on nightly (unrolled 8 times in the actual benchmark code):

    0.00 :   18a260:       mov    BYTE PTR [rsp+0x8],0x0
    0.80 :   18a265:       movzx  ecx,BYTE PTR [rsp]
    0.99 :   18a269:       movzx  ecx,BYTE PTR [rsp+0x1]
    1.09 :   18a26e:       movzx  ecx,BYTE PTR [rsp+0x2]
    0.87 :   18a273:       movzx  ecx,BYTE PTR [rsp+0x3]
    0.97 :   18a278:       movzx  ecx,BYTE PTR [rsp+0x4]
    0.83 :   18a27d:       movzx  ecx,BYTE PTR [rsp+0x5]
    1.32 :   18a282:       movzx  ecx,BYTE PTR [rsp+0x6]
    0.87 :   18a287:       movzx  ecx,BYTE PTR [rsp+0x7]
    0.84 :   18a28c:       movzx  ecx,BYTE PTR [rsp+0x9]
    0.86 :   18a291:       movzx  ecx,BYTE PTR [rsp+0xa]
    1.01 :   18a296:       movzx  ecx,BYTE PTR [rsp+0xb]
    1.04 :   18a29b:       movzx  ecx,BYTE PTR [rsp+0x8]

On the good commit, it looks like this (and is only unrolled 4 times):

   17.76 :   183f62:       mov    DWORD PTR [rsp],0x0
    0.00 :   183f69:       mov    edx,DWORD PTR [rsp+0x4]
    3.26 :   183f6d:       mov    edx,DWORD PTR [rsp+0x8]
    0.00 :   183f71:       mov    edx,DWORD PTR [rsp]

Trying to reduce this down, I came up with this example:

use std::future::Future;

pub async fn foo(v: i32) -> i32 { v }

macro_rules! wrap {
    ($v:expr) => {
        async move { ($v).await }
    }
}

pub fn bar(v: i32) -> impl Future<Output = i32> {
    wrap!(wrap!(wrap!(wrap!(wrap!(wrap!(foo(v)))))))
}

In godbolt, the nightly has an offset of 52 -- and that increases by 8 each additional wrap! -- whereas stable stays constant at 4, the size of i32 (presumably). I think that's the bug here, though I'm not sure how the discriminant size managed to affect that. Maybe previously we were getting split into two separate LLVM "words" via SROA or something and now we aren't since we fit into a single register or something like that? That could I guess plausibly lead to problems here; my assumption is that the benchmark is loading many more words from the stack on nightly because the "size" of the future has increased drastically? OTOH, size_of_val etc do not report a size increase -- so why the offset is increasing I'm not sure. But I suspect it's for the same reason we're seeing more stack loads.

Nightly:

example::foo:
        mov     eax, edi
        ret

example::bar:
        mov     rax, rdi
        mov     dword ptr [rdi], esi
        mov     byte ptr [rdi + 52], 0
        ret

and on stable:

example::foo:
        mov     eax, edi
        ret

example::bar:
        mov     rax, rdi
        mov     dword ptr [rdi], esi
        mov     dword ptr [rdi + 4], 0
        ret

[jonas-schievink]

Thanks, that's helpful!

[jonas-schievink]

The dword vs. byte moves can be explained by the alignment changing from 4 (due to using u32 as the discriminant) to 1 (since we now use u8 as the discriminant), as long as nothing with a larger alignment is contained.

[jonas-schievink]

In godbolt, the nightly has an offset of 52 -- and that increases by 8 each additional wrap! -- whereas stable stays constant at 4, the size of i32 (presumably).

Not entirely certain, but I think this is just automatic field reordering doing its thing. The discriminant is now a u8 with alignment 1, so it gets moved to the very end of the memory layout to not waste space with padding. Previously it was a u32 with alignment 4, just like all the other fields in the generator, so reordering anything there doesn't make sense.

If I take your example and replace i32 with bool, the size of foo goes from 8 to 2 bytes (and its alignment from 4 to 1), while bar goes from 56 to 14 bytes (its alignment also goes from 4 to 1).

So far this looks like a fairly expected outcome of my PR; it's pretty well-known that smaller alignment can generate less efficient code (OTOH apparently modern x86 chips can do unaligned moves just as fast as aligned ones, so maybe the byte-wise moves should not be emitted, but in the end this is completely up to LLVM). Since, AFAICT, it should only affect futures that do not store any data with alignment in them, the impact should be pretty minimal in practice, so we can also consider closing this without a fix.

(this does make me wish that we had better layout debugging capabilities in rustc though)

[memoryruins]

this does make me wish that we had better layout debugging capabilities in rustc

A #[rustc_layout(debug)] attribute was added in #69901 (edit: heh, just noticed you're the first comment on it ^^). With the help of type_alias_impl_trait, rustc output some layout information for bar's future: playground

[jonas-schievink]

Ah, nice to see that land, that's certainly a step in the right direction

[nagisa]

@jonas-schievink a quick test to see if this is indeed an alignment problem is to stick a #[repr(align(4))]-equivalent onto the generator.