From ce153e3fffde388712629d205663dd4af55013db Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sat, 8 Jul 2023 12:45:56 +0000 Subject: [PATCH] Replace sort.Sort with faster slices.SortFunc The generic version is more efficient. Signed-off-by: Bryan Boreham --- promql/quantile.go | 10 +++++----- storage/remote/codec.go | 11 ++++------- tsdb/head_read.go | 2 +- tsdb/index/index.go | 8 +------- tsdb/ooo_head_read.go | 31 +++++++++++-------------------- tsdb/ooo_head_read_test.go | 7 ++++--- util/stats/timer.go | 33 ++++++++------------------------- web/federate.go | 19 ++++++------------- 8 files changed, 40 insertions(+), 81 deletions(-) diff --git a/promql/quantile.go b/promql/quantile.go index 78d0bbaf0..d80345e81 100644 --- a/promql/quantile.go +++ b/promql/quantile.go @@ -17,6 +17,8 @@ import ( "math" "sort" + "golang.org/x/exp/slices" + "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" ) @@ -38,10 +40,6 @@ type bucket struct { // buckets implements sort.Interface. type buckets []bucket -func (b buckets) Len() int { return len(b) } -func (b buckets) Swap(i, j int) { b[i], b[j] = b[j], b[i] } -func (b buckets) Less(i, j int) bool { return b[i].upperBound < b[j].upperBound } - type metricWithBuckets struct { metric labels.Labels buckets buckets @@ -83,7 +81,9 @@ func bucketQuantile(q float64, buckets buckets) float64 { if q > 1 { return math.Inf(+1) } - sort.Sort(buckets) + slices.SortFunc(buckets, func(a, b bucket) bool { + return a.upperBound < b.upperBound + }) if !math.IsInf(buckets[len(buckets)-1].upperBound, +1) { return math.NaN() } diff --git a/storage/remote/codec.go b/storage/remote/codec.go index 6a58ec4ac..3f426204e 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -26,6 +26,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/golang/snappy" "github.com/prometheus/common/model" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" @@ -178,7 +179,9 @@ func FromQueryResult(sortSeries bool, res *prompb.QueryResult) storage.SeriesSet } if sortSeries { - sort.Sort(byLabel(series)) + slices.SortFunc(series, func(a, b storage.Series) bool { + return labels.Compare(a.Labels(), b.Labels()) < 0 + }) } return &concreteSeriesSet{ series: series, @@ -313,12 +316,6 @@ func MergeLabels(primary, secondary []prompb.Label) []prompb.Label { return result } -type byLabel []storage.Series - -func (a byLabel) Len() int { return len(a) } -func (a byLabel) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a byLabel) Less(i, j int) bool { return labels.Compare(a[i].Labels(), a[j].Labels()) < 0 } - // errSeriesSet implements storage.SeriesSet, just returning an error. type errSeriesSet struct { err error diff --git a/tsdb/head_read.go b/tsdb/head_read.go index 0e6e005ea..8d0a35fc2 100644 --- a/tsdb/head_read.go +++ b/tsdb/head_read.go @@ -450,7 +450,7 @@ func (s *memSeries) oooMergedChunk(meta chunks.Meta, cdm *chunks.ChunkDiskMapper // Next we want to sort all the collected chunks by min time so we can find // those that overlap and stop when we know the rest don't. - sort.Sort(byMinTimeAndMinRef(tmpChks)) + slices.SortFunc(tmpChks, refLessByMinTimeAndMinRef) mc := &mergedOOOChunks{} absoluteMax := int64(math.MinInt64) diff --git a/tsdb/index/index.go b/tsdb/index/index.go index ef2d167dc..d7650a8b3 100644 --- a/tsdb/index/index.go +++ b/tsdb/index/index.go @@ -924,7 +924,7 @@ func (w *Writer) writePostingsToTmpFiles() error { values = append(values, v) } // Symbol numbers are in order, so the strings will also be in order. - sort.Sort(uint32slice(values)) + slices.Sort(values) for _, v := range values { value, err := w.symbols.Lookup(v) if err != nil { @@ -1017,12 +1017,6 @@ func (w *Writer) writePostings() error { return nil } -type uint32slice []uint32 - -func (s uint32slice) Len() int { return len(s) } -func (s uint32slice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (s uint32slice) Less(i, j int) bool { return s[i] < s[j] } - type labelIndexHashEntry struct { keys []string offset uint64 diff --git a/tsdb/ooo_head_read.go b/tsdb/ooo_head_read.go index 8ba3ea39a..8030fc367 100644 --- a/tsdb/ooo_head_read.go +++ b/tsdb/ooo_head_read.go @@ -17,7 +17,8 @@ package tsdb import ( "errors" "math" - "sort" + + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -130,7 +131,7 @@ func (oh *OOOHeadIndexReader) series(ref storage.SeriesRef, builder *labels.Scra // Next we want to sort all the collected chunks by min time so we can find // those that overlap. - sort.Sort(metaByMinTimeAndMinRef(tmpChks)) + slices.SortFunc(tmpChks, lessByMinTimeAndMinRef) // Next we want to iterate the sorted collected chunks and only return the // chunks Meta the first chunk that overlaps with others. @@ -175,30 +176,20 @@ type chunkMetaAndChunkDiskMapperRef struct { origMaxT int64 } -type byMinTimeAndMinRef []chunkMetaAndChunkDiskMapperRef - -func (b byMinTimeAndMinRef) Len() int { return len(b) } -func (b byMinTimeAndMinRef) Less(i, j int) bool { - if b[i].meta.MinTime == b[j].meta.MinTime { - return b[i].meta.Ref < b[j].meta.Ref +func refLessByMinTimeAndMinRef(a, b chunkMetaAndChunkDiskMapperRef) bool { + if a.meta.MinTime == b.meta.MinTime { + return a.meta.Ref < b.meta.Ref } - return b[i].meta.MinTime < b[j].meta.MinTime + return a.meta.MinTime < b.meta.MinTime } -func (b byMinTimeAndMinRef) Swap(i, j int) { b[i], b[j] = b[j], b[i] } - -type metaByMinTimeAndMinRef []chunks.Meta - -func (b metaByMinTimeAndMinRef) Len() int { return len(b) } -func (b metaByMinTimeAndMinRef) Less(i, j int) bool { - if b[i].MinTime == b[j].MinTime { - return b[i].Ref < b[j].Ref +func lessByMinTimeAndMinRef(a, b chunks.Meta) bool { + if a.MinTime == b.MinTime { + return a.Ref < b.Ref } - return b[i].MinTime < b[j].MinTime + return a.MinTime < b.MinTime } -func (b metaByMinTimeAndMinRef) Swap(i, j int) { b[i], b[j] = b[j], b[i] } - func (oh *OOOHeadIndexReader) Postings(name string, values ...string) (index.Postings, error) { switch len(values) { case 0: diff --git a/tsdb/ooo_head_read_test.go b/tsdb/ooo_head_read_test.go index f3ec862f5..ed9ca2769 100644 --- a/tsdb/ooo_head_read_test.go +++ b/tsdb/ooo_head_read_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -337,7 +338,7 @@ func TestOOOHeadIndexReader_Series(t *testing.T) { } expChunks = append(expChunks, meta) } - sort.Sort(metaByMinTimeAndMinRef(expChunks)) // we always want the chunks to come back sorted by minTime asc + slices.SortFunc(expChunks, lessByMinTimeAndMinRef) // We always want the chunks to come back sorted by minTime asc. if headChunk && len(intervals) > 0 { // Put the last interval in the head chunk @@ -1116,7 +1117,7 @@ func TestSortByMinTimeAndMinRef(t *testing.T) { for _, tc := range tests { t.Run(fmt.Sprintf("name=%s", tc.name), func(t *testing.T) { - sort.Sort(byMinTimeAndMinRef(tc.input)) + slices.SortFunc(tc.input, refLessByMinTimeAndMinRef) require.Equal(t, tc.exp, tc.input) }) } @@ -1180,7 +1181,7 @@ func TestSortMetaByMinTimeAndMinRef(t *testing.T) { for _, tc := range tests { t.Run(fmt.Sprintf("name=%s", tc.name), func(t *testing.T) { - sort.Sort(metaByMinTimeAndMinRef(tc.inputMetas)) + slices.SortFunc(tc.inputMetas, lessByMinTimeAndMinRef) require.Equal(t, tc.expMetas, tc.inputMetas) }) } diff --git a/util/stats/timer.go b/util/stats/timer.go index e47162680..df1e2f931 100644 --- a/util/stats/timer.go +++ b/util/stats/timer.go @@ -16,8 +16,9 @@ package stats import ( "bytes" "fmt" - "sort" "time" + + "golang.org/x/exp/slices" ) // A Timer that can be started and stopped and accumulates the total time it @@ -78,35 +79,17 @@ func (t *TimerGroup) GetTimer(name fmt.Stringer) *Timer { return timer } -// Timers is a slice of Timer pointers that implements Len and Swap from -// sort.Interface. -type Timers []*Timer - -type byCreationTimeSorter struct{ Timers } - -// Len implements sort.Interface. -func (t Timers) Len() int { - return len(t) -} - -// Swap implements sort.Interface. -func (t Timers) Swap(i, j int) { - t[i], t[j] = t[j], t[i] -} - -func (s byCreationTimeSorter) Less(i, j int) bool { - return s.Timers[i].created < s.Timers[j].created -} - // Return a string representation of a TimerGroup. func (t *TimerGroup) String() string { - timers := byCreationTimeSorter{} + timers := make([]*Timer, 0, len(t.timers)) for _, timer := range t.timers { - timers.Timers = append(timers.Timers, timer) + timers = append(timers, timer) } - sort.Sort(timers) + slices.SortFunc(timers, func(a, b *Timer) bool { + return a.created < b.created + }) result := &bytes.Buffer{} - for _, timer := range timers.Timers { + for _, timer := range timers { fmt.Fprintf(result, "%s\n", timer) } return result.String() diff --git a/web/federate.go b/web/federate.go index b0f4c0610..1c50faed0 100644 --- a/web/federate.go +++ b/web/federate.go @@ -25,6 +25,7 @@ import ( dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" "github.com/prometheus/common/model" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" @@ -166,7 +167,11 @@ Loop: return } - sort.Sort(byName(vec)) + slices.SortFunc(vec, func(a, b promql.Sample) bool { + ni := a.Metric.Get(labels.MetricName) + nj := b.Metric.Get(labels.MetricName) + return ni < nj + }) externalLabels := h.config.GlobalConfig.ExternalLabels.Map() if _, ok := externalLabels[model.InstanceLabel]; !ok { @@ -313,15 +318,3 @@ Loop: } } } - -// byName makes a model.Vector sortable by metric name. -type byName promql.Vector - -func (vec byName) Len() int { return len(vec) } -func (vec byName) Swap(i, j int) { vec[i], vec[j] = vec[j], vec[i] } - -func (vec byName) Less(i, j int) bool { - ni := vec[i].Metric.Get(labels.MetricName) - nj := vec[j].Metric.Get(labels.MetricName) - return ni < nj -}