[PATCH 1 of 5 RFC] rust: pure Rust lazyancestors iterator

Yuya Nishihara yuya at tcha.org
Sun Sep 30 10:57:38 EDT 2018


On Sun, 30 Sep 2018 15:47:43 +0200, Georges Racinet wrote:
> On 9/30/18 5:46 AM, Yuya Nishihara wrote:
> > On Fri, 28 Sep 2018 15:31:08 +0200, Georges Racinet wrote:
> >> # HG changeset patch
> >> # User Georges Racinet <gracinet at anybox.fr>
> >> # Date 1538060596 -7200
> >> #      Thu Sep 27 17:03:16 2018 +0200
> >> # Node ID d8c9571755a64e1fc3429587dfd3949b9862eceb
> >> # Parent  d3d4b4b5f725124ef9e93cf74d779a7a05aa11b7
> >> # EXP-Topic rustancestors-rfc
> >> rust: pure Rust lazyancestors iterator
> > First, thanks for tackling on the oxidization plain. I've quickly scanned
> > a first few patches, and I'm generally positive to these.
> Thanks for your interest and the positive feedback !
> >
> > Here's random thoughts regarding the project/module layout:
> >
> >  - place pure Rust code under rust/ ?
> >
> >    I think that's the intention why we have the rust/ tree. Perhaps, the
> >    pure Rust modules can be packaged into a crate (e.g. hg-core), and the
> >    Rust FFI part will depend on it (e.g. hg-ffi).
> Ah, I was thinking that the rust/ tree was dedicated to the executable.
> >
> >    And the compiled CPython module (and maybe source) will be placed at
> >    mercurial/rust.
> >
> >  - HGMODULEPOLICY=rust
> >
> >    Perhaps, we'll want a separate policy to enable the Rust bindings.
> >    Most of the unimplemented functions will be forwarded to cext, just like
> >    the cffi -> pure fallback.
> I have yet to understand a bit more how way policy build and runtime
> works, but this sounds nice too. Would you see this as part of the
> patchset or in a subsequent one ?

That can be addressed later. But my concern is that this patch series hooks
into the cext sources. I'm afraid of doing that because the cext is, say,
uneasy to review, whereas it should be stable.

I hope the Rust cext part will be in a separate module even if it partially
depends on the current CPython extension.

> >> +    #[inline]
> >> +    fn conditionally_push_rev(&mut self, rev: i32) {
> >> +        if self.stoprev <= rev && !self.seen.contains(&rev) {
> >> +            self.seen.insert(rev);
> >> +            self.visit.push(rev);
> > We could eliminate the seen set by skipping duplicated revisions in next().
> > It's theoretically slower and was actually slower in Python, but might be
> > worth trying in Rust.
> 
> What would be the procedure for submission here ? Maybe as a subsequent
> patch, so that performance comparison would be easier to make, or do you
> suggest we'd go in that direction right away ?

That's just an idea. No need to try out right way.


More information about the Mercurial-devel mailing list