Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for some sort of collect()/extend() into LinkedList<Vec<T>> #1127

Closed
coolreader18 opened this issue Feb 6, 2024 · 1 comment · Fixed by #1128
Closed

Support for some sort of collect()/extend() into LinkedList<Vec<T>> #1127

coolreader18 opened this issue Feb 6, 2024 · 1 comment · Fixed by #1128

Comments

@coolreader18
Copy link
Contributor

coolreader18 commented Feb 6, 2024

From what I can tell, building a LinkedList<Vec<T>> is an efficient (one of the more efficient?) ways of collecting a parallel iterator, which is why ParallelExtend for Vec uses it. However, if you don't need specifically a Vec the process of concatenating all the vecs might be unnecessary overhead. It'd be nice if ListVecConsumer was exposed in some way, so that users could collect like that. (Yes, you could just .fold(Vec::new, |vec, item| vec.push(item)), but that's an element-wise push that skips specialization of Extend for Vec)

Directly implementing FromParallelIterator<T> for LinkedList<Vec<T>> might break type inference, but there could be a collect_vec_list() method on ParallelIterator, maybe.

@cuviper
Copy link
Member

cuviper commented Feb 6, 2024

From what I can tell, building a LinkedList<Vec<T>> is an efficient (one of the more efficient?) ways of collecting a parallel iterator, which is why ParallelExtend for Vec uses it.

It's the most efficient way that we know, at least for unindexed iterators. For IndexedParallelIterator, we have a bit of pseudo-specialization (via opt_len) that lets it collect directly into the final pre-allocated Vec. We could still do that in your scenario as well, just slightly wastefully allocating a single LinkedList node to hold the entire Vec.

there could be a collect_vec_list() method on ParallelIterator, maybe.

I think that sounds reasonable!

I don't think we need an extend flavor though, because anyone can cheaply append a collection to an existing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants