( )⚙ D1403 treedirstate: better iteration using visitor pattern

This is an archive of the discontinued Mercurial Phabricator instance.

treedirstate: better iteration using visitor pattern
ClosedPublic

Authored by mbthomas on Nov 14 2017, 12:39 PM.
Tags
None
Subscribers

Details

Reviewers
jsgf
Group Reviewers
Restricted Project
Commits
rFBHGXe5128555cede: treedirstate: better iteration using visitor pattern
Summary

In order to allow the python interface code to perform actions on each node in
the tree without creating python-specific interfaces in the generic Rust code,
add a method of iterating over the tree, executing a closure at each file.

Use this to implement the methods that give iterators over the filenames in the
tree. This performs better than the get_first/get_next-style iterators.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mbthomas created this revision.Nov 14 2017, 12:39 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 14 2017, 12:39 PM
mbthomas updated this revision to Diff 3600.Nov 17 2017, 9:45 AM
mbthomas updated this revision to Diff 3655.Nov 20 2017, 12:21 PM
durham added a subscriber: durham.Nov 21 2017, 12:49 AM
durham added inline comments.
treedirstate/__init__.py
120

Do we eventually do the same thing for these iterators?

mbthomas added inline comments.Nov 21 2017, 6:46 AM
treedirstate/__init__.py
120

I don't have a better implementation for these iterators. For the iterators that return lists of names, building a list in one go and then returning an iterator over that list turned out to be much more performant than using the getfirst/getnext iterators (not really surprising), but is also better than core where the equivalent function iterates over the dict keys and then sorts them.

These ones are more difficult, as we need the names and the state tuples, which we construct on the fly from the data in the tree. I'm not sure that constructing the whole sequence in one iteration would be the right approach.

This seems adequate for now, and we can iterate on it later if it turns out we're spending all our time in treedirstatemapiterator.__next__.

mbthomas updated this revision to Diff 3730.Nov 21 2017, 1:35 PM
mbthomas updated this revision to Diff 3771.Nov 22 2017, 1:14 PM
jsgf accepted this revision.Nov 22 2017, 6:52 PM
jsgf added a subscriber: jsgf.
jsgf added inline comments.
rust/treedirstate/src/python.rs
183

Does the compiler complain without the annotations, or is it just for documentation?

treedirstate/__init__.py
120

There's experimental generator support in Rust nightly, which may well land in stable. It might be helpful here.

This revision is now accepted and ready to land.Nov 22 2017, 6:52 PM
mbthomas added inline comments.Nov 23 2017, 7:24 AM
rust/treedirstate/src/python.rs
183

It's needed. The compiler can't deduce the types for filepath or state without them.

mbthomas updated this revision to Diff 3824.Nov 24 2017, 11:53 AM
mbthomas updated this revision to Diff 3842.Nov 24 2017, 3:18 PM
mbthomas updated this revision to Diff 3865.Nov 27 2017, 8:00 AM
This revision was automatically updated to reflect the committed changes.