This is an archive of the discontinued Mercurial Phabricator instance.

rust-status: move to recursive traversal to prepare for parallel traversal
ClosedPublic

Authored by Alphare on Mar 4 2020, 10:29 AM.

Details

Summary

I have looked into traversing the working directory in parallel either
by a recursive or an iterative algorithm. The recursive approach won quite
decisively both in terms of performance and code readability.

You can look at my experiment here:
https://heptapod.octobus.net/Alphare/rayon-recursive-traversal

The chance of a stack overflow happening because the directories get too nested
seems slim.

This change does not yet do anything in parallel.

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.Mar 4 2020, 10:29 AM
durin42 added a subscriber: durin42.Mar 4 2020, 4:29 PM

I'm broadly happy with this series, but:

  1. I get test-check-rust-format.t errors
  2. I don't seem to be having any luck running the tests. It looks like the rust module policy doesn't cause the right things to get installed? In particular, HGWITHRUSTEXT=cpython HGMODULEPOLICY=rust+c make clean tests flunks test-remotefilelog-datapack.py because it can't import parsers.so? And it looks like test-dirstate-race.t is just broken by this series?
  3. The re2-missing fallback message breaks tests for users that want rust things, but lack re2. This isn't going to work, I'm afraid.

The series is so big it's not practical for me to figure out which revision(s) I should be commenting on, so hopefully that's actionable feedback...

Alphare updated this revision to Diff 20500.Mar 5 2020, 3:34 AM
  1. I get test-check-rust-format.t errors

Indeed. It appears that none of the machines I use to run the entire test suite (so... not my laptop) have rustfmt installed, which skips this test. I have updated the series to fix this issue, and will install rustfmt where needed.

  1. I don't seem to be having any luck running the tests. It looks like the rust module policy doesn't cause the right things to get installed? In particular, HGWITHRUSTEXT=cpython HGMODULEPOLICY=rust+c make clean tests flunks test-remotefilelog-datapack.py because it can't import parsers.so? And it looks like test-dirstate-race.t is just broken by this series?

I will look into this, this is bad.

  1. The re2-missing fallback message breaks tests for users that want rust things, but lack re2. This isn't going to work, I'm afraid.

@marmoute suggested I do something in debuginstall to check for re2 on the Rust side instead, I think it's a good idea.

The series is so big it's not practical for me to figure out which revision(s) I should be commenting on, so hopefully that's actionable feedback...

It is, thanks!

And it looks like test-dirstate-race.t is just broken by this series?

I can't reproduce that however. Do you have a particular way of doing so? Because it's a race test... it might be flaky.

I still don't have a clear solution for the modulepolicy thing, because everything is broken if we use strict policies.

I still don't have a clear solution for the modulepolicy thing, because everything is broken if we use strict policies.

It appears that I got confused by an unrelated issue. @marmoute pushed a fix for this as D8245.

Alphare updated this revision to Diff 20561.Mar 6 2020, 11:31 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.