recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Skip "git for-each-ref" when identifying deleted commits

Summary:
Ref T13647. The ref discovery process prunes commits that no longer exist in the repository before executing "git log <new heads> --not <old heads>" to identify newly published commits.

If we don't do this, the "git log" command will fail if any old head has been pruned from the repository.

Currently, this test for missing commits starts with a call to "git for-each-ref" to attempt to resolve symbols as tag or branch names, but:

- this is painfully slow in repositories with many refs; and
- this is incorrect (not consistent with "git" behavior) for 40-character hex strings, which Git will never resolve as symbolic names.

Instead, when a symbol is a 40-character hex string, skip "git for-each-ref" and jump directly to "git cat-file --batch-check".

Test Plan:
- Ran `bin/repository update` in a repository with 65K refs and extra debugging info.
- Before: took ~30s, three calls to `git for-each-ref`.
- After: took ~20s, two calls to `git for-each-ref`. Same resolution result on queries.

Maniphest Tasks: T13647

Differential Revision: https://secure.phabricator.com/D21658

epriestley aa70b008 5b8b5f21

+49 -31
+49 -31
src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php
··· 63 63 $unresolved = array_fuse($this->refs); 64 64 $results = array(); 65 65 66 - // First, resolve branches and tags. 67 - $ref_map = id(new DiffusionLowLevelGitRefQuery()) 68 - ->setRepository($repository) 69 - ->withRefTypes( 70 - array( 71 - PhabricatorRepositoryRefCursor::TYPE_BRANCH, 72 - PhabricatorRepositoryRefCursor::TYPE_TAG, 73 - )) 74 - ->execute(); 75 - $ref_map = mgroup($ref_map, 'getShortName'); 66 + $possible_symbols = array(); 67 + foreach ($unresolved as $ref) { 76 68 77 - $tag_prefix = 'refs/tags/'; 78 - foreach ($unresolved as $ref) { 79 - if (empty($ref_map[$ref])) { 69 + // See T13647. If this symbol is exactly 40 hex characters long, it may 70 + // never resolve as a branch or tag name. Filter these symbols out for 71 + // consistency with Git behavior -- and to avoid an expensive 72 + // "git for-each-ref" when resolving only commit hashes, which happens 73 + // during repository updates. 74 + 75 + if (preg_match('(^[a-f0-9]{40}\z)', $ref)) { 80 76 continue; 81 77 } 82 78 83 - foreach ($ref_map[$ref] as $result) { 84 - $fields = $result->getRawFields(); 85 - $objectname = idx($fields, 'refname'); 86 - if (!strncmp($objectname, $tag_prefix, strlen($tag_prefix))) { 87 - $type = 'tag'; 88 - } else { 89 - $type = 'branch'; 79 + $possible_symbols[$ref] = $ref; 80 + } 81 + 82 + // First, resolve branches and tags. 83 + if ($possible_symbols) { 84 + $ref_map = id(new DiffusionLowLevelGitRefQuery()) 85 + ->setRepository($repository) 86 + ->withRefTypes( 87 + array( 88 + PhabricatorRepositoryRefCursor::TYPE_BRANCH, 89 + PhabricatorRepositoryRefCursor::TYPE_TAG, 90 + )) 91 + ->execute(); 92 + $ref_map = mgroup($ref_map, 'getShortName'); 93 + 94 + $tag_prefix = 'refs/tags/'; 95 + foreach ($possible_symbols as $ref) { 96 + if (empty($ref_map[$ref])) { 97 + continue; 90 98 } 91 99 92 - $info = array( 93 - 'type' => $type, 94 - 'identifier' => $result->getCommitIdentifier(), 95 - ); 100 + foreach ($ref_map[$ref] as $result) { 101 + $fields = $result->getRawFields(); 102 + $objectname = idx($fields, 'refname'); 103 + if (!strncmp($objectname, $tag_prefix, strlen($tag_prefix))) { 104 + $type = 'tag'; 105 + } else { 106 + $type = 'branch'; 107 + } 96 108 97 - if ($type == 'tag') { 98 - $alternate = idx($fields, 'objectname'); 99 - if ($alternate) { 100 - $info['alternate'] = $alternate; 109 + $info = array( 110 + 'type' => $type, 111 + 'identifier' => $result->getCommitIdentifier(), 112 + ); 113 + 114 + if ($type == 'tag') { 115 + $alternate = idx($fields, 'objectname'); 116 + if ($alternate) { 117 + $info['alternate'] = $alternate; 118 + } 101 119 } 120 + 121 + $results[$ref][] = $info; 102 122 } 103 123 104 - $results[$ref][] = $info; 124 + unset($unresolved[$ref]); 105 125 } 106 - 107 - unset($unresolved[$ref]); 108 126 } 109 127 110 128 // If we resolved everything, we're done.