Skip to content

Add if len > CAP { unreachable_unchecked() }#314

Open
barakugav wants to merge 2 commits into
bluss:masterfrom
barakugav:assume-len
Open

Add if len > CAP { unreachable_unchecked() }#314
barakugav wants to merge 2 commits into
bluss:masterfrom
barakugav:assume-len

Conversation

@barakugav

@barakugav barakugav commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

First of all, love the library! use it all the time 😄

This PR add a if len > CAP { unreachable_unchecked() } call in ArrayVec::len().
We know this assumption, and telling the compiler about it let it generate better code and eliminate some bound checks.

It also include a small benchmark for ArrayVec::clone() that demonstrate the (massive) speed up.

# before
> cargo bench clone
running 2 tests
test clone_512 ... bench:         345 ns/iter (+/- 20) = 1484 MB/s
test clone_8   ... bench:           4 ns/iter (+/- 0) = 2000 MB/s

# after
> cargo bench clone
running 2 tests
test clone_512 ... bench:         205 ns/iter (+/- 15) = 2497 MB/s
test clone_8   ... bench:           2 ns/iter (+/- 0) = 4000 MB/s

Clone is my specific use case, but im sure other generated code is more optimized when the compiler knows this.

Thanks for the support in advance ❤️

Comment thread build.rs Outdated
Comment on lines +14 to +29
fn rustc_version() -> Option<u32> {
// Code copied from cxx crate

let rustc = std::env::var_os("RUSTC")?;
let output = std::process::Command::new(rustc)
.arg("--version")
.output()
.ok()?;
let version = String::from_utf8(output.stdout).ok()?;
let mut pieces = version.split('.');
if pieces.next() != Some("rustc 1") {
return None;
}
let minor = pieces.next()?.parse().ok()?;
Some(minor)
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Two big questions for the maintainer here: should we add a build script? Going from no build script to having one is a step back.

Second, reviewing the function: this function rustc_version must be removed. Use a maintained crate to do this - preferably autocfg - don't copy/code your own version check. The reason is long-term maintainability.

Suggested resolution: Wait until the MSRV allows this without trouble. Publish a new 0.8.x soon with updated MSRV (and a slowly sliding MSRV policy), use this without build.rs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i found a workaround that doesn't require build.rs, tell me what you think

@barakugav

Copy link
Copy Markdown
Contributor Author

@bluss I replaced hint::assert_unchecked(len <= CAP) with if len > CAP { hint::unreachable_unchecked() }, which is stable in 1.51, and got the same speedup/asm. build.rs removed.

@barakugav barakugav changed the title Add hint::assert_unchecked(len <= CAP) Add if len > CAP { unreachable_unchecked() } Jul 9, 2026
@barakugav

barakugav commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

oh, unreachable_unchecked is stable since 1.27 but as const fn only from 1.57... at least is lower than assert_unchecked'a 1.81 requirement.
This can wait until we bump msrv to 1.57, or i can revert to the build.rs solution. Either way, i think this optimization can have a large impact for all users, so please consider it 🙏

@bluss

bluss commented Jul 9, 2026

Copy link
Copy Markdown
Owner

We're fixing clone in the other PR, so we need one more example for when this helps in this case. I thought "codegen tests" are a good way to look at this, so if it's possible to make one like for clone it would be good.

Alt solution, add .len_dynamic() which is non-const and use it in critical places in the crate. It would be internal, so not in the API, so only help internal methods.

@barakugav

barakugav commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

for the following simple example:

#[inline(never)]
fn do_work(array: &ArrayVec<u8, 1>) -> u8 {
    let mut sum: u8 = 0;
    for i in 0..array.len() {
        sum = sum.wrapping_add(array[i]);
    }
    sum
}

asm on my machine after this PR:

hello::do_work:
Lfunc_begin5:
        .cfi_startproc
        cmp w0, #0
        csel w0, wzr, w1, eq
        ret

asm on my machine before this PR:

hello::do_work:
Lfunc_begin5:
        .cfi_startproc
        ldr w9, [x0]
        cbz w9, LBB5_3
        cmp w9, #7
        b.hi LBB5_4
        mov w8, #0
        mov x10, #0
        b LBB5_13
LBB5_3:
        mov w8, #0
        mov x0, x8
        ret
LBB5_4:
        cmp w9, #64
        b.hs LBB5_6
        mov x10, #0
        mov w8, #0
        b LBB5_10
LBB5_6:
        and x11, x9, #0x38
        and x10, x9, #0xffffffc0
        add x8, x0, #36
        movi.2d v0, #0000000000000000
        and x12, x9, #0xffffffc0
        movi.2d v1, #0000000000000000
        movi.2d v2, #0000000000000000
        movi.2d v3, #0000000000000000
LBB5_7:
        ldp q4, q5, [x8, #-32]
        ldp q6, q7, [x8], #64
        add.16b v0, v4, v0
        add.16b v1, v5, v1
        add.16b v2, v6, v2
        add.16b v3, v7, v3
        subs x12, x12, #64
        b.ne LBB5_7
        add.16b v0, v1, v0
        add.16b v0, v2, v0
        add.16b v0, v3, v0
        addv.16b b0, v0
        fmov w8, s0
        cmp x10, x9
        b.eq LBB5_15
        cbz x11, LBB5_13
LBB5_10:
        mov x11, x10
        and x10, x9, #0xfffffff8
        movi.2d v0, #0000000000000000
        mov.b v0[0], w8
        sub x8, x11, x10
        add x11, x11, x0
        add x11, x11, #4
LBB5_11:
        ldr d1, [x11], #8
        add.8b v0, v1, v0
        adds x8, x8, #8
        b.ne LBB5_11
        addv.8b b0, v0
        fmov w8, s0
        cmp x10, x9
        b.eq LBB5_15
LBB5_13:
        add x11, x10, x0
        add x11, x11, #4
        sub x9, x9, x10
LBB5_14:
        ldrb w10, [x11], #1
        add w8, w10, w8
        subs x9, x9, #1
        b.ne LBB5_14
LBB5_15:
        mov x0, x8
        ret

I added a sum benchmark. The speed up is mostly seen in small arrays, as for many accesses the branch predictor does a good job eliminating the cost of the check.

# before this PR
> cargo bench sum
running 3 tests
test sum_1   ... bench:          78 ns/iter (+/- 3) = 12 MB/s
test sum_128 ... bench:         144 ns/iter (+/- 2) = 888 MB/s
test sum_16  ... bench:         114 ns/iter (+/- 2) = 140 MB/s

# after this PR
> cargo bench sum
running 3 tests
test sum_1   ... bench:          44 ns/iter (+/- 2) = 22 MB/s
test sum_128 ... bench:         128 ns/iter (+/- 10) = 1000 MB/s
test sum_16  ... bench:          87 ns/iter (+/- 4) = 183 MB/s

Personally i think adding .len_dynamic() make the code a bit clumsy, although its a reasonable solution and we will see the benefits. For example we can call it in Deref and every access using [] will be improved.
I would wait for a msrv, but the decision is yours, tell me what you think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants