Page MenuHomePhabricator

Improve "refengine" performance for testing large numbers of Mercurial branches
ClosedPublic

Authored by epriestley on Oct 20 2017, 5:25 PM.
Tags
None
Referenced Files
F14049954: D18717.diff
Thu, Nov 14, 3:28 PM
F14035265: D18717.diff
Sun, Nov 10, 5:13 AM
F14019480: D18717.diff
Tue, Nov 5, 10:24 PM
F14001255: D18717.diff
Fri, Oct 25, 5:43 AM
F13966480: D18717.id.diff
Oct 16 2024, 8:11 AM
F13959763: D18717.id44933.diff
Oct 14 2024, 8:41 PM
Unknown Object (File)
Sep 12 2024, 9:05 AM
Unknown Object (File)
Sep 9 2024, 2:40 PM
Subscribers
None
Tokens
"Love" token, awarded by ckolos.

Details

Summary

See PHI158. In the RefEngine, we test if any old branch positions have been removed from the repository. This is uncommon (but not impossible) in Mercurial, and corresponds to users deleting branches in Git.

Currently, we end up running hg log for each position, in parallel. Because of Python's large startup overhead, this can be resource intensive for repositories with a large number of branches.

We have to do this in the general case because the caller may be asking us to resolve tip, newfeature, tip~3, 9, etc. However, in the specific case where the refs are 40-digit hashes, we can bulk resolve them if they exist, like this:

hg log ... --rev (abcd or def0 or ab12 or ...)

In the general case, we could probably do less of this than we currently do (instead of testing all old heads, we could prune the list by removing commits which we know are still pointed to by current heads) but that's a slightly more involved change and the effect here is already dramatic.

Test Plan

Verified that CPU usage drops from ~110s -> ~0.9s:

Before:

epriestley@orbital ~/dev/phabricator $ time ./bin/repository refs nss
Updating refs in "nss"...
Done.

real	0m14.676s
user	1m24.714s
sys	0m21.645s

After:

epriestley@orbital ~/dev/phabricator $ time ./bin/repository refs nss
Updating refs in "nss"...
Done.

real	0m0.861s
user	0m0.882s
sys	0m0.213s
  • Manually resolved blue, tip, 9, etc., got expected results.
  • Tried to resolve invalid hashes, got expected result (no resolution).

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley created this revision.

(I cheated a little bit and executed part of my test plan after writing it, and found a bug.)

  • Fix error handling when the ref list includes invalid, hashlike refs.
This revision is now accepted and ready to land.Oct 20 2017, 6:02 PM
This revision was automatically updated to reflect the committed changes.