Patchwork D7118: rust-dirstatemap: remove additional lookups in traverse

login
register
mail settings
Submitter phabricator
Date Oct. 16, 2019, 1:48 p.m.
Message ID <differential-rev-PHID-DREV-g2tzrcjpqvl43cznhomd-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42384/
State New
Headers show

Comments

phabricator - Oct. 16, 2019, 1:48 p.m.
Alphare created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We use the same trick as the Python implementation.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Oct. 18, 2019, 10:44 a.m.
> +++ b/mercurial/dirstate.py
> @@ -919,6 +919,9 @@
>          matchalways = match.always()
>          matchtdir = match.traversedir
>          dmap = self._map
> +        if rustmod is not None:
> +            dmap = self._map._rustmap

If it's the same trick, can't it be abstracted away? `if rustmod` seems weird.
phabricator - Oct. 18, 2019, 10:47 a.m.
yuja added a comment.


  > +++ b/mercurial/dirstate.py
  > @@ -919,6 +919,9 @@
  >
  >   matchalways = match.always()
  >   matchtdir = match.traversedir
  >   dmap = self._map
  >
  > +        if rustmod is not None:
  > +            dmap = self._map._rustmap
  
  If it's the same trick, can't it be abstracted away? `if rustmod` seems weird.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7118/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - Oct. 18, 2019, 12:24 p.m.
Alphare added a comment.


  In D7118#104834 <https://phab.mercurial-scm.org/D7118#104834>, @yuja wrote:
  
  >> +++ b/mercurial/dirstate.py
  >> @@ -919,6 +919,9 @@
  >>
  >>   matchalways = match.always()
  >>   matchtdir = match.traversedir
  >>   dmap = self._map
  >>
  >> +        if rustmod is not None:
  >> +            dmap = self._map._rustmap
  >
  > If it's the same trick, can't it be abstracted away? `if rustmod` seems weird.
  
  The main issue I faced is that Python classes built from `rust-cpython` cannot be subclassed, so we have to resort to an additional level of indirection. IIRC, the main reason was that there was no way of enforcing a subclass to call `super()` which would result in the Rust class not being initialized properly. Or maybe I'm missing the point?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7118/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - Oct. 18, 2019, 12:35 p.m.
>   >> +++ b/mercurial/dirstate.py
>   >> @@ -919,6 +919,9 @@
>   >>
>   >>   matchalways = match.always()
>   >>   matchtdir = match.traversedir
>   >>   dmap = self._map
>   >>
>   >> +        if rustmod is not None:
>   >> +            dmap = self._map._rustmap
>   >
>   > If it's the same trick, can't it be abstracted away? `if rustmod` seems weird.
>   
>   The main issue I faced is that Python classes built from `rust-cpython` cannot be subclassed, so we have to resort to an additional level of indirection. IIRC, the main reason was that there was no way of enforcing a subclass to call `super()` which would result in the Rust class not being initialized properly. Or maybe I'm missing the point?

I mean there could be a unified `self._map._map` call if we need to take
the internal map from both py dirstatemap and rust dirstatemap.
phabricator - Oct. 18, 2019, 12:40 p.m.
yuja added a comment.


  >   >> +++ b/mercurial/dirstate.py
  >   >> @@ -919,6 +919,9 @@
  >   >>
  >   >>   matchalways = match.always()
  >   >>   matchtdir = match.traversedir
  >   >>   dmap = self._map
  >   >>
  >   >> +        if rustmod is not None:
  >   >> +            dmap = self._map._rustmap
  >   >
  >   > If it's the same trick, can't it be abstracted away? `if rustmod` seems weird.
  >   The main issue I faced is that Python classes built from `rust-cpython` cannot be subclassed, so we have to resort to an additional level of indirection. IIRC, the main reason was that there was no way of enforcing a subclass to call `super()` which would result in the Rust class not being initialized properly. Or maybe I'm missing the point?
  
  I mean there could be a unified `self._map._map` call if we need to take
  the internal map from both py dirstatemap and rust dirstatemap.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7118/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -919,6 +919,9 @@ 
         matchalways = match.always()
         matchtdir = match.traversedir
         dmap = self._map
+        if rustmod is not None:
+            dmap = self._map._rustmap
+
         listdir = util.listdir
         lstat = os.lstat
         dirkind = stat.S_IFDIR