Skip to content

Remove duplicate slice during Silences query#4696

Merged
SuperQ merged 5 commits intoprometheus:mainfrom
ultrotter:deduplicatequeryallocation
Nov 10, 2025
Merged

Remove duplicate slice during Silences query#4696
SuperQ merged 5 commits intoprometheus:mainfrom
ultrotter:deduplicatequeryallocation

Conversation

@ultrotter
Copy link
Contributor

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. This creates savings for each queries, scaling with the number of silences.

@ultrotter ultrotter force-pushed the deduplicatequeryallocation branch 2 times, most recently from 7d97788 to dc26347 Compare November 5, 2025 17:37
return res, s.version, nil
}

func (s *Silences) appendIfFiltersMatch(res *[]*pb.Silence, sil *pb.Silence, q *query, now time.Time) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about the following?

  • define a new type for []*pb.Silence
  • add this method to that type
  • pass s as an argument to that method

for example:

Suggested change
func (s *Silences) appendIfFiltersMatch(res *[]*pb.Silence, sil *pb.Silence, q *query, now time.Time) error {
type result []*pb.Silence
func (r *result) appendOnMatch(s *Silences, sil *pb.Silence, q *query, now time.Time) error {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the concern is the mutation of the res argument, I think we could just write

Suggested change
func (s *Silences) appendIfFiltersMatch(res *[]*pb.Silence, sil *pb.Silence, q *query, now time.Time) error {
func (s *Silences) appendIfFiltersMatch(res []*pb.Silence, sil *pb.Silence, q *query, now time.Time) ([]*pb.Silence, error) {
for _, f := range q.filters {
ok, err := f(sil, s, now)
if err != nil {
return res, err
}
if !ok {
return res, nil
}
}
return append(res, cloneSilence(sil)), nil
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this would be called like

var err error
res, err = appendIfFiltersMatch(res, sil, now)
if err != nil {
  // whatever
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried going for @Spaceman1701's approach to avoid defining a new internal type with a one method just for this... Hopefully it's as clear!

@ultrotter ultrotter force-pushed the deduplicatequeryallocation branch 2 times, most recently from 447848c to 9383210 Compare November 5, 2025 21:51
Copy link
Contributor

@siavashs siavashs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, do you mind also adding inline docs to the logic so it is clear for people looking at the code later on?

@ultrotter ultrotter force-pushed the deduplicatequeryallocation branch 2 times, most recently from 8025900 to 9276773 Compare November 7, 2025 11:56
Guido Trotter added 5 commits November 7, 2025 12:08
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 <guido@hudson-trading.com>
Signed-off-by: Guido Trotter <guido@hudson-trading.com>
Signed-off-by: Guido Trotter <guido@hudson-trading.com>
Signed-off-by: Guido Trotter <guido@hudson-trading.com>
… a module function or method of Silences

Signed-off-by: Guido Trotter <guido@hudson-trading.com>
@ultrotter ultrotter force-pushed the deduplicatequeryallocation branch from 9276773 to 004e30e Compare November 7, 2025 17:08
Copy link
Contributor

@rajagopalanand rajagopalanand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SuperQ SuperQ merged commit bbab475 into prometheus:main Nov 10, 2025
7 checks passed
@SoloJacobs SoloJacobs mentioned this pull request Nov 24, 2025
@ultrotter ultrotter deleted the deduplicatequeryallocation branch December 3, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants