Commit 297ba4e
committed
diffcore-rename, merge-ort: use handle_early_known_dir_renames()
Put the work of the last several patches to work by calling the new
handle_early_known_dir_renames() function just after doing
basename-guided rename detection.
For the testcases mentioned in commit 557ac03 ("merge-ort: begin
performance work; instrument with trace2_region_* calls", 2020-10-28),
this change improves the performance as follows:
Before After
no-renames: 5.680 s ± 0.096 s 5.665 s ± 0.129 s
mega-renames: 13.812 s ± 0.162 s 11.435 s ± 0.158 s
just-one-mega: 506.0 ms ± 3.9 ms 494.2 ms ± 6.1 ms
While this improvement looks rather modest for these testcases (because
all the previous optimizations were sufficient to nearly remove all time
spent in rename detection already), there was a cherry-pick in a real
world (but private) repository at $DAYJOB that showed a speedup factor
of ~7x from this optimization.
An important side note for future further optimization:
There is a possible improvement to this optimization that I have not yet
attempted: we could first check whether exact renames provide enough
information for us to determine directory renames, and avoid doing
basename-guided rename detection on some or all of the RELEVANT_LOCATION
files within those directories. In effect, this variant would mean
doing the handle_early_known_dir_renames() both after exact rename
detection and again after basename-guided rename detection, though I
remember thinking at one point that there was some extra cleanup needed
between the steps if we were to try that. Checking for skippable
renames an extra time might be a valuable optimization based on an
understanding of the difference in performance of exact rename detection
and basename-guided rename detection:
While basename-guided rename detection is faster than full inexact
rename detection with the latter's NxM file content comparisons,
basename-guided rename detection is vastly slower than exact rename
detection. The reason for this is that the first time you need to
compare a file's contents to another is the most expensive; after
diffcore-rename has generated a sequence of hashes for a file's
contents, any subsequent comparisons of that file (via the sequence
of hashes) to another file is much cheaper.
So, removing files from rename detection before doing some
basename-guided rename detection could potentially improve this
optimization further. However, this particular optimization was
actually the last one I did in original implementation order, and by the
time I implemented this idea, every testcase I had was sufficiently fast
that further optimization was unwarranted. If future testcases arise
that tax rename detection more heavily, it may be worth implementing
this more involved variant.
Signed-off-by: Elijah Newren <newren@gmail.com>1 parent 78967a5 commit 297ba4e
1 file changed
Lines changed: 6 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1084 | 1084 | | |
1085 | 1085 | | |
1086 | 1086 | | |
1087 | | - | |
1088 | 1087 | | |
1089 | 1088 | | |
1090 | 1089 | | |
| |||
1325 | 1324 | | |
1326 | 1325 | | |
1327 | 1326 | | |
| 1327 | + | |
| 1328 | + | |
| 1329 | + | |
| 1330 | + | |
1328 | 1331 | | |
1329 | 1332 | | |
1330 | 1333 | | |
| 1334 | + | |
| 1335 | + | |
1331 | 1336 | | |
1332 | 1337 | | |
1333 | 1338 | | |
| |||
0 commit comments