In ready_chunks, reserve capacity based on size_hint

[stepancheg]

Reserving ready_chunks limit every time may be too expensive when the limit is high but only a few number of messages available.

Now rely on Stream::size_hint to reserve the capacity. If underlying stream implements size_hint, this will work great. Otherwise ready_chunk will be somewhat less efficient.

This is better alternative to https://github.com/rust-lang/futures-rs/pull/2657

[notgull]

Do you have any benchmarks pr use cases that demonstrate an improvement in performance?

[stepancheg]

Do you have any benchmarks pr use cases that demonstrate an improvement in performance?

I can create a benchmark which demonstrates the behavior:

• call ready_chunks with argument 1000000
• create a channel which provides one message at a time
• observe excessive memory usage: 1000000 * size_of::<T>()

Current behavior of excessive memory allocation prevents using ready_chunks with large capacity.

[taiki-e]

Thanks for the PR! size_hint means the number of elements remaining in the entire stream, and since the information we really want here is the number of elements until the next Pending, I guess the effect of this is not very large.

That said, this seems to be somewhat more reasonable than #2657 or the current implementation. I tend to merge this once the review is addressed.

[tzachar]

Is there a plan to merge this? Also, on a side note, why is the reallocation of a new buffer required on every entry to poll_next? wouldn't it be more efficient to allocate the buffer on creation, in ReadyChunks::new, and reuse it on every entry to poll_next?

[taiki-e]

reuse it on every entry to poll_next?

How to reuse it? poll_next returns an owned Vec.

[tzachar]

Can't we return a reference to the internal vector? make Self::Item be a & Vec<St::Item> ? Or is there a different way of achieving this, where we are not reallocating a new vector on each call to poll_next?

[taiki-e]

Can't we return a reference to the internal vector? make Self::Item be a & Vec<St::Item> ?

I don't think it is possible without using GAT in the Future trait.

[tzachar]

Its worth trying to support this. Any idea about how to go about it?