D5370: rust: core implementation of missingancestors (no bindings)
gracinet (Georges Racinet)
phabricator at mercurial-scm.org
Wed Dec 12 10:11:13 EST 2018
gracinet marked 11 inline comments as done.
gracinet added a comment.
@yuja trying to submit the v2 with phabsend instead of arcanist… hope it'll be linked properly
INLINE COMMENTS
> kevincox wrote in ancestors.rs:201
> Isn't this loop redundant with the retain call above?
indeed, must have been a leftover
> kevincox wrote in ancestors.rs:235
> I would prefer `let (p0, p1) = ...`. This makes it obvious exactly what data you are getting.
In the next version, I'm simply doing a for loop on `.iter().cloned()`, it's even better imho.
> kevincox wrote in lib.rs:26
> 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.
Thanks for the tip with `.cloned()` that's exactly what I was missing.
Out of curiosity, I took a look at the generated optimized assembly code: zero cost abstraction, indeed.
About HashSet, yes that's exactly what I'm thinking.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5370
To: gracinet, #hg-reviewers
Cc: yuja, durin42, kevincox, mjpieters, mercurial-devel
More information about the Mercurial-devel
mailing list