From f956fdf240cfcc52f0d9f73f4843227de1d4d1f6 Mon Sep 17 00:00:00 2001
From: Dan Winship <danwinship@redhat.com>
Date: Mon, 29 Apr 2024 16:17:30 -0400
Subject: [PATCH] Don't sort endpointSliceData objects

EndpointSliceCache cached EndpointSlices into endpointSliceData
objects, in part so it could sort the ports and addresses, so that if
those fields got reordered without otherwise changing, it would not
trigger an OnEndpointSliceUpdate().

However, the EndpointSlice controller and mirroring controller always
output the ports in the same order, and they never reorder the
addresses of an existing slice unless the set of addresses actually
changed. So in the normal case, sorting the data adds more work than
it saves.
---
 pkg/proxy/endpointslicecache.go      | 35 +---------------------------
 pkg/proxy/endpointslicecache_test.go |  4 ++--
 2 files changed, 3 insertions(+), 36 deletions(-)

diff --git a/pkg/proxy/endpointslicecache.go b/pkg/proxy/endpointslicecache.go
index fa98836284c..77bbc206931 100644
--- a/pkg/proxy/endpointslicecache.go
+++ b/pkg/proxy/endpointslicecache.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
-	"strings"
 	"sync"
 
 	v1 "k8s.io/api/core/v1"
@@ -113,15 +112,11 @@ func newEndpointSliceTracker() *endpointSliceTracker {
 // newEndpointSliceData generates endpointSliceData from an EndpointSlice.
 func newEndpointSliceData(endpointSlice *discovery.EndpointSlice, remove bool) *endpointSliceData {
 	esData := &endpointSliceData{
-		Ports:     make([]discovery.EndpointPort, len(endpointSlice.Ports)),
+		Ports:     endpointSlice.Ports,
 		Endpoints: []*endpointData{},
 		Remove:    remove,
 	}
 
-	// copy here to avoid mutating shared EndpointSlice object.
-	copy(esData.Ports, endpointSlice.Ports)
-	sort.Sort(byPort(esData.Ports))
-
 	if !remove {
 		for _, endpoint := range endpointSlice.Endpoints {
 			epData := &endpointData{
@@ -146,8 +141,6 @@ func newEndpointSliceData(endpointSlice *discovery.EndpointSlice, remove bool) *
 
 			esData.Endpoints = append(esData.Endpoints, epData)
 		}
-
-		sort.Sort(byAddress(esData.Endpoints))
 	}
 
 	return esData
@@ -373,19 +366,6 @@ func endpointSliceCacheKeys(endpointSlice *discovery.EndpointSlice) (types.Names
 	return types.NamespacedName{Namespace: endpointSlice.Namespace, Name: serviceName}, endpointSlice.Name, err
 }
 
-// byAddress helps sort endpointData
-type byAddress []*endpointData
-
-func (e byAddress) Len() int {
-	return len(e)
-}
-func (e byAddress) Swap(i, j int) {
-	e[i], e[j] = e[j], e[i]
-}
-func (e byAddress) Less(i, j int) bool {
-	return strings.Join(e[i].Addresses, ",") < strings.Join(e[j].Addresses, ",")
-}
-
 // byEndpoint helps sort endpoints by endpoint string.
 type byEndpoint []Endpoint
 
@@ -398,16 +378,3 @@ func (e byEndpoint) Swap(i, j int) {
 func (e byEndpoint) Less(i, j int) bool {
 	return e[i].String() < e[j].String()
 }
-
-// byPort helps sort EndpointSlice ports by port number
-type byPort []discovery.EndpointPort
-
-func (p byPort) Len() int {
-	return len(p)
-}
-func (p byPort) Swap(i, j int) {
-	p[i], p[j] = p[j], p[i]
-}
-func (p byPort) Less(i, j int) bool {
-	return *p[i].Port < *p[j].Port
-}
diff --git a/pkg/proxy/endpointslicecache_test.go b/pkg/proxy/endpointslicecache_test.go
index 8c36811a8e2..0aa032d8274 100644
--- a/pkg/proxy/endpointslicecache_test.go
+++ b/pkg/proxy/endpointslicecache_test.go
@@ -370,7 +370,7 @@ func TestEsDataChanged(t *testing.T) {
 				ObjectMeta: objMeta,
 				Ports:      []discovery.EndpointPort{port80, port443},
 			},
-			expectChanged: false,
+			expectChanged: true,
 		},
 		"port removed": {
 			cache: NewEndpointSliceCache("", v1.IPv4Protocol, nil, nil),
@@ -422,7 +422,7 @@ func TestEsDataChanged(t *testing.T) {
 				Ports:      []discovery.EndpointPort{port443},
 				Endpoints:  []discovery.Endpoint{endpoint2, endpoint1},
 			},
-			expectChanged: false,
+			expectChanged: true,
 		},
 		"identical with endpoint added": {
 			cache: NewEndpointSliceCache("", v1.IPv4Protocol, nil, nil),