fix(repo-server): optimize short ref resolution in revision comparison (#26456)

Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com>

docs/operator-manual/high_availability.md

@@ -265,6 +265,42 @@ manifest generation requires to change a file in the local repository clone then
 per server instance is allowed. This limitation might significantly slow down Argo CD if you have a monorepo with
 multiple applications (50+).
 
+### Use Fully Qualified Git References
+
+When specifying the `targetRevision` in your Application manifests, using fully qualified Git reference paths instead of short names can significantly improve repo-server performance, especially in large monorepos with hundreds of thousands of commits and tags.
+
+**Performance Impact:**
+
+When resolving a Git reference (e.g., converting `main` to a commit SHA), Argo CD needs to:
+
+1. Load all Git references (branches and tags)
+2. Iterate through all references to find a match
+3. Resolve symbolic references if needed
+
+For repositories with many references, this process is CPU and memory intensive. Using fully qualified references allows Argo CD to optimize the resolution process and leverage caching more effectively.
+
+**Recommended approach:**
+
+```yaml
+# ❌ Less efficient - requires iteration through all refs
+spec:
+  source:
+    targetRevision: main
+
+# ✅ More efficient - directly identifies the reference type
+spec:
+  source:
+    targetRevision: refs/heads/main
+```
+
+**Common fully qualified reference formats:**
+
+* **Branches**: `refs/heads/<branch-name>` (e.g., `refs/heads/main`, `refs/heads/develop`)
+* **Tags**: `refs/tags/<tag-name>` (e.g., `refs/tags/v1.0.0`)
+* **Pull requests** (GitHub): `refs/pull/<pr-number>/head` (e.g., `refs/pull/123/head`)
+* **Merge requests** (GitLab): `refs/merge-requests/<mr-number>/head`
+
+
 ### Enable Concurrent Processing
 
 Argo CD determines if manifest generation might change local files in the local repository clone based on the config

util/git/client.go

@@ -776,6 +776,9 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 	// symbolic reference (like HEAD), in which case we will resolve it from the refToHash map
 	refToResolve := ""
 
+	isShortRef := IsShortRef(revision)
+	log.Debugf("Attempting to resolve revision '%s' (is short ref: %t)", revision, isShortRef)
+
 	for _, ref := range refs {
 		refName := ref.Name().String()
 		hash := ref.Hash().String()
@@ -783,7 +786,7 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 			refToHash[refName] = hash
 		}
 		// log.Debugf("%s\t%s", hash, refName)
-		if ref.Name().Short() == revision || refName == revision {
+		if (isShortRef && ref.Name().Short() == revision) || refName == revision {
 			if ref.Type() == plumbing.HashReference {
 				log.Debugf("revision '%s' resolved to '%s'", revision, hash)
 				return hash, nil

util/git/git.go

@@ -5,6 +5,8 @@ import (
 	"net/url"
 	"regexp"
 	"strings"
+
+	"github.com/go-git/go-git/v5/plumbing"
 )
 
 // EnsurePrefix idempotently ensures that a base string has a given prefix.
@@ -105,3 +107,15 @@ func TestRepo(repo string, creds Creds, insecure bool, enableLfs bool, proxy str
 	}
 	return nil
 }
+
+// IsShortRef determines if the supplied revision is a short ref (e.g. "master" instead of "refs/heads/master").
+// ref.Name().Short() is an expensive call to be performed in a loop over all refs in a repository, so we want to avoid calling it if we can determine up front that the supplied revision is not a short ref.
+// The intention is to optimize for a case where the full ref string is supplied, as comparing the full ref string is cheaper than calling Short() on every ref in the loop.
+// If the supplied revision is a short ref, we will compare it with the short version of each ref in the loop.
+// If the supplied revision is not a short ref, we will compare it with the full ref string in the loop, which is cheaper than calling Short() on every ref.
+// This performance optimization is based on the observation coming from larger repositories where the number of refs can be in the order of tens of thousands,
+// and we want to avoid calling Short() on every ref if we can determine up front that the supplied revision is not a short ref.
+func IsShortRef(revision string) bool {
+	refTentative := plumbing.NewReferenceFromStrings(revision, "dummyHash")
+	return refTentative.Name().Short() == revision
+}

util/git/git_test.go

@@ -722,3 +722,122 @@ func TestAnnotatedTagHandling(t *testing.T) {
 	// Verify tag exists in the list and points to a valid commit SHA
 	assert.Contains(t, refs.Tags, "v1.0.0", "Tag v1.0.0 should exist in refs")
 }
+
+func TestIsShortRef(t *testing.T) {
+	tests := []struct {
+		name     string
+		revision string
+		expected bool
+	}{
+		// Short refs (should return true)
+		{
+			name:     "short branch name",
+			revision: "master",
+			expected: true,
+		},
+		{
+			name:     "short branch name with slashes",
+			revision: "feature/my-feature",
+			expected: true,
+		},
+		{
+			name:     "short branch name with multiple slashes",
+			revision: "release/v1.0/bugfix",
+			expected: true,
+		},
+		{
+			name:     "short tag name",
+			revision: "v1.0.0",
+			expected: true,
+		},
+		{
+			name:     "short tag name without version prefix",
+			revision: "release-1.0",
+			expected: true,
+		},
+		{
+			name:     "simple branch name",
+			revision: "main",
+			expected: true,
+		},
+		{
+			name:     "branch with hyphens",
+			revision: "my-feature-branch",
+			expected: true,
+		},
+		{
+			name:     "branch with underscores",
+			revision: "my_feature_branch",
+			expected: true,
+		},
+		// Full refs (should return false)
+		{
+			name:     "full branch ref",
+			revision: "refs/heads/master",
+			expected: false,
+		},
+		{
+			name:     "full tag ref",
+			revision: "refs/tags/v1.0.0",
+			expected: false,
+		},
+		{
+			name:     "full remote branch ref",
+			revision: "refs/remotes/origin/master",
+			expected: false,
+		},
+		{
+			name:     "full branch ref with slashes in name",
+			revision: "refs/heads/feature/my-feature",
+			expected: false,
+		},
+		{
+			name:     "full tag ref with slashes in name",
+			revision: "refs/tags/release/v1.0.0",
+			expected: false,
+		},
+		{
+			name:     "full remote branch ref with slashes",
+			revision: "refs/remotes/origin/feature/branch",
+			expected: false,
+		},
+		{
+			name:     "full pull request ref",
+			revision: "refs/pull/123/head",
+			expected: false,
+		},
+		{
+			name:     "full notes ref",
+			revision: "refs/notes/commits",
+			expected: false,
+		},
+		// Special cases
+		{
+			name:     "HEAD",
+			revision: "HEAD",
+			expected: true,
+		},
+		{
+			name:     "commit SHA",
+			revision: "9d921f65f3c5373b682e2eb4b37afba6592e8f8b",
+			expected: true,
+		},
+		{
+			name:     "truncated commit SHA",
+			revision: "9d921f6",
+			expected: true,
+		},
+		{
+			name:     "empty string",
+			revision: "",
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsShortRef(tt.revision)
+			assert.Equal(t, tt.expected, result, "IsShortRef(%q) = %v, expected %v", tt.revision, result, tt.expected)
+		})
+	}
+}