fix: reduce memory consumption by eliminating redundant string copies (#965)

* fix: reduce memory consumption by eliminating redundant string copies

Changes manifest.Parse and parseContent to accept []byte instead of
string, avoiding a full copy of the entire manifest when converting from
the raw helm output bytes. Raw byte slices are released after parsing
to allow GC reclamation before diff computation begins.

Also adds a content length ratio check in rename detection to skip
obviously different resources before computing expensive full diffs.

Closes #915

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove ineffectual nil assignments flagged by linter

The releaseManifest and installManifest nil assignments were flagged
by ineffassign since the variables are not read afterward. The main
memory optimization comes from the []byte signature change.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments for reduce memory consumption

- Replace append copy in Parse with io.MultiReader to avoid
 manifest copy
- Extract length ratio constants for named constants
  Add unit tests for length ratio heuristic
- Restructure cmd flows to parse into locals before diff

 release raw bytes earlier

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address all PR review comments

- Nil out raw manifest byte slices after parsing in upgrade/release/rollback/revision
  to allow GC reclamation before diff computation
- Fix unkeyed Options struct literals in diff_test.go (4 locations) to use
  keyed field names for resilience against struct changes
- Skip length-ratio filter for Secrets in contentSearch() since Secret raw
  content lengths before redaction/decoding are unreliable rename predictors
- Extract empty-content check before the Secret kind branch (DRY)

Agent-Logs-Url: https://github.com/databus23/helm-diff/sessions/c2d2d0b3-3840-48a8-89f0-4b6913c83973

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: suppress ineffassign warnings on GC-hint nil assignments

Add //nolint:ineffassign comments on the nil assignments that are
intentionally written to allow GC to reclaim raw manifest bytes
before the diff computation begins. The ineffassign linter correctly
identifies these as never-read-after-assignment, but the intent is
a deliberate GC hint for long-running diff operations.

Agent-Logs-Url: https://github.com/databus23/helm-diff/sessions/aa2df3f5-57bb-4939-ac45-3f3612deac60

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Author: yxxhero

cmd/release.go

@@ -109,9 +109,14 @@ func (d *release) differentiateHelm3() error {
 	}
 
 	if releaseChart1 == releaseChart2 {
+		oldSpecs := manifest.Parse(releaseResponse1, namespace1, d.normalizeManifests, excludes...)
+		newSpecs := manifest.Parse(releaseResponse2, namespace2, d.normalizeManifests, excludes...)
+		releaseResponse1 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+		releaseResponse2 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+
 		seenAnyChanges := diff.Releases(
-			manifest.Parse(string(releaseResponse1), namespace1, d.normalizeManifests, excludes...),
-			manifest.Parse(string(releaseResponse2), namespace2, d.normalizeManifests, excludes...),
+			oldSpecs,
+			newSpecs,
 			&d.Options,
 			os.Stdout)
 

cmd/revision.go

@@ -99,9 +99,14 @@ func (d *revision) differentiateHelm3() error {
 			return err
 		}
 
+		oldSpecs := manifest.Parse(revisionResponse, namespace, d.normalizeManifests, excludes...)
+		newSpecs := manifest.Parse(releaseResponse, namespace, d.normalizeManifests, excludes...)
+		revisionResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+		releaseResponse = nil  //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+
 		diff.Manifests(
-			manifest.Parse(string(revisionResponse), namespace, d.normalizeManifests, excludes...),
-			manifest.Parse(string(releaseResponse), namespace, d.normalizeManifests, excludes...),
+			oldSpecs,
+			newSpecs,
 			&d.Options,
 			os.Stdout)
 
@@ -122,9 +127,14 @@ func (d *revision) differentiateHelm3() error {
 			return err
 		}
 
+		oldSpecs := manifest.Parse(revisionResponse1, namespace, d.normalizeManifests, excludes...)
+		newSpecs := manifest.Parse(revisionResponse2, namespace, d.normalizeManifests, excludes...)
+		revisionResponse1 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+		revisionResponse2 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+
 		seenAnyChanges := diff.Manifests(
-			manifest.Parse(string(revisionResponse1), namespace, d.normalizeManifests, excludes...),
-			manifest.Parse(string(revisionResponse2), namespace, d.normalizeManifests, excludes...),
+			oldSpecs,
+			newSpecs,
 			&d.Options,
 			os.Stdout)
 

cmd/rollback.go

@@ -90,9 +90,14 @@ func (d *rollback) backcastHelm3() error {
 	}
 
 	// create a diff between the current manifest and the version of the manifest that a user is intended to rollback
+	oldSpecs := manifest.Parse(releaseResponse, namespace, d.normalizeManifests, excludes...)
+	newSpecs := manifest.Parse(revisionResponse, namespace, d.normalizeManifests, excludes...)
+	releaseResponse = nil  //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+	revisionResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
+
 	seenAnyChanges := diff.Manifests(
-		manifest.Parse(string(releaseResponse), namespace, d.normalizeManifests, excludes...),
-		manifest.Parse(string(revisionResponse), namespace, d.normalizeManifests, excludes...),
+		oldSpecs,
+		newSpecs,
 		&d.Options,
 		os.Stdout)
 

cmd/upgrade.go

@@ -327,11 +327,12 @@ func (d *diffCmd) runHelm3() error {
 			releaseManifest = append(releaseManifest, hooks...)
 		}
 		if d.includeTests {
-			currentSpecs = manifest.Parse(string(releaseManifest), d.namespace, d.normalizeManifests)
+			currentSpecs = manifest.Parse(releaseManifest, d.namespace, d.normalizeManifests)
 		} else {
-			currentSpecs = manifest.Parse(string(releaseManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
+			currentSpecs = manifest.Parse(releaseManifest, d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
 		}
 	}
+	releaseManifest = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
 
 	var newOwnedReleases map[string]diff.OwnershipDiff
 	if d.takeOwnership {
@@ -347,10 +348,11 @@ func (d *diffCmd) runHelm3() error {
 
 	var newSpecs map[string]*manifest.MappingResult
 	if d.includeTests {
-		newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests)
+		newSpecs = manifest.Parse(installManifest, d.namespace, d.normalizeManifests)
 	} else {
-		newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
+		newSpecs = manifest.Parse(installManifest, d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
 	}
+	installManifest = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation
 
 	seenAnyChanges := diff.ManifestsOwnership(currentSpecs, newSpecs, newOwnedReleases, &d.Options, os.Stdout)
 

diff/diff.go

@@ -181,6 +181,11 @@ func actualChanges(diff []difflib.DiffRecord) int {
 	return changes
 }
 
+const (
+	renameDetectionMinLengthRatio float32 = 0.1
+	renameDetectionMaxLengthRatio float32 = 10.0
+)
+
 func contentSearch(report *Report, possiblyRemoved []string, oldIndex map[string]*manifest.MappingResult, possiblyAdded []string, newIndex map[string]*manifest.MappingResult, options *Options) ([]string, []string) {
 	if options.FindRenames <= 0 {
 		return possiblyRemoved, possiblyAdded
@@ -198,6 +203,21 @@ func contentSearch(report *Report, possiblyRemoved []string, oldIndex map[string
 				continue
 			}
 
+			oldLen := len(oldContent.Content)
+			newLen := len(newContent.Content)
+			if oldLen == 0 || newLen == 0 {
+				continue
+			}
+			// Skip the length-ratio filter for Secrets: their raw content length can
+			// differ greatly from the post-processed (redacted/decoded) length, so the
+			// ratio would be an unreliable predictor of content similarity.
+			if oldContent.Kind != "Secret" {
+				ratio := float32(oldLen) / float32(newLen)
+				if ratio < renameDetectionMinLengthRatio || ratio > renameDetectionMaxLengthRatio {
+					continue
+				}
+			}
+
 			switch {
 			case options.ShowSecretsDecoded:
 				decodeSecrets(oldContent, newContent)
@@ -208,7 +228,7 @@ func contentSearch(report *Report, possiblyRemoved []string, oldIndex map[string
 			diff := diffMappingResults(oldContent, newContent, options.StripTrailingCR)
 			delta := actualChanges(diff)
 			if delta == 0 || len(diff) == 0 {
-				continue // Should never happen, but better safe than sorry
+				continue
 			}
 			fraction := float32(delta) / float32(len(diff))
 			if fraction > 0 && fraction < smallestFraction {

diff/diff_test.go

@@ -617,8 +617,8 @@ spec:
       - name: app
         image: demo:v2
 `
-	oldIndex := manifest.Parse(oldManifest, "prod", true)
-	newIndex := manifest.Parse(newManifest, "prod", true)
+	oldIndex := manifest.Parse([]byte(oldManifest), "prod", true)
+	newIndex := manifest.Parse([]byte(newManifest), "prod", true)
 
 	var buf bytes.Buffer
 	changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -656,7 +656,7 @@ metadata:
   namespace: ops
 spec: {}
 `
-	newIndex := manifest.Parse(newManifest, "ops", true)
+	newIndex := manifest.Parse([]byte(newManifest), "ops", true)
 
 	var buf bytes.Buffer
 	changed := Manifests(map[string]*manifest.MappingResult{}, newIndex, opts, &buf)
@@ -702,8 +702,8 @@ metadata:
 data:
   password: Zm9v
 `
-	oldIndex := manifest.Parse(oldManifest, "default", true)
-	newIndex := manifest.Parse(newManifest, "default", true)
+	oldIndex := manifest.Parse([]byte(oldManifest), "default", true)
+	newIndex := manifest.Parse([]byte(newManifest), "default", true)
 
 	var buf bytes.Buffer
 	changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -822,8 +822,8 @@ metadata:
   name: test
   namespace: default
 `
-		oldIndex := manifest.Parse(emptyManifest, "default", true)
-		newIndex := manifest.Parse(validManifest, "default", true)
+		oldIndex := manifest.Parse([]byte(emptyManifest), "default", true)
+		newIndex := manifest.Parse([]byte(validManifest), "default", true)
 
 		var buf bytes.Buffer
 		changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -846,8 +846,8 @@ metadata:
   name: test
   namespace: default
 `
-		oldIndex := manifest.Parse(nullManifest, "default", true)
-		newIndex := manifest.Parse(validManifest, "default", true)
+		oldIndex := manifest.Parse([]byte(nullManifest), "default", true)
+		newIndex := manifest.Parse([]byte(validManifest), "default", true)
 
 		var buf bytes.Buffer
 		changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -890,8 +890,8 @@ data:
       deeply:
         value: new
 `
-		oldIndex := manifest.Parse(oldManifest, "default", true)
-		newIndex := manifest.Parse(newManifest, "default", true)
+		oldIndex := manifest.Parse([]byte(oldManifest), "default", true)
+		newIndex := manifest.Parse([]byte(newManifest), "default", true)
 
 		var buf bytes.Buffer
 		changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -944,8 +944,8 @@ spec:
         - name: KEY2
           value: val2
 `
-		oldIndex := manifest.Parse(oldManifest, "prod", true)
-		newIndex := manifest.Parse(newManifest, "prod", true)
+		oldIndex := manifest.Parse([]byte(oldManifest), "prod", true)
+		newIndex := manifest.Parse([]byte(newManifest), "prod", true)
 
 		var buf bytes.Buffer
 		changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -967,8 +967,8 @@ spec:
 		emptyManifest1 := ``
 		emptyManifest2 := ``
 
-		oldIndex := manifest.Parse(emptyManifest1, "default", true)
-		newIndex := manifest.Parse(emptyManifest2, "default", true)
+		oldIndex := manifest.Parse([]byte(emptyManifest1), "default", true)
+		newIndex := manifest.Parse([]byte(emptyManifest2), "default", true)
 
 		var buf bytes.Buffer
 		changed := Manifests(oldIndex, newIndex, opts, &buf)
@@ -1593,3 +1593,128 @@ data:
 		require.Contains(t, new.Content, "key1: '++++++++ # (6 bytes)'")
 	})
 }
+
+func TestRenameDetectionLengthRatio(t *testing.T) {
+	ansi.DisableColors(true)
+
+	makeSpec := func(name string, content string) map[string]*manifest.MappingResult {
+		return map[string]*manifest.MappingResult{
+			name: {
+				Name:    name,
+				Kind:    "Deployment",
+				Content: content,
+			},
+		}
+	}
+
+	shortContent := `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: short
+spec:
+  replicas: 1
+`
+
+	shortContentRenamed := `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: short-renamed
+spec:
+  replicas: 1
+`
+
+	longContent := `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: very-long
+spec:
+  replicas: 1
+  template:
+    spec:
+      containers:
+      - name: app
+        image: myapp:v1
+        ports:
+        - containerPort: 8080
+        env:
+        - name: VAR1
+          value: "hello"
+        - name: VAR2
+          value: "world"
+        - name: VAR3
+          value: "foo"
+        - name: VAR4
+          value: "bar"
+        - name: VAR5
+          value: "baz"
+        resources:
+          limits:
+            cpu: "1"
+            memory: "1Gi"
+`
+
+	t.Run("similar length detects rename", func(t *testing.T) {
+		var buf bytes.Buffer
+		opts := &Options{OutputFormat: "diff", OutputContext: 10, ShowSecrets: true, FindRenames: 0.5}
+
+		oldSpec := makeSpec("default, short, Deployment (apps)", shortContent)
+		newSpec := makeSpec("default, short-renamed, Deployment (apps)", shortContentRenamed)
+
+		changed := Manifests(oldSpec, newSpec, opts, &buf)
+		require.True(t, changed)
+		require.Contains(t, buf.String(), "default, short, Deployment (apps) has changed")
+	})
+
+	t.Run("very different length skips rename", func(t *testing.T) {
+		var buf bytes.Buffer
+		opts := &Options{OutputFormat: "diff", OutputContext: 10, ShowSecrets: true, FindRenames: 0.5}
+
+		oldSpec := makeSpec("default, short, Deployment (apps)", shortContent)
+		newSpec := makeSpec("default, very-long, Deployment (apps)", longContent)
+
+		changed := Manifests(oldSpec, newSpec, opts, &buf)
+		require.True(t, changed)
+		require.Contains(t, buf.String(), "default, short, Deployment (apps) has been removed")
+		require.Contains(t, buf.String(), "default, very-long, Deployment (apps) has been added")
+	})
+
+	t.Run("empty content skipped", func(t *testing.T) {
+		var buf bytes.Buffer
+		opts := &Options{OutputFormat: "diff", OutputContext: 10, ShowSecrets: true, FindRenames: 0.5}
+
+		oldSpec := map[string]*manifest.MappingResult{
+			"default, empty, Deployment (apps)": {
+				Name:    "default, empty, Deployment (apps)",
+				Kind:    "Deployment",
+				Content: "",
+			},
+		}
+		newSpec := makeSpec("default, short-renamed, Deployment (apps)", shortContentRenamed)
+
+		changed := Manifests(oldSpec, newSpec, opts, &buf)
+		require.True(t, changed)
+		require.Contains(t, buf.String(), "has been added")
+	})
+
+	t.Run("different kind skipped", func(t *testing.T) {
+		var buf bytes.Buffer
+		opts := &Options{OutputFormat: "diff", OutputContext: 10, ShowSecrets: true, FindRenames: 0.5}
+
+		oldSpec := map[string]*manifest.MappingResult{
+			"default, svc, Service (v1)": {
+				Name:    "default, svc, Service (v1)",
+				Kind:    "Service",
+				Content: shortContent,
+			},
+		}
+		newSpec := makeSpec("default, svc-renamed, Deployment (apps)", shortContentRenamed)
+
+		changed := Manifests(oldSpec, newSpec, opts, &buf)
+		require.True(t, changed)
+		require.Contains(t, buf.String(), "has been removed")
+		require.Contains(t, buf.String(), "has been added")
+	})
+}