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

Yuya Nishihara yuya at tcha.org
Thu Dec 6 08:56:02 EST 2018


Quickly scanned, and looks generally good to me.

>   rust: iterator version of Graph.parents
>   rust: translation of missingancestors
>   rust: translated random test of missingancestors

Can you send these as separate patches?

>   An alternative would have been to expose to Python
>   MissingAncestors<VecGraphs> but that would have meant
>   
>   - pollution of the release build used from Python,  whereas we do it in this changeset within the tests submodule
>   - waiting on rust-cpython bindings to be ready or doing the cumbersome direct-ffi (more pollution with unsafe C code)

Still I want some CPython interface to measure the perf win. Are there
lots of work remaining to bring rust-cpython to us?

>   although one could argue that actually parents() should return an
>   array instead of a tuple, giving us a similar iterator for free (but on
>   references instead of values, unless we also use the arrayvec crate
>   could help). Notably, the current C-backed parents() internally uses an
>   array for communication with C code, so that currently, we'd get less memory
>   copy and less code using an array.

I prefer changing parents() to return `[Revision; 2]`. Then, we can write a
simple utility function that drops NULL_REVISION, `&[Revision; 2] -> &[Revision]`.


More information about the Mercurial-devel mailing list