From 3330d85ba8cd6c53e74af23ab96e9e2c134bec39 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 30 Sep 2022 15:33:56 +0100 Subject: [PATCH] Replace sort.Strings and sort.Ints with faster slices.Sort (#11318) Use new experimental package `golang.org/x/exp/slices`. slices.Sort works on values that are directly comparable, like ints, so avoids the overhad of an interface call to `.Less()`. Left tests unchanged, because they don't need the speed and it may be a cross-check that slices.Sort gives the same answer. Signed-off-by: Bryan Boreham --- promql/engine.go | 9 +++++---- storage/merge.go | 5 +++-- tsdb/block.go | 4 ++-- tsdb/chunks/head_chunks.go | 10 +++++----- tsdb/head_read.go | 7 ++++--- tsdb/index/index.go | 9 +++++---- tsdb/index/postings.go | 2 +- tsdb/querier.go | 8 ++++---- tsdb/wal/watcher.go | 4 ++-- web/api/v1/api.go | 8 ++++---- 10 files changed, 35 insertions(+), 31 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index b6842e8f4..18be6f3dc 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -35,6 +35,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/timestamp" @@ -1243,7 +1244,7 @@ func (ev *evaluator) eval(expr parser.Expr) (parser.Value, storage.Warnings) { case *parser.AggregateExpr: // Grouping labels must be sorted (expected both by generateGroupingKey() and aggregation()). sortedGrouping := e.Grouping - sort.Strings(sortedGrouping) + slices.Sort(sortedGrouping) // Prepare a function to initialise series helpers with the grouping key. buf := make([]byte, 0, 1024) @@ -2059,13 +2060,13 @@ func (ev *evaluator) VectorBinop(op parser.ItemType, lhs, rhs Vector, matching * func signatureFunc(on bool, b []byte, names ...string) func(labels.Labels) string { if on { - sort.Strings(names) + slices.Sort(names) return func(lset labels.Labels) string { return string(lset.BytesWithLabels(b, names...)) } } names = append([]string{labels.MetricName}, names...) - sort.Strings(names) + slices.Sort(names) return func(lset labels.Labels) string { return string(lset.BytesWithoutLabels(b, names...)) } @@ -2267,7 +2268,7 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without // operator is less frequently used than other aggregations, we're fine having to // re-compute the grouping key on each step for this case. grouping = append(grouping, valueLabel) - sort.Strings(grouping) + slices.Sort(grouping) recomputeGroupingKey = true } } diff --git a/storage/merge.go b/storage/merge.go index 2f175d3e7..5d6eaad63 100644 --- a/storage/merge.go +++ b/storage/merge.go @@ -18,9 +18,10 @@ import ( "container/heap" "fmt" "math" - "sort" "sync" + "golang.org/x/exp/slices" + "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/tsdb/chunkenc" "github.com/prometheus/prometheus/tsdb/chunks" @@ -240,7 +241,7 @@ func (q *mergeGenericQuerier) LabelNames(matchers ...*labels.Matcher) ([]string, for name := range labelNamesMap { labelNames = append(labelNames, name) } - sort.Strings(labelNames) + slices.Sort(labelNames) return labelNames, warnings, nil } diff --git a/tsdb/block.go b/tsdb/block.go index 8fd1066ba..8dbeff16e 100644 --- a/tsdb/block.go +++ b/tsdb/block.go @@ -19,13 +19,13 @@ import ( "io" "os" "path/filepath" - "sort" "sync" "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/oklog/ulid" "github.com/pkg/errors" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -463,7 +463,7 @@ func (r blockIndexReader) SortedLabelValues(name string, matchers ...*labels.Mat } else { st, err = r.LabelValues(name, matchers...) if err == nil { - sort.Strings(st) + slices.Sort(st) } } diff --git a/tsdb/chunks/head_chunks.go b/tsdb/chunks/head_chunks.go index b2ad59fd6..a0bd735b8 100644 --- a/tsdb/chunks/head_chunks.go +++ b/tsdb/chunks/head_chunks.go @@ -21,7 +21,6 @@ import ( "io" "os" "path/filepath" - "sort" "strconv" "sync" @@ -29,6 +28,7 @@ import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "go.uber.org/atomic" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/tsdb/chunkenc" tsdb_errors "github.com/prometheus/prometheus/tsdb/errors" @@ -308,7 +308,7 @@ func (cdm *ChunkDiskMapper) openMMapFiles() (returnErr error) { } // Check for gaps in the files. - sort.Ints(chkFileIndices) + slices.Sort(chkFileIndices) if len(chkFileIndices) == 0 { return nil } @@ -777,7 +777,7 @@ func (cdm *ChunkDiskMapper) IterateAllChunks(f func(seriesRef HeadSeriesRef, chu for seg := range cdm.mmappedChunkFiles { segIDs = append(segIDs, seg) } - sort.Ints(segIDs) + slices.Sort(segIDs) for _, segID := range segIDs { mmapFile := cdm.mmappedChunkFiles[segID] fileEnd := mmapFile.byteSlice.Len() @@ -894,7 +894,7 @@ func (cdm *ChunkDiskMapper) Truncate(fileNo uint32) error { for seq := range cdm.mmappedChunkFiles { chkFileIndices = append(chkFileIndices, seq) } - sort.Ints(chkFileIndices) + slices.Sort(chkFileIndices) var removedFiles []int for _, seq := range chkFileIndices { @@ -934,7 +934,7 @@ func (cdm *ChunkDiskMapper) Truncate(fileNo uint32) error { // deleteFiles deletes the given file sequences in order of the sequence. // In case of an error, it returns the sorted file sequences that were not deleted from the _disk_. func (cdm *ChunkDiskMapper) deleteFiles(removedFiles []int) ([]int, error) { - sort.Ints(removedFiles) // To delete them in order. + slices.Sort(removedFiles) // To delete them in order. cdm.readPathMtx.Lock() for _, seq := range removedFiles { if err := cdm.closers[seq].Close(); err != nil { diff --git a/tsdb/head_read.go b/tsdb/head_read.go index a97537a1f..70cd4049e 100644 --- a/tsdb/head_read.go +++ b/tsdb/head_read.go @@ -21,6 +21,7 @@ import ( "github.com/go-kit/log/level" "github.com/pkg/errors" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -65,7 +66,7 @@ func (h *headIndexReader) Symbols() index.StringIter { func (h *headIndexReader) SortedLabelValues(name string, matchers ...*labels.Matcher) ([]string, error) { values, err := h.LabelValues(name, matchers...) if err == nil { - sort.Strings(values) + slices.Sort(values) } return values, err } @@ -95,7 +96,7 @@ func (h *headIndexReader) LabelNames(matchers ...*labels.Matcher) ([]string, err if len(matchers) == 0 { labelNames := h.head.postings.LabelNames() - sort.Strings(labelNames) + slices.Sort(labelNames) return labelNames, nil } @@ -229,7 +230,7 @@ func (h *headIndexReader) LabelNamesFor(ids ...storage.SeriesRef) ([]string, err for name := range namesMap { names = append(names, name) } - sort.Strings(names) + slices.Sort(names) return names, nil } diff --git a/tsdb/index/index.go b/tsdb/index/index.go index 29295c45f..06a0f3e71 100644 --- a/tsdb/index/index.go +++ b/tsdb/index/index.go @@ -29,6 +29,7 @@ import ( "unsafe" "github.com/pkg/errors" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -819,7 +820,7 @@ func (w *Writer) writePostingsToTmpFiles() error { for n := range w.labelNames { names = append(names, n) } - sort.Strings(names) + slices.Sort(names) if err := w.f.Flush(); err != nil { return err @@ -1469,7 +1470,7 @@ func (r *Reader) SymbolTableSize() uint64 { func (r *Reader) SortedLabelValues(name string, matchers ...*labels.Matcher) ([]string, error) { values, err := r.LabelValues(name, matchers...) if err == nil && r.version == FormatV1 { - sort.Strings(values) + slices.Sort(values) } return values, err } @@ -1571,7 +1572,7 @@ func (r *Reader) LabelNamesFor(ids ...storage.SeriesRef) ([]string, error) { names = append(names, name) } - sort.Strings(names) + slices.Sort(names) return names, nil } @@ -1743,7 +1744,7 @@ func (r *Reader) LabelNames(matchers ...*labels.Matcher) ([]string, error) { } labelNames = append(labelNames, name) } - sort.Strings(labelNames) + slices.Sort(labelNames) return labelNames, nil } diff --git a/tsdb/index/postings.go b/tsdb/index/postings.go index a9898fc4a..22e85ab36 100644 --- a/tsdb/index/postings.go +++ b/tsdb/index/postings.go @@ -91,7 +91,7 @@ func (p *MemPostings) Symbols() StringIter { res = append(res, k) } - sort.Strings(res) + slices.Sort(res) return NewStringListIter(res) } diff --git a/tsdb/querier.go b/tsdb/querier.go index 5141a2c1d..586478f36 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -15,11 +15,11 @@ package tsdb import ( "math" - "sort" "strings" "unicode/utf8" "github.com/pkg/errors" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" @@ -317,7 +317,7 @@ func postingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Postings, erro if m.Type == labels.MatchRegexp { setMatches := findSetMatches(m.GetRegexString()) if len(setMatches) > 0 { - sort.Strings(setMatches) + slices.Sort(setMatches) return ix.Postings(m.Name, setMatches...) } } @@ -344,7 +344,7 @@ func postingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Postings, erro } if !isSorted { - sort.Strings(res) + slices.Sort(res) } return ix.Postings(m.Name, res...) } @@ -369,7 +369,7 @@ func inversePostingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Posting } if !isSorted { - sort.Strings(res) + slices.Sort(res) } return ix.Postings(m.Name, res...) } diff --git a/tsdb/wal/watcher.go b/tsdb/wal/watcher.go index 5b949fc00..85d01880f 100644 --- a/tsdb/wal/watcher.go +++ b/tsdb/wal/watcher.go @@ -19,7 +19,6 @@ import ( "math" "os" "path" - "sort" "strconv" "strings" "time" @@ -28,6 +27,7 @@ import ( "github.com/go-kit/log/level" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/tsdb/record" @@ -317,7 +317,7 @@ func (w *Watcher) segments(dir string) ([]int, error) { } refs = append(refs, k) } - sort.Ints(refs) + slices.Sort(refs) for i := 0; i < len(refs)-1; i++ { if refs[i]+1 != refs[i+1] { return nil, errors.New("segments are not sequential") diff --git a/web/api/v1/api.go b/web/api/v1/api.go index bb97e7eeb..bd040912e 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -23,7 +23,6 @@ import ( "net/url" "os" "path/filepath" - "sort" "strconv" "strings" "time" @@ -37,6 +36,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" "github.com/prometheus/common/route" + "golang.org/x/exp/slices" "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/model/exemplar" @@ -628,7 +628,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { for key := range labelNamesSet { names = append(names, key) } - sort.Strings(names) + slices.Sort(names) } else { names, warnings, err = q.LabelNames() if err != nil { @@ -713,7 +713,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { } } - sort.Strings(vals) + slices.Sort(vals) return apiFuncResult{vals, nil, warnings, closer} } @@ -908,7 +908,7 @@ func (api *API) targets(r *http.Request) apiFuncResult { keys = append(keys, k) n += len(targets[k]) } - sort.Strings(keys) + slices.Sort(keys) return keys, n }