Add if len > CAP { unreachable_unchecked() }#314
Conversation
| 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) | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
i found a workaround that doesn't require build.rs, tell me what you think
|
@bluss I replaced |
hint::assert_unchecked(len <= CAP)if len > CAP { unreachable_unchecked() }
|
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. |
|
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 |
|
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
retasm 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
retI added a # 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/sPersonally i think adding |
First of all, love the library! use it all the time 😄
This PR add a
if len > CAP { unreachable_unchecked() }call inArrayVec::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.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 ❤️