Skip to content

Commit ec3ffd2

Browse files
arkodgarkodg
authored
perf: improve mem allocation in TruncatePolicyAncestors (envoyproxy#6998)
* perf: improve mem allocation in TruncatePolicyAncestors Relates to envoyproxy#6919 Signed-off-by: Arko Dasgupta <[email protected]> * lint Signed-off-by: Arko Dasgupta <[email protected]> * also sort on ns before name Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]>
1 parent 6e3ccbc commit ec3ffd2

6 files changed

+49
-47
lines changed

‎internal/gatewayapi/status/policy.go‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
package status
77

88
import (
9-
"cmp"
10-
"fmt"
11-
"slices"
12-
"strings"
9+
"sort"
1310
"time"
1411

1512
gocmp "github.com/google/go-cmp/cmp"
@@ -20,10 +17,6 @@ import (
2017
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
2118
)
2219

23-
const (
24-
conditionMessageMaxLength = 32768
25-
)
26-
2720
type PolicyResolveError struct {
2821
Reason gwapiv1a2.PolicyConditionReason
2922
Message string
@@ -113,22 +106,31 @@ func TruncatePolicyAncestors(policyStatus *gwapiv1a2.PolicyStatus, controllerNam
113106
// we need to truncate policy ancestor status due to the item limit (max 16).
114107
// so we are choosing to preserve the 16 most important ancestors.
115108
// negative polarity (Conflicted, Overridden...) should be clearly indicated to the user.
116-
slices.SortStableFunc(policyStatus.Ancestors, func(a, b gwapiv1a2.PolicyAncestorStatus) int {
117-
if r := cmp.Compare(sortRankForPolicyAncestor(a), sortRankForPolicyAncestor(b)); r != 0 {
118-
return r
109+
sort.Slice(policyStatus.Ancestors, func(i, j int) bool {
110+
a, b := policyStatus.Ancestors[i], policyStatus.Ancestors[j]
111+
aRank := sortRankForPolicyAncestor(a)
112+
bRank := sortRankForPolicyAncestor(b)
113+
114+
if aRank != bRank {
115+
return aRank < bRank
116+
}
117+
// First compare by namespace, then by name
118+
aNamespace := ""
119+
if a.AncestorRef.Namespace != nil {
120+
aNamespace = string(*a.AncestorRef.Namespace)
121+
}
122+
bNamespace := ""
123+
if b.AncestorRef.Namespace != nil {
124+
bNamespace = string(*b.AncestorRef.Namespace)
119125
}
120-
return strings.Compare(string(a.AncestorRef.Name), string(b.AncestorRef.Name))
126+
127+
if aNamespace != bNamespace {
128+
return aNamespace < bNamespace
129+
}
130+
return string(a.AncestorRef.Name) < string(b.AncestorRef.Name)
121131
})
122132

123-
aggregated := make([]string, len(policyStatus.Ancestors)-16)
124-
for i, ancestor := range policyStatus.Ancestors[16:] {
125-
aggregated[i] = string(ancestor.AncestorRef.Name)
126-
}
127-
aggregatedMessage := fmt.Sprintf("Ancestors have been aggregated because the number of policy ancestors exceeds 16. "+
128-
"The aggregated ancestors: %s", strings.Join(aggregated, ", "))
129-
if len(aggregatedMessage) > conditionMessageMaxLength {
130-
aggregatedMessage = aggregatedMessage[:conditionMessageMaxLength]
131-
}
133+
aggregatedMessage := "Ancestors have been truncated because the number of policy ancestors exceeds 16."
132134

133135
policyStatus.Ancestors = policyStatus.Ancestors[:16]
134136
SetConditionForPolicyAncestor(policyStatus,

‎internal/gatewayapi/testdata/backendtlspolicy-status-conditions-truncated.out.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ backendTLSPolicies:
179179
status: "True"
180180
type: Accepted
181181
- lastTransitionTime: null
182-
message: 'Ancestors have been aggregated because the number of policy ancestors
183-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
182+
message: Ancestors have been truncated because the number of policy ancestors
183+
exceeds 16.
184184
reason: Aggregated
185185
status: "True"
186186
type: Aggregated
@@ -365,8 +365,8 @@ backendTLSPolicies:
365365
status: "False"
366366
type: Accepted
367367
- lastTransitionTime: null
368-
message: 'Ancestors have been aggregated because the number of policy ancestors
369-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
368+
message: Ancestors have been truncated because the number of policy ancestors
369+
exceeds 16.
370370
reason: Aggregated
371371
status: "True"
372372
type: Aggregated

‎internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions-truncated.out.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@ backendTrafficPolicies:
213213
status: "True"
214214
type: Accepted
215215
- lastTransitionTime: null
216-
message: 'Ancestors have been aggregated because the number of policy ancestors
217-
exceeds 16. The aggregated ancestors: gateway-7, gateway-8'
216+
message: Ancestors have been truncated because the number of policy ancestors
217+
exceeds 16.
218218
reason: Aggregated
219219
status: "True"
220220
type: Aggregated
@@ -440,8 +440,8 @@ backendTrafficPolicies:
440440
status: "False"
441441
type: Accepted
442442
- lastTransitionTime: null
443-
message: 'Ancestors have been aggregated because the number of policy ancestors
444-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
443+
message: Ancestors have been truncated because the number of policy ancestors
444+
exceeds 16.
445445
reason: Aggregated
446446
status: "True"
447447
type: Aggregated
@@ -651,8 +651,8 @@ backendTrafficPolicies:
651651
status: "True"
652652
type: Accepted
653653
- lastTransitionTime: null
654-
message: 'Ancestors have been aggregated because the number of policy ancestors
655-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
654+
message: Ancestors have been truncated because the number of policy ancestors
655+
exceeds 16.
656656
reason: Aggregated
657657
status: "True"
658658
type: Aggregated

‎internal/gatewayapi/testdata/clienttrafficpolicy-status-conditions-truncated.out.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,8 @@ envoyExtensionPolicies:
255255
status: "True"
256256
type: Accepted
257257
- lastTransitionTime: null
258-
message: 'Ancestors have been aggregated because the number of policy ancestors
259-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
258+
message: Ancestors have been truncated because the number of policy ancestors
259+
exceeds 16.
260260
reason: Aggregated
261261
status: "True"
262262
type: Aggregated
@@ -533,8 +533,8 @@ envoyExtensionPolicies:
533533
status: "False"
534534
type: Accepted
535535
- lastTransitionTime: null
536-
message: 'Ancestors have been aggregated because the number of policy ancestors
537-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
536+
message: Ancestors have been truncated because the number of policy ancestors
537+
exceeds 16.
538538
reason: Aggregated
539539
status: "True"
540540
type: Aggregated

‎internal/gatewayapi/testdata/envoyextensionpolicy-status-conditions-truncated.out.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@ envoyExtensionPolicies:
213213
status: "True"
214214
type: Accepted
215215
- lastTransitionTime: null
216-
message: 'Ancestors have been aggregated because the number of policy ancestors
217-
exceeds 16. The aggregated ancestors: gateway-7, gateway-8'
216+
message: Ancestors have been truncated because the number of policy ancestors
217+
exceeds 16.
218218
reason: Aggregated
219219
status: "True"
220220
type: Aggregated
@@ -440,8 +440,8 @@ envoyExtensionPolicies:
440440
status: "False"
441441
type: Accepted
442442
- lastTransitionTime: null
443-
message: 'Ancestors have been aggregated because the number of policy ancestors
444-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
443+
message: Ancestors have been truncated because the number of policy ancestors
444+
exceeds 16.
445445
reason: Aggregated
446446
status: "True"
447447
type: Aggregated
@@ -651,8 +651,8 @@ envoyExtensionPolicies:
651651
status: "True"
652652
type: Accepted
653653
- lastTransitionTime: null
654-
message: 'Ancestors have been aggregated because the number of policy ancestors
655-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
654+
message: Ancestors have been truncated because the number of policy ancestors
655+
exceeds 16.
656656
reason: Aggregated
657657
status: "True"
658658
type: Aggregated

‎internal/gatewayapi/testdata/securitypolicy-status-conditions-truncated.out.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,8 +1960,8 @@ securityPolicies:
19601960
status: "True"
19611961
type: Accepted
19621962
- lastTransitionTime: null
1963-
message: 'Ancestors have been aggregated because the number of policy ancestors
1964-
exceeds 16. The aggregated ancestors: gateway-7, gateway-8'
1963+
message: Ancestors have been truncated because the number of policy ancestors
1964+
exceeds 16.
19651965
reason: Aggregated
19661966
status: "True"
19671967
type: Aggregated
@@ -2187,8 +2187,8 @@ securityPolicies:
21872187
status: "False"
21882188
type: Accepted
21892189
- lastTransitionTime: null
2190-
message: 'Ancestors have been aggregated because the number of policy ancestors
2191-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
2190+
message: Ancestors have been truncated because the number of policy ancestors
2191+
exceeds 16.
21922192
reason: Aggregated
21932193
status: "True"
21942194
type: Aggregated
@@ -2398,8 +2398,8 @@ securityPolicies:
23982398
status: "True"
23992399
type: Accepted
24002400
- lastTransitionTime: null
2401-
message: 'Ancestors have been aggregated because the number of policy ancestors
2402-
exceeds 16. The aggregated ancestors: gateway-8, gateway-9'
2401+
message: Ancestors have been truncated because the number of policy ancestors
2402+
exceeds 16.
24032403
reason: Aggregated
24042404
status: "True"
24052405
type: Aggregated

0 commit comments

Comments
 (0)