D5550: rust-cpython: bindings for MissingAncestors

gracinet (Georges Racinet) phabricator at mercurial-scm.org
Mon Jan 14 19:45:51 EST 2019


gracinet added inline comments.

INLINE COMMENTS

> kevincox wrote in ancestors.rs:170
> 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.

Thanks for the detailed answer.

Then the problem, in case of iterators coming directly from Python, is that they don't currently implement `size_hint()`, so there's an improvement to be done at this level (many Pythoin iterators do have a `__length_hint()` method, so it shouldn't be a problem to reuse it)

I have currently some suspicions that the conversions between Rust and Python are the bottleneck in some important cases, so I'll have to measure this things anyway. I won't touch that immediately, but I'll get back to it with real soon.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list