From 1e7cc72cd835eae282f7ea6359716b7d3a7d5610 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 3 Apr 2020 13:18:29 -0700 Subject: [PATCH] Improve performance when marking commits as unreachable after multiple ref deletions Summary: See PHI1688. If many refs with a large amount of shared ancestry are deleted from a repository, we can spend much longer than necessary marking their mutual ancestors as unreachable over and over again. For example, if refs A, B and C all point near the head of an obsolete "develop" branch and have about 1K shared commits reachable from no other refs, deleting all three refs will lead to us performing 3,000 mark-as-unreachable operations (once for each "" pair). Instead, we can stop exploring history once we reach an already-unreachable commit. Test Plan: - Destroyed 7 similar refs simultaneously. - Ran `bin/repository refs`, saw 7 entries appear in the `oldref` table. - Ran `bin/repository discover` with some debugging statements added, saw sensible-seeming behavior which didn't double-mark any newly-unreachable refs. Differential Revision: https://secure.phabricator.com/D21056 --- .../engine/PhabricatorRepositoryDiscoveryEngine.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php index 790b95d775..2fc1abbf34 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php @@ -840,6 +840,13 @@ final class PhabricatorRepositoryDiscoveryEngine $seen[$target_identifier] = true; + // See PHI1688. If this commit is already marked as unreachable, we don't + // need to consider its ancestors. This may skip a lot of work if many + // branches with a lot of shared ancestry are deleted at the same time. + if ($target->isUnreachable()) { + continue; + } + try { $stream->getCommitDate($target_identifier); $reachable = true;