Skip to content

Commit bbab475

Browse files
ultrotterGuido Trotter
andauthored
Remove duplicate slice during Silences query (#4696)
* Remove duplicate slice during Silences query If there are a lot of silences Silences.query, for queries without q.ids, first copies all their pointers to a slice, causing a lot of allocations, and then does it again to a new shorter slice filtered by the query. We avoid this by only adding to the result slice in case of a match. goos: linux goarch: amd64 pkg: github.com/prometheus/alertmanager/silence cpu: AMD EPYC Processor (with IBPB) │ before-query.txt │ after-query.txt │ │ sec/op │ sec/op vs base │ QueryParallel/100_silences,_1_goroutine-40 18.41µ ± ∞ ¹ 16.35µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_2_goroutines-40 17.06µ ± ∞ ¹ 17.46µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_4_goroutines-40 17.00µ ± ∞ ¹ 17.45µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_8_goroutines-40 17.31µ ± ∞ ¹ 18.05µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_1_goroutine-40 47.32µ ± ∞ ¹ 39.09µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_2_goroutines-40 45.99µ ± ∞ ¹ 37.03µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_4_goroutines-40 44.99µ ± ∞ ¹ 33.96µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_8_goroutines-40 47.82µ ± ∞ ¹ 36.76µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_1_goroutine-40 394.1µ ± ∞ ¹ 369.0µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_2_goroutines-40 395.0µ ± ∞ ¹ 375.9µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_4_goroutines-40 397.3µ ± ∞ ¹ 345.9µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_8_goroutines-40 387.7µ ± ∞ ¹ 363.0µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_10%_add_rate-40 154.5µ ± ∞ ¹ 145.6µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_1%_add_rate-40 72.80µ ± ∞ ¹ 68.34µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_0.1%_add_rate-40 53.81µ ± ∞ ¹ 40.63µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_1%_add_rate-40 540.4µ ± ∞ ¹ 562.1µ ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_0.1%_add_rate-40 427.6µ ± ∞ ¹ 351.1µ ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 89.27µ 80.02µ -10.37% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 │ before-query.txt │ after-query.txt │ │ B/op │ B/op vs base │ QueryParallel/100_silences,_1_goroutine-40 6.073Ki ± ∞ ¹ 3.884Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_2_goroutines-40 6.051Ki ± ∞ ¹ 3.897Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_4_goroutines-40 6.038Ki ± ∞ ¹ 3.899Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_8_goroutines-40 6.039Ki ± ∞ ¹ 3.916Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_1_goroutine-40 41.38Ki ± ∞ ¹ 24.16Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_2_goroutines-40 41.26Ki ± ∞ ¹ 24.12Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_4_goroutines-40 41.27Ki ± ∞ ¹ 24.12Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_8_goroutines-40 41.34Ki ± ∞ ¹ 24.13Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_1_goroutine-40 524.9Ki ± ∞ ¹ 221.7Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_2_goroutines-40 524.9Ki ± ∞ ¹ 221.7Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_4_goroutines-40 524.9Ki ± ∞ ¹ 221.7Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_8_goroutines-40 524.9Ki ± ∞ ¹ 221.7Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_10%_add_rate-40 273.5Ki ± ∞ ¹ 231.0Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_1%_add_rate-40 92.60Ki ± ∞ ¹ 75.46Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_0.1%_add_rate-40 57.66Ki ± ∞ ¹ 29.83Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_1%_add_rate-40 523.5Ki ± ∞ ¹ 232.9Ki ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_0.1%_add_rate-40 525.0Ki ± ∞ ¹ 222.3Ki ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 77.05Ki 42.64Ki -44.66% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 │ before-query.txt │ after-query.txt │ │ allocs/op │ allocs/op vs base │ QueryParallel/100_silences,_1_goroutine-40 42.00 ± ∞ ¹ 34.00 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_2_goroutines-40 42.00 ± ∞ ¹ 34.00 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_4_goroutines-40 42.00 ± ∞ ¹ 34.00 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/100_silences,_8_goroutines-40 42.00 ± ∞ ¹ 34.00 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_1_goroutine-40 138.0 ± ∞ ¹ 127.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_2_goroutines-40 138.0 ± ∞ ¹ 127.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_4_goroutines-40 139.0 ± ∞ ¹ 127.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/1000_silences,_8_goroutines-40 138.0 ± ∞ ¹ 127.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_1_goroutine-40 1.049k ± ∞ ¹ 1.031k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_2_goroutines-40 1.049k ± ∞ ¹ 1.031k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_4_goroutines-40 1.049k ± ∞ ¹ 1.031k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryParallel/10000_silences,_8_goroutines-40 1.049k ± ∞ ¹ 1.031k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_10%_add_rate-40 1.056k ± ∞ ¹ 1.044k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_1%_add_rate-40 326.0 ± ∞ ¹ 349.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/1000_initial_silences,_0.1%_add_rate-40 163.0 ± ∞ ¹ 155.0 ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_1%_add_rate-40 1.060k ± ∞ ¹ 1.045k ± ∞ ¹ ~ (p=1.000 n=1) ² QueryWithConcurrentAdds/10000_initial_silences,_0.1%_add_rate-40 1.051k ± ∞ ¹ 1.034k ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 255.8 237.3 -7.24% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 Signed-off-by: Guido Trotter <[email protected]> * change appendIfFiltersMatch to return the slice, instead of modifying it Signed-off-by: Guido Trotter <[email protected]> * Add comments explaining the logic of query and appendIfFiltersMatch Signed-off-by: Guido Trotter <[email protected]> * Add forgotten . in comment 😅 Signed-off-by: Guido Trotter <[email protected]> * Move appendIfFiltersMatch to be a private helper of query, instead of a module function or method of Silences Signed-off-by: Guido Trotter <[email protected]> --------- Signed-off-by: Guido Trotter <[email protected]> Co-authored-by: Guido Trotter <[email protected]>
1 parent 5c8df7c commit bbab475

File tree

1 file changed

+32
-21
lines changed

1 file changed

+32
-21
lines changed

silence/silence.go

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -834,45 +834,56 @@ func (s *Silences) CountState(states ...types.SilenceState) (int, error) {
834834
return len(sils), nil
835835
}
836836

837+
// query executes the given query and returns the resulting silences.
837838
func (s *Silences) query(q *query, now time.Time) ([]*pb.Silence, int, error) {
838-
// If we have no ID constraint, all silences are our base set. This and
839-
// the use of post-filter functions is the trivial solution for now.
840839
var res []*pb.Silence
840+
var err error
841841

842+
// appendIfFiltersMatch appends the given silence to the result set
843+
// if it matches all filters in the query. In case of a filter error, the error is returned.
844+
appendIfFiltersMatch := func(res []*pb.Silence, sil *pb.Silence) ([]*pb.Silence, error) {
845+
for _, f := range q.filters {
846+
matches, err := f(sil, s, now)
847+
// In case of error return it immediately and don't process further filters.
848+
if err != nil {
849+
return res, err
850+
}
851+
// If one filter doesn't match, return the result unchanged, immediately.
852+
if !matches {
853+
return res, nil
854+
}
855+
}
856+
// All filters matched, append the silence to the result.
857+
return append(res, cloneSilence(sil)), nil
858+
}
859+
860+
// Take a read lock on Silences: we can read but not modify the Silences struct.
842861
s.mtx.RLock()
843862
defer s.mtx.RUnlock()
844863

864+
// If we have IDs, only consider the silences with the given IDs, if they exist.
845865
if q.ids != nil {
846866
for _, id := range q.ids {
847-
if s, ok := s.st[id]; ok {
848-
res = append(res, s.Silence)
867+
if sil, ok := s.st[id]; ok {
868+
// append the silence to the results if it satisfies the query.
869+
res, err = appendIfFiltersMatch(res, sil.Silence)
870+
if err != nil {
871+
return nil, s.version, err
872+
}
849873
}
850874
}
851875
} else {
876+
// No IDs given, consider all silences.
852877
for _, sil := range s.st {
853-
res = append(res, sil.Silence)
854-
}
855-
}
856-
857-
var resf []*pb.Silence
858-
for _, sil := range res {
859-
remove := false
860-
for _, f := range q.filters {
861-
ok, err := f(sil, s, now)
878+
// append the silence to the results if it satisfies the query.
879+
res, err = appendIfFiltersMatch(res, sil.Silence)
862880
if err != nil {
863881
return nil, s.version, err
864882
}
865-
if !ok {
866-
remove = true
867-
break
868-
}
869-
}
870-
if !remove {
871-
resf = append(resf, cloneSilence(sil))
872883
}
873884
}
874885

875-
return resf, s.version, nil
886+
return res, s.version, nil
876887
}
877888

878889
// loadSnapshot loads a snapshot generated by Snapshot() into the state.

0 commit comments

Comments
 (0)