Avoid unnecessary Vec resize.

[ttsugriy]

If size > 0 current implementation will first create an empty vec and then push an element into it, which will cause a resize that can be easily avoided.

It's obviously not a big deal, but this also gets rid of mut local variable.

[RalfJung]

Vec::new is completely free, so the old code isn't resizing either. It is just creating a new allocation in the size > 0 case.

I like avoiding the mut, but this expression is getting a bit big. Can you let-bind the initial vector?

[ttsugriy]

Vec::new is completely free, so the old code isn't resizing either. It is just creating a new allocation in the size > 0 case.

that's not entirely true and instead of arguing I'll just drop the benchmark results below:

     Running benches/new_vec_benchmark.rs (target/release/deps/new_vec_benchmark-8ca517b46c5e51d3)
singleton ve/push       time:   [37.803 ns 38.432 ns 39.277 ns]
singleton ve/vec!       time:   [32.089 ns 32.846 ns 33.658 ns]

I like avoiding the mut, but this expression is getting a bit big. Can you let-bind the initial vector?

sorry, I'm not sure I understand. Are you suggesting something like

let v = if size > 0 { vec![Elem { range: 0..size, data: init }] } else { Vec::new() };
RangeMap { v }

?

[RalfJung]

that's not entirely true and instead of arguing I'll just drop the benchmark results below:

Hm, interesting. I'm a bit worried that the allocations are optimized away entirely, can you drop(hint::black_box(v)) to make sure that does not happen? Or does criterion do that automatically with the return value?

sorry, I'm not sure I understand. Are you suggesting something like

Yes.

[ttsugriy]

that's not entirely true and instead of arguing I'll just drop the benchmark results below:

Hm, interesting. I'm a bit worried that the allocations are optimized away entirely, can you drop(hint::black_box(v)) to make sure that does not happen? Or does criterion do that automatically with the return value?

criterion does it automatically but it's even easier to understand the perf impact by looking at assembly for both versions side by side - https://godbolt.org/z/GoT4Kes5b

sorry, I'm not sure I understand. Are you suggesting something like

Yes.

I don't have a preference, so I've updated this PR with your suggestion :)

[RalfJung]

Thanks! I highly doubt this will even be measurable in any Miri benchmark, but I like getting rid of the mut. :)

@bors r+

[bors]

📌 Commit 6cd057e has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6cd057e with merge 4810142...

[ttsugriy]

Thanks! I highly doubt this will even be measurable in any Miri benchmark, but I like getting rid of the mut. :)

You are very welcome! I'm sure this change won't affect Miri's perf on the global scale, but since most of the code in the world is written by copy-pasting it's best to have more "better" examples :D

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 4810142 to master...