Skip to content

Commit

Permalink
Hopefully final optimization.
Browse files Browse the repository at this point in the history
benchstat -delta-test=none v6.txt v9.txt
name                                                           old time/op    new time/op    delta
CachedTGatherer_Update/Update_of_one_element_without_reset-12    13.1ms ± 0%     0.0ms ± 0%  -99.81%
CachedTGatherer_Update/Update_of_all_elements_with_reset-12       309ms ± 0%     282ms ± 0%   -8.77%
CachedTGatherer_Update/Gather-12                                  422ms ± 0%       0ms ± 0%  -99.95%

name                                                           old alloc/op   new alloc/op   delta
CachedTGatherer_Update/Update_of_one_element_without_reset-12      208B ± 0%      208B ± 0%    0.00%
CachedTGatherer_Update/Update_of_all_elements_with_reset-12      2.47kB ± 0%    1.67kB ± 0%  -32.56%
CachedTGatherer_Update/Gather-12                                 52.8kB ± 0%    24.6kB ± 0%  -53.34%

name                                                           old allocs/op  new allocs/op  delta
CachedTGatherer_Update/Update_of_one_element_without_reset-12      3.00 ± 0%      3.00 ± 0%    0.00%
CachedTGatherer_Update/Update_of_all_elements_with_reset-12        0.00           0.00         0.00%
CachedTGatherer_Update/Gather-12                                  1.00k ± 0%     0.00k ± 0%  -99.60%

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
  • Loading branch information
bwplotka committed Jan 26, 2022
1 parent 11bdfa3 commit 961f8ca
Showing 1 changed file with 29 additions and 25 deletions.
54 changes: 29 additions & 25 deletions prometheus/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,13 @@ var separatorByteSlice = []byte{model.SeparatorByte} // For convenient use with
type CachedTGatherer struct {
metricFamiliesByName map[string]*family
mMu sync.RWMutex

desiredTouchState bool
}

func NewCachedTGatherer() *CachedTGatherer {
return &CachedTGatherer{
desiredTouchState: true,
metricFamiliesByName: map[string]*family{},
}
}
Expand All @@ -57,30 +60,19 @@ type family struct {
*dto.MetricFamily

metricsByHash map[uint64]*metric
touched bool
touchState bool
needsRebuild bool
}

type metric struct {
*dto.Metric
touched bool
touchState bool
}

// normalizeMetricFamilies returns a MetricFamily slice with empty
// MetricFamilies pruned and the remaining MetricFamilies sorted by name within
// the slice, with the contained Metrics sorted within each MetricFamily.
func normalizeMetricFamilies(metricFamiliesByName map[string]*family) []*dto.MetricFamily {
// TODO(bwplotka): We could optimize this further by bookkeeping this slice in place.
for _, mf := range metricFamiliesByName {
if cap(mf.Metric) < len(mf.metricsByHash) {
mf.Metric = make([]*dto.Metric, 0, len(mf.metricsByHash))
}
mf.Metric = mf.Metric[:0]
for _, m := range mf.metricsByHash {
mf.Metric = append(mf.Metric, m.Metric)
}
sort.Sort(internal.MetricSorter(mf.Metric))
}

names := make([]string, 0, len(metricFamiliesByName))
for name, mf := range metricFamiliesByName {
if len(mf.Metric) > 0 {
Expand All @@ -98,9 +90,6 @@ func normalizeMetricFamilies(metricFamiliesByName map[string]*family) []*dto.Met
// Gather implements TransactionalGatherer interface.
func (c *CachedTGatherer) Gather() (_ []*dto.MetricFamily, done func(), err error) {
c.mMu.RLock()

// BenchmarkCachedTGatherer_Update shows, even for 1 million metrics among 1000 families
// this is efficient enough (~400ms and ~50 kB per op), no need to cache it for now.
return normalizeMetricFamilies(c.metricFamiliesByName), c.mMu.RUnlock, nil
}

Expand Down Expand Up @@ -180,7 +169,10 @@ func (c *CachedTGatherer) Update(reset bool, inserts []Insert, deletions []Key)
}
mf.Name = &inserts[i].FQName
}
mf.touched = true
if reset {
mf.touchState = c.desiredTouchState
}

mf.Type = inserts[i].ValueType.ToDTO()
mf.Help = &inserts[i].Help

Expand All @@ -200,8 +192,11 @@ func (c *CachedTGatherer) Update(reset bool, inserts []Insert, deletions []Key)
})
}
sort.Sort(internal.LabelPairSorter(m.Label))
mf.needsRebuild = true
}
if reset {
m.touchState = c.desiredTouchState
}
m.touched = true

switch inserts[i].ValueType {
case prometheus.CounterValue:
Expand Down Expand Up @@ -263,18 +258,19 @@ func (c *CachedTGatherer) Update(reset bool, inserts []Insert, deletions []Key)
continue
}

mf.needsRebuild = true
delete(mf.metricsByHash, hSum)
}

if reset {
// Trading off-time instead of memory allocated for otherwise needed replacement map.
for name, mf := range c.metricFamiliesByName {
if !mf.touched {
if mf.touchState != c.desiredTouchState {
delete(c.metricFamiliesByName, name)
continue
}
for hash, m := range mf.metricsByHash {
if !m.touched {
if m.touchState != c.desiredTouchState {
delete(mf.metricsByHash, hash)
continue
}
Expand All @@ -283,15 +279,23 @@ func (c *CachedTGatherer) Update(reset bool, inserts []Insert, deletions []Key)
delete(c.metricFamiliesByName, name)
}
}

// Avoid resetting state by flipping what we will expect in the next update.
c.desiredTouchState = !c.desiredTouchState
}

// TODO(bwplotka): Potentially move this only for reset, but then code would assume
// you either only update or only reset update. For now we can live with small overhead.
for _, mf := range c.metricFamiliesByName {
mf.touched = false
if !mf.needsRebuild {
continue
}

mf.Metric = mf.Metric[:0]
for _, m := range mf.metricsByHash {
m.touched = false
mf.Metric = append(mf.Metric, m.Metric)
}
sort.Sort(internal.MetricSorter(mf.Metric))

mf.needsRebuild = false
}

return errs.MaybeUnwrap()
Expand Down

0 comments on commit 961f8ca

Please sign in to comment.