Page MenuHomePhabricator

rust-dirstatemap: remove additional lookups in traverse
Needs ReviewPublic

Authored by Alphare on Oct 16 2019, 9:48 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

We use the same trick as the Python implementation.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Oct 16 2019, 9:48 AM
yuja added a subscriber: yuja.Fri, Oct 18, 6:47 AM

+++ 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.

In 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?

yuja added a comment.Fri, Oct 18, 8:40 AM
>> +++ 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.