From 0dc7036db3f9747a3b6ee43278f67f6591400ec0 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Sat, 28 Oct 2023 11:25:12 +0200 Subject: [PATCH] Optimising dependencies/dependents funcs to not produce new slices each request Signed-off-by: Danny Kopping --- rules/group.go | 26 +++++++++++--------------- rules/manager_test.go | 27 ++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/rules/group.go b/rules/group.go index 2be41c8015..568d606b58 100644 --- a/rules/group.go +++ b/rules/group.go @@ -892,35 +892,31 @@ func NewGroupMetrics(reg prometheus.Registerer) *Metrics { // output metric produced by another recording rule in its expression (i.e. as its "input"). type dependencyMap map[Rule][]Rule -// dependents returns all rules which use the output of the given rule as one of their inputs. -func (m dependencyMap) dependents(r Rule) []Rule { - if len(m) == 0 { - return nil - } - - return m[r] +// dependents returns the count of rules which use the output of the given rule as one of their inputs. +func (m dependencyMap) dependents(r Rule) int { + return len(m[r]) } -// dependencies returns all the rules on which the given rule is dependent for input. -func (m dependencyMap) dependencies(r Rule) []Rule { +// dependencies returns the count of rules on which the given rule is dependent for input. +func (m dependencyMap) dependencies(r Rule) int { if len(m) == 0 { - return nil + return 0 } - var parents []Rule - for parent, children := range m { + var count int + for _, children := range m { if len(children) == 0 { continue } for _, child := range children { if child == r { - parents = append(parents, parent) + count++ } } } - return parents + return count } func (m dependencyMap) isIndependent(r Rule) bool { @@ -928,7 +924,7 @@ func (m dependencyMap) isIndependent(r Rule) bool { return false } - return len(m.dependents(r)) == 0 && len(m.dependencies(r)) == 0 + return m.dependents(r)+m.dependencies(r) == 0 } // buildDependencyMap builds a data-structure which contains the relationships between rules within a group. diff --git a/rules/manager_test.go b/rules/manager_test.go index c2b23716f1..2a9b3a1d73 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -1419,20 +1419,37 @@ func TestDependencyMap(t *testing.T) { expr, err = parser.ParseExpr("user:requests:rate1m <= 0") require.NoError(t, err) rule2 := NewAlertingRule("ZeroRequests", expr, 0, 0, labels.Labels{}, labels.Labels{}, labels.EmptyLabels(), "", true, log.NewNopLogger()) + + expr, err = parser.ParseExpr("sum by (user) (rate(requests[5m]))") + require.NoError(t, err) + rule3 := NewRecordingRule("user:requests:rate5m", expr, labels.Labels{}) + + expr, err = parser.ParseExpr("increase(user:requests:rate1m[1h])") + require.NoError(t, err) + rule4 := NewRecordingRule("user:requests:increase1h", expr, labels.Labels{}) + group := NewGroup(GroupOptions{ Name: "rule_group", Interval: time.Second, - Rules: []Rule{rule, rule2}, + Rules: []Rule{rule, rule2, rule3, rule4}, Opts: opts, }) - require.Equal(t, []Rule{rule2}, group.dependencyMap.dependents(rule)) - require.Len(t, group.dependencyMap.dependencies(rule), 0) + require.Zero(t, group.dependencyMap.dependencies(rule)) + require.Equal(t, 2, group.dependencyMap.dependents(rule)) require.False(t, group.dependencyMap.isIndependent(rule)) - require.Len(t, group.dependencyMap.dependents(rule2), 0) - require.Equal(t, []Rule{rule}, group.dependencyMap.dependencies(rule2)) + require.Zero(t, group.dependencyMap.dependents(rule2)) + require.Equal(t, 1, group.dependencyMap.dependencies(rule2)) require.False(t, group.dependencyMap.isIndependent(rule2)) + + require.Zero(t, group.dependencyMap.dependents(rule3)) + require.Zero(t, group.dependencyMap.dependencies(rule3)) + require.True(t, group.dependencyMap.isIndependent(rule3)) + + require.Zero(t, group.dependencyMap.dependents(rule4)) + require.Equal(t, 1, group.dependencyMap.dependencies(rule4)) + require.False(t, group.dependencyMap.isIndependent(rule4)) } func TestNoDependency(t *testing.T) {