D5550: rust-cpython: bindings for MissingAncestors

[kevincox (Kevin Cox)]

kevincox added inline comments.

INLINE COMMENTS

> gracinet wrote in ancestors.rs:170
> Hi @kevincox. I have a general question for these, please correct me if I'm making wrong assumptions.
> 
> Given that we know in advance exactly the number of elements required, and that it can be large, doesn't it add overhead to convert everything to collection of iterators? I suppose the `Vec` would have to grow several times, and that's as many calls to `malloc()` inernally. I'm not sure at this point it would be very costly, but in this case where we consume the `Vec` immediately, is there a reason to believe that going the `collect()` way could have their own performance benefits?
> 
> Note: this code landed, but I'm about to submit some related refactors, and in this specific case, it's going to be replace I hope very soon with a call to `PySet`, but I'm asking in general.
> 
> Cheers,

Iterator has a size_hint <https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.size_hint> method which can provide a clue as to the size of the iterator if know. There is also the ExactSizeIterator trait but it is sufficient to say that most simple operations on slice (or Vec) iterators will maintain the size hint and that collecting into a vector will be the most efficient way to construct a vector.

In theory the performance could be slightly better for the collect approach as you could avoid some bounds checking and incrementing the size each time but in practice I would expect similar performance.

So the TL;DR is don't worry about collect performance especially for the simple situations. If there are more allocations then necessary then the bug is in the rust `std` crate.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5550

To: gracinet, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel