Re: [PATCH v3 17/25] rust: alloc: implement `collect` for `IntoIter`

[Danilo Krummrich <dakr@kernel.org>]

On Thu, Aug 01, 2024 at 05:10:22PM +0200, Alice Ryhl wrote:
> On Thu, Aug 1, 2024 at 2:08 AM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > Currently, we can't implement `FromIterator`. There are a couple of
> > issues with this trait in the kernel, namely:
> >
> >   - Rust's specialization feature is unstable. This prevents us to
> >     optimze for the special case where `I::IntoIter` equals `Vec`'s
> >     `IntoIter` type.
> >   - We also can't use `I::IntoIter`'s type ID either to work around this,
> >     since `FromIterator` doesn't require this type to be `'static`.
> >   - `FromIterator::from_iter` does return `Self` instead of
> >     `Result<Self, AllocError>`, hence we can't properly handle allocation
> >     failures.
> >   - Neither `Iterator::collect` nor `FromIterator::from_iter` can handle
> >     additional allocation flags.
> >
> > Instead, provide `IntoIter::collect`, such that we can at least convert
> > `IntoIter` into a `Vec` again.
> >
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> 
> I'm not convinced a collect implementation specific to IntoIter is necessary?

For the reasons above, we can't implement `FromIterator`. At some point we may
want to implement our own kernel `FromIterator` trait, but that's out of scope
for this series.

For now, I just want to provide a way to get a `Vec` from `IntoIter` again,
which without `Vec::collect` would be impossible.

> 
> > +
> > +        // SAFETY: `buf` points to the start of the backing buffer and `len` is guaranteed to be
> > +        // smaller than `cap`. Depending on `alloc` this operation may shrink the buffer or leaves
> > +        // it as it is.
> > +        ptr = match unsafe { A::realloc(Some(buf.cast()), layout, flags) } {
> 
> Why would you shrink it? You can just keep the capacity.

What if the vector was pretty huge and meanwhile as advanced by a lot? I think
we don't want to waste those resources.

Ideally the corresponding `Allocator` implements a proper heuristic for when to
actually shrink. For instance, krealloc() never shrinks, and it's probably not
worth it. For vrealloc() though we clearly want to shrink properly (i.e. unmap
and free spare pages) at some point.

> 
> Alice
>