fix(accesscontrol): Reduce memory usage in GroupScopesByActionContext (#112295)

Signed-off-by: Dave Henderson <dave.henderson@grafana.com>
2025-10-22 18:25:10 -04:00
parent b1ddba9767
commit fbc81d2fd0
3 changed files with 97 additions and 24 deletions
@@ -244,10 +244,59 @@ func GroupScopesByActionContext(ctx context.Context, permissions []Permission) m
 	))
 	defer span.End()

-	m := make(map[string][]string)
-	for i := range permissions {
-		m[permissions[i].Action] = append(m[permissions[i].Action], permissions[i].Scope)
+	// Note: this has been optimized to improve memory usage in large instances
+	// where there are lots of permissions. This isn't quite as fast as it can
+	// be, but we should prioritize memory over speed in this case.
+
+	if len(permissions) == 0 {
+		return make(map[string][]string)
 	}
+
+	// Use index-based approach with cached lookups for better performance
+	// First pass: assign and cache indices for each permission
+	actionIndex := make(map[string]int)
+	indices := make([]int, len(permissions))
+
+	for i := range permissions {
+		action := permissions[i].Action
+		if idx, ok := actionIndex[action]; ok {
+			indices[i] = idx
+		} else {
+			idx = len(actionIndex)
+			actionIndex[action] = idx
+			indices[i] = idx
+		}
+	}
+
+	// Count scopes per action using cached indices
+	actionCounts := make([]int, len(actionIndex))
+	for i := range indices {
+		actionCounts[indices[i]]++
+	}
+
+	// Preallocate slice array with exact capacities
+	scopes := make([][]string, len(actionCounts))
+	for i, count := range actionCounts {
+		scopes[i] = make([]string, 0, count)
+	}
+
+	// Second pass: append scopes using cached indices (no map lookups!)
+	for i := range permissions {
+		idx := indices[i]
+		scopes[idx] = append(scopes[idx], permissions[i].Scope)
+	}
+
+	// Build result map
+	m := make(map[string][]string, len(actionIndex))
+	for action, idx := range actionIndex {
+		m[action] = scopes[idx]
+	}
+
+	span.SetAttributes(
+		attribute.Int("unique_actions", len(actionIndex)),
+		attribute.Float64("avg_scopes_per_action", float64(len(permissions))/float64(len(actionIndex))),
+	)
+
 	return m
 }

@@ -155,27 +155,47 @@ func TestGroupScopesByActionContext(t *testing.T) {
 }

 func BenchmarkGroupScopesByAction(b *testing.B) {
-	// create a big list of permissions with a bunch of duplicates
-	permissions := []Permission{}
-	for i := 0; i < 100; i++ {
-		for j := 0; j < 500+i; j++ {
-			permissions = append(permissions, Permission{
-				Action: fmt.Sprintf("action:%d", i),
-				Scope:  fmt.Sprintf("scope:%d_%d", i, j),
-			})
-		}
-		// add duplicate scopes
-		for j := 0; j < 10; j++ {
-			permissions = append(permissions, Permission{
-				Action: fmt.Sprintf("action:%d", i),
-				Scope:  fmt.Sprintf("scope:%d_%d", i, 0),
-			})
-		}
+	testCases := []struct {
+		name         string
+		numActions   int
+		totalPerms   int
+		avgPerAction int
+	}{
+		{"small", 10, 1000, 100},
+		{"medium", 50, 10000, 200},
+		{"large", 100, 70000, 700},
 	}

-	b.ResetTimer()
+	for _, tc := range testCases {
+		b.Run(tc.name, func(b *testing.B) {
+			permissions := make([]Permission, 0, tc.totalPerms)

-	for i := 0; i < b.N; i++ {
-		GroupScopesByActionContext(context.Background(), permissions)
+			// Create realistic distribution with variance
+			// Some actions have more scopes than others
+			for i := 0; i < tc.numActions; i++ {
+				// Add variance: some actions get more scopes
+				scopeCount := tc.avgPerAction
+				if i%3 == 0 {
+					scopeCount = scopeCount * 2
+				} else if i%5 == 0 {
+					scopeCount = scopeCount / 2
+				}
+
+				for j := 0; j < scopeCount && len(permissions) < tc.totalPerms; j++ {
+					permissions = append(permissions, Permission{
+						Action: fmt.Sprintf("action:%d", i),
+						Scope:  fmt.Sprintf("scope:%d_%d", i, j),
+					})
+				}
+			}
+
+			b.ReportMetric(float64(len(permissions)), "permissions")
+			b.ReportMetric(float64(tc.numActions), "actions")
+			b.ResetTimer()
+
+			for b.Loop() {
+				GroupScopesByActionContext(context.Background(), permissions)
+			}
+		})
 	}
 }