Page MenuHomePhabricator

rust-dirstate: add rust implementation of `parse_dirstate` and `pack_dirstate`

Authored by Alphare on May 6 2019, 5:03 PM.



Working towards the goal of having a complete Rust implementation of
hg status, these two utils are a first step of many to be taken
to improve performance and code maintainability.

Two dependencies have been added: memchr and byteorder.
Both of them have been written by reputable community members and are
very mature crates.
The Rust code will often need to use their byte-oriented functions.

A few unit tests have been added and may help future development and debugging.

In a future patch that uses parse_dirstate to stat the working tree in
parallel - which neither the Python nor the C implementations do - actual
performance improvements will be seen for larger repositories.

Diff Detail

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

Event Timeline

Alphare created this revision.May 6 2019, 5:03 PM
kevincox requested changes to this revision.May 10 2019, 11:04 AM
kevincox added inline comments.

You have a lot of tuples here an it is unclear what they represent. I would much prefer making structs for these, then you can give the members names which will really help the reader.


You check for PARENT_SIZE then index by PERENT_SIZE*2. This will panic if the size is between the two.


It would be nice to define the above the definition of parents and use it in that slice.


This isn't a very useful name. Maybe entry or entry_bytes.


This is a lot of manual index numbers. It would probably me more clear to wrap it in a Cursor and use to read the values. This way the cursor is automatically moved to match the data you have read.

This revision now requires changes to proceed.May 10 2019, 11:04 AM
Alphare marked 5 inline comments as done.May 13 2019, 5:47 AM
Alphare updated this revision to Diff 15066.

Thanks for review. I think I've addressed all of your points.


Good catch, thanks!

This revision was automatically updated to reflect the committed changes.