argoproj / gitops-engine

Democratizing GitOps
https://pkg.go.dev/github.com/argoproj/gitops-engine?tab=subdirectories
Apache License 2.0
1.7k stars 264 forks source link

chore: avoid unnecessary deep copy #628

Closed crenshaw-dev closed 2 months ago

crenshaw-dev commented 2 months ago

I'm not convinced we need to deep copy these objects at all. But to be cautious, I think we can save some resources by only deep copying if we need to mutate the objects.

Here's the benchmark result:

before: Benchmark_threeWayMergePatch-16            23341             52004 ns/op           39070 B/op        810 allocs/op
after:  Benchmark_threeWayMergePatch-16            24838             48706 ns/op           37709 B/op        800 allocs/op

We save a little memory and CPU.

Here's the benchmark code:

func Benchmark_threeWayMergePatch(b *testing.B) {
    orig := []byte(`
apiVersion: v1
kind: Service
metadata:
  name: my-service
  namespace: default
`)
    config := []byte(`
apiVersion: v1
kind: Service
metadata:
  name: my-service
  namespace: default
`)
    live := []byte(`
apiVersion: v1
kind: Service
metadata:
  name: my-service
  namespace: default
`)

    origUnstructured := unstructured.Unstructured{}
    err := yaml.Unmarshal(orig, &origUnstructured)
    require.NoError(b, err)
    configUnstructured := unstructured.Unstructured{}
    err = yaml.Unmarshal(config, &configUnstructured)
    require.NoError(b, err)
    liveUnstructured := unstructured.Unstructured{}
    err = yaml.Unmarshal(live, &liveUnstructured)
    require.NoError(b, err)

    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        _, err := ThreeWayDiff(&origUnstructured, &configUnstructured, &liveUnstructured)
        require.NoError(b, err)
    }
}
sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

crenshaw-dev commented 2 months ago

Fixing the test failure makes the change more costly than the original. Leaving it as-is for now.