public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Alice Ryhl" <aliceryhl@google.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] rust: alloc: implement `extend` for `Vec`
Date: Wed, 23 Apr 2025 18:40:07 +0900	[thread overview]
Message-ID: <D9DX39CF2RB7.IM219BZLVMCY@nvidia.com> (raw)
In-Reply-To: <aAiqCXB9sJe_v6Yc@google.com>

On Wed Apr 23, 2025 at 5:51 PM JST, Alice Ryhl wrote:
> On Wed, Apr 23, 2025 at 10:02:58AM +0900, Alexandre Courbot wrote:
>> The problem I see is that if you try and do something like:
>> 
>>   vec.extend((0..10).into_iter().skip(2));
>> 
>> with the standard library, then the use of `skip` will remove the
>> `TrustedLen` implementation from the resulting iterator and
>> `extend_desugared` will be called instead of `extend_trusted`, which
>> could add some unwanted (and unexpected) overhead.
>> 
>> If we want an implementation of `extend` as simple as "confidently
>> increase the length of the vector and copy the new items into it, once",
>> then we need a trait that can be implemented on both shrinking and
>> extending adapters. Anything else and we might trick the caller into a
>> code path less efficient than expected (i.e. my original version, which
>> generates more core even for the obvious cases that are `extend_with`
>> and `extend_from_slice`). Or if we rely on `TrustedLen` solely in the
>> kernel, then `extend` could not be called at all with this particular
>> iterator.
>> 
>> There is also the fact that `TrustedLen` is behind a nightly feature,
>> which I guess is another obstacle for using it.
>
> The stdlib alloc crate relies on specialization to speed up methods
> related to iterators. We can't use specialization, so losing these
> optimizations is simply a cost of not using the upstream alloc library
> that we have to accept.

Yeah I was surprised to see

  impl<T, I, A: Allocator> SpecExtend<T, I> for Vec<T, A>
  where
      I: Iterator<Item = T>

and

  impl<T, I, A: Allocator> SpecExtend<T, I> for Vec<T, A>
  where
      I: TrustedLen<Item = T>

in the standard library, which clearly looks like an overlap. Didn't
know it was relying on a non-standard feature.

That's going to limit what we can do in the kernel, but nonetheless if
we can support only the cases that can be optimized I think we would
have our bases covered.

  reply	other threads:[~2025-04-23  9:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-06 13:01 [PATCH v3] rust: alloc: implement `extend` for `Vec` Alexandre Courbot
2025-04-07 11:01 ` Danilo Krummrich
2025-04-08 13:34   ` Alexandre Courbot
2025-04-21  8:15     ` Alexandre Courbot
2025-04-22 17:03       ` Danilo Krummrich
2025-04-23  1:02         ` Alexandre Courbot
2025-04-23  8:51           ` Alice Ryhl
2025-04-23  9:40             ` Alexandre Courbot [this message]
2025-04-23 16:03               ` Boqun Feng
2025-04-24 11:50                 ` Alice Ryhl
2025-04-24 13:36                   ` Boqun Feng
2025-04-23  9:47           ` Danilo Krummrich
2025-04-23 13:15             ` Alexandre Courbot
  -- strict thread matches above, loose matches on Subject: below --
2025-04-07 16:33 Benno Lossin
2025-04-08 14:00 ` Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D9DX39CF2RB7.IM219BZLVMCY@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox