D5370: rust: core implementation of missingancestors (no bindings)

[kevincox (Kevin Cox)]

kevincox added a comment.


  I think sticking close to the python version makes sense for the initial version. Then improvements can be made in follow-ups.

INLINE COMMENTS

> gracinet wrote in lib.rs:26
> The reason I've preferred to implement it directly is that `into_iter()` iterates on references, which I found an unnecessary overhead,.
> 
> But maybe that's a case of premature optimization ? In the same vein, I don't think `HashSet` is the best choice for a set of `i32`, unless it becomes really big, but I don't know where the  threshold would be, compared to, say, a bit array.

You can add `.cloned()` to remove the references. I would be surprised if this has poor code generation.

As for HashSet vs bit array I would stick to HashSet for now. HashSet also has the advantage of being sparse. I guess after the initial implementation we could benchmark the two to see what is better.

REPOSITORY
  rHG Mercurial

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

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