( )⚙ D9133 rust: start plugging the dirstate tree behind a feature gate

This is an archive of the discontinued Mercurial Phabricator instance.

rust: start plugging the dirstate tree behind a feature gate
ClosedPublic

Authored by Alphare on Sep 30 2020, 1:36 PM.

Details

Summary

The previous patch added the dirstate-tree feature gate to enable the two
dirstate implementations to co-habit while the tree-based one gets better.

This patch copies over the code that differs, be it because the algorithm
changed or because the borrowing rules are different.

Indeed, DirstateTree is not observationally equivalent to the std HashMap in
the APIs we use: it does not have the Entry API (yet?) and its iterator
returns owned values instead of references. This last point is because the
implementation needs to be changed to a more clever and efficient solution.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Sep 30 2020, 1:36 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.
martinvonz added inline comments.
rust/hg-core/src/dirstate/dirs_multiset.rs
60

I think I asked you a long time ago if we still need this ugly argument. Do you know if we do? I.e, I was hoping we could remove it from the Python and C code as well by filtering those entries out before we call this.

66–73

Two alternatives that avoid the repeated multiset.add_path() (it's just one short line, so it's not a big deal, of course). I don't know if they result in the same binary code.

match skip_state {
    Some(skip) if skip == state => {
        // we were told to skip this files in this state
    },
    _ => {
        multiset.add_path(filename)?;
    }
}
if Some(*state) != skip_state {
    multiset.add_path(filename)?;
}
rust/hg-core/src/dirstate/dirstate_map.rs
303–308

Did you consider making these conditions functions on the DirstateEntry struct?

rust/hg-core/src/dirstate/parsers.rs
183

Can we remove the vector by using iter_mut() on line 188?

191–208

Same here, would it make sense to write this something like entry.is_potential_race(now) (feel free to pick a better name)?

rust/hg-core/src/dirstate/status.rs
733

nit: ref no longer needed with recent rust versions? but maybe we need to be compatible with older versions?