New article: VecDeque::resize() optimization

[Folyd]

No description provided.

[ZhangHanDong]

lgtm

[Folyd]

Hi, @scottmcm, would you mind reviewing my article? Thanks. ❤️

[XeCycle]

Came over this and I'm curious, does repeat_n need an unsafe block at all? Feeling that Option<(NonZeroUsize, T)> may be able to do the same. I looked at the original PR briefly but it did not explain the rationales.

[Folyd]

Came over this and I'm curious, does repeat_n need an unsafe block at all? Feeling that Option<(NonZeroUsize, T)> may be able to do the same. I looked at the original PR briefly but it did not explain the rationales.

Hi, can you explain how Option<(NonZeroUsize, T)> would avoid unnecessary cloning?

[XeCycle]

can you explain how Option<(NonZeroUnsize, T)> would avoid unnecessary cloning?

Created running example at https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fd82d81d0f485acd769c3b0aaeeb5a3b

[XeCycle]

Another version of that next() without unwrap():

    fn next(&mut self) -> Option<Self::Item> {
        let inner = self.0.as_mut()?;
        if let Some(next) = NonZeroUsize::new(inner.0.get() - 1) {
            inner.0 = next;
            Some(inner.1.clone())
        } else {
            Some(self.0.take()?.1)
        }
    }

Godbolt says this can be optimized to 2 branches, which I assume corresponds to 2 special cases for count == 0 and count == 1, i.e. second ? can be elided.

[scottmcm]

Feeling that Option<(NonZeroUsize, T)> may be able to do the same.

Good question! I actually tried that first. I wasn't able to get it to optimize as nicely, though.

I think it might be because that's essentially (usize, MaybeUninit<T>) -- because None doesn't have a valid value of T -- rather than the (usize, ManuallyDrop<T>) I ended up using. That affects whether it can have a niche, for example (see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5e8a9b728df0f55dec1975bfad1e4902) and might have some impact on when LLVM can treat the reads as the same, rather than worrying about potential undef.

Definitely give it a shot and see if you can make it work, though! I'd love to get rid of all the unsafe, though thankfully ManuallyDrop's unsafety is one of the easier kinds to deal with.

[scottmcm]

Look reasonable to me!

One thing you might consider adding is a bit more about why not cloning is useful. Probably doesn't need to be a huge section, but it might be worth a mention, or an example in which you can tell between different rust versions that something different is happening.

(For extra credit, consider when you might want the buffer to be reused and what code you're write to do that instead.)

[XeCycle]

@scottmcm your playground link tests for Option<Option(NonZeroUsize, String)>>, but we don't need 2 layers of Option. Just Option<(NonZeroUsize, String)> and it will fill the niche: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e3402f532fa99e63a2a951605e9b1c38

[scottmcm]

@XeCycle I did that on purpose to show the niche behaviour -- you'll notice the RepeatN I checked in doesn't have an Option either, but I did put it in that playground.

The goal is to show that Option<RepeatN<String>> will be the same size as RepeatN<String>.

[XeCycle]

@scottmcm Ahh sorry, missed your point. Thanks a lot, now I get it that you want the RepeatN to have a usable niche if the inner T has one, for use with Option<RepeatN<T>>, but Option<(NonZeroUsize, T)> failed to propagate that. Yes, your design is strictly superior. Maybe worth mentioning in this article?

[scottmcm]

Yes, your design is strictly superior.

Rarely is anything strictly 🙂

One thing that the merged implementation doesn't allow is a RepeatN::empty() -- an implementation using the Option approach could do that via None, but there's no way to get a valid-but-unusable ManuallyDrop.

(AKA every niche is also a constraint.)

[Folyd]

I added an alternative section.

One thing you might consider adding is a bit more about why not cloning is useful. Probably doesn't need to be a huge section, but it might be worth a mention, or an example in which you can tell between different rust versions that something different is happening.

I have no idea how to add something about why not cloning is useful. Can you give some suggestions? Thanks. @scottmcm

[Folyd]

Hi, @scottmcm, do you mind giving another review? We plan to release the first issue end of January. 😄

[LCrossman]

I understood unneccessary cloning will just take up memory, especially if the cloned item is large - is that what @scottmcm meant @Folyd ?