perf: remove expensive reflection from xDS hot path (#14934)

Replaces the reflection-based implementation of proxycfg's
ConfigSnapshot.Clone with code generated by deep-copy.

While load testing server-based xDS (for consul-dataplane) we discovered
this method is extremely expensive. The ConfigSnapshot struct, directly
or indirectly, contains a copy of many of the structs in the agent/structs
package, which creates a large graph for copystructure.Copy to traverse
at runtime, on every proxy reconfiguration.

.changelog/14934.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+xds: Removed a bottleneck in Envoy config generation, enabling a higher number of dataplanes per server
+```

.circleci/config.yml

@@ -263,6 +263,26 @@ jobs:
           name: "Protobuf Lint"
           command: make proto-lint
 
+  check-generated-deep-copy:
+    docker:
+      - image: *GOLANG_IMAGE
+    environment:
+      <<: *ENVIRONMENT
+      # tput complains if this isn't set to something.
+      TERM: ansi
+    steps:
+      - checkout
+      - run:
+          name: Install deep-copy
+          command: make codegen-tools
+      - run:
+          command: make --always-make deep-copy
+      - run: |
+          if ! git diff --exit-code; then
+            echo "Generated code was not updated correctly"
+            exit 1
+          fi
+
   go-test-arm64:
     machine:
       image: *UBUNTU_CI_IMAGE
@@ -1036,6 +1056,7 @@ workflows:
                 - /^docs\/.*/
                 - /^ui\/.*/
       - check-generated-protobuf: *filter-ignore-non-go-branches
+      - check-generated-deep-copy: *filter-ignore-non-go-branches
       - lint-enums: *filter-ignore-non-go-branches
       - lint-consul-retry: *filter-ignore-non-go-branches
       - lint: *filter-ignore-non-go-branches

GNUmakefile

@@ -13,6 +13,7 @@ BUF_VERSION='v1.4.0'
 PROTOC_GEN_GO_GRPC_VERSION="v1.2.0"
 MOG_VERSION='v0.3.0'
 PROTOC_GO_INJECT_TAG_VERSION='v1.3.0'
+DEEP_COPY_VERSION='bc3f5aa5735d8a54961580a3a24422c308c831c2'
 
 MOCKED_PB_DIRS= pbdns
 
@@ -321,6 +322,16 @@ lint-tools:
 proto-tools:
 	@$(SHELL) $(CURDIR)/build-support/scripts/devtools.sh -protobuf
 
+.PHONY: codegen-tools
+codegen-tools:
+	@$(SHELL) $(CURDIR)/build-support/scripts/devtools.sh -codegen
+
+.PHONY: deep-copy
+deep-copy:
+	@$(SHELL) $(CURDIR)/agent/structs/deep-copy.sh
+	@$(SHELL) $(CURDIR)/proto/pbpeering/deep-copy.sh
+	@$(SHELL) $(CURDIR)/agent/proxycfg/deep-copy.sh
+
 version:
 	@echo -n "Version:                    "
 	@$(SHELL) $(CURDIR)/build-support/scripts/version.sh

agent/proxycfg/deep-copy.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+readonly PACKAGE_DIR="$(dirname "${BASH_SOURCE[0]}")"
+cd $PACKAGE_DIR
+
+# Uses: https://github.com/globusdigital/deep-copy
+deep-copy -pointer-receiver \
+  -o ./proxycfg.deepcopy.go \
+  -type ConfigSnapshot \
+  -type ConfigSnapshotUpstreams \
+  -type PeerServersValue \
+  -type PeeringServiceValue \
+  -type configSnapshotConnectProxy \
+  -type configSnapshotIngressGateway \
+  -type configSnapshotMeshGateway \
+  -type configSnapshotTerminatingGateway \
+  ./

agent/proxycfg/internal/watch/watchmap.go

@@ -2,11 +2,18 @@ package watch
 
 import "context"
 
+// DeepCopyable describes a type that implements the DeepCopy
+// method to get a copy of itself that is safe to pass around
+// without worrying about receivers modifying the original.
+type DeepCopyable[T any] interface {
+	DeepCopy() T
+}
+
 // Map safely stores and retrieves values by validating that
 // there is a live watch for a key. InitWatch must be called
 // to associate a key with its cancel function before any
 // Set's are called.
-type Map[K comparable, V any] struct {
+type Map[K comparable, V DeepCopyable[V]] struct {
 	M map[K]watchedVal[V]
 }
 
@@ -20,10 +27,26 @@ type watchedVal[V any] struct {
 	cancel context.CancelFunc
 }
 
-func NewMap[K comparable, V any]() Map[K, V] {
+func NewMap[K comparable, V DeepCopyable[V]]() Map[K, V] {
 	return Map[K, V]{M: make(map[K]watchedVal[V])}
 }
 
+// DeepCopy returns a copy of the Map that is safe to be passed
+// around without worrying about receivers modifying the original
+// or canceling its watches.
+func (m Map[K, V]) DeepCopy() Map[K, V] {
+	dup := make(map[K]watchedVal[V], len(m.M))
+	for k, v := range m.M {
+		var val *V
+		if v.Val != nil {
+			dc := (*v.Val).DeepCopy()
+			val = &dc
+		}
+		dup[k] = watchedVal[V]{Val: val}
+	}
+	return Map[K, V]{M: dup}
+}
+
 // InitWatch associates a cancel function with a key,
 // allowing Set to be called for the key. The cancel
 // function is allowed to be nil.

agent/proxycfg/internal/watch/watchmap_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestMap(t *testing.T) {
-	m := NewMap[string, string]()
+	m := NewMap[string, testVal]()
 
 	// Set without init is a no-op
 	{
@@ -51,7 +51,7 @@ func TestMap(t *testing.T) {
 	{
 		got, ok := m.Get("hello")
 		require.True(t, ok)
-		require.Equal(t, "world", got)
+		require.Equal(t, "world", string(got))
 	}
 
 	// CancelWatch successful
@@ -76,15 +76,11 @@ func TestMap(t *testing.T) {
 }
 
 func TestMap_ForEach(t *testing.T) {
-	type testType struct {
-		s string
-	}
-
-	m := NewMap[string, any]()
-	inputs := map[string]any{
-		"hello": 13,
-		"foo":   struct{}{},
-		"bar":   &testType{s: "wow"},
+	m := NewMap[string, testVal]()
+	inputs := map[string]testVal{
+		"hello": "world",
+		"foo":   "bar",
+		"baz":   "bat",
 	}
 	for k, v := range inputs {
 		m.InitWatch(k, nil)
@@ -114,15 +110,11 @@ func TestMap_ForEach(t *testing.T) {
 }
 
 func TestMap_ForEachE(t *testing.T) {
-	type testType struct {
-		s string
-	}
-
-	m := NewMap[string, any]()
-	inputs := map[string]any{
-		"hello": 13,
-		"foo":   struct{}{},
-		"bar":   &testType{s: "wow"},
+	m := NewMap[string, testVal]()
+	inputs := map[string]testVal{
+		"hello": "world",
+		"foo":   "bar",
+		"baz":   "bat",
 	}
 	for k, v := range inputs {
 		m.InitWatch(k, nil)
@@ -152,3 +144,27 @@ func TestMap_ForEachE(t *testing.T) {
 		require.Errorf(t, err, "boo")
 	}
 }
+
+func TestMap_DeepCopy(t *testing.T) {
+	orig := NewMap[string, testVal]()
+	inputs := map[string]testVal{
+		"hello": "world",
+		"foo":   "bar",
+		"baz":   "bat",
+	}
+	for k, v := range inputs {
+		orig.InitWatch(k, nil)
+		orig.Set(k, v)
+	}
+	require.Equal(t, 3, orig.Len())
+
+	clone := orig.DeepCopy()
+	require.Equal(t, 3, clone.Len())
+
+	orig.CancelWatch("hello")
+	require.NotEqual(t, orig.Len(), clone.Len())
+}
+
+type testVal string
+
+func (tv testVal) DeepCopy() testVal { return tv }

agent/proxycfg/manager_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/mitchellh/copystructure"
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/acl"
@@ -326,17 +325,14 @@ func TestManager_BasicLifecycle(t *testing.T) {
 			dataSources.ConfigEntry.Set(meshConfigReq, &structs.ConfigEntryResponse{Entry: nil})
 			tt.setup(t, dataSources)
 
-			expectSnapCopy, err := tt.expectSnap.Clone()
-			require.NoError(t, err)
-
-			webProxyCopy, err := copystructure.Copy(webProxy)
-			require.NoError(t, err)
+			expectSnapCopy := tt.expectSnap.Clone()
+			webProxyCopy := webProxy.DeepCopy()
 
 			testManager_BasicLifecycle(t,
 				dataSources,
 				rootsReq, leafReq,
 				roots,
-				webProxyCopy.(*structs.NodeService),
+				webProxyCopy,
 				expectSnapCopy,
 			)
 		})