From 15102e6dfd3eddca34d83e2264c79d05d665939f Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Nov 2024 10:34:16 +0300 Subject: [PATCH] [#1471] Replace sort.Slice in some places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `slices.SortFunc` doesn't use reflection and is a bit faster. I have done some micro-benchmarks for `[]NodeInfo`: ``` $ benchstat -col "/func" out goos: linux goarch: amd64 pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/pilorama cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz │ sort.Slice │ slices.SortFunc │ │ sec/op │ sec/op vs base │ Sort-8 2.130µ ± 2% 1.253µ ± 2% -41.20% (p=0.000 n=10) ``` Haven't included them, though, as they I don't see them being used a lot. Signed-off-by: Evgenii Stratonikov --- cmd/frostfs-cli/internal/client/client.go | 5 ++--- cmd/frostfs-cli/internal/common/tracing.go | 10 +++------- pkg/local_object_storage/pilorama/batch.go | 6 +++--- pkg/local_object_storage/pilorama/boltdb.go | 12 ++++++++---- pkg/local_object_storage/pilorama/forest.go | 5 +---- pkg/local_object_storage/pilorama/inmemory.go | 13 ++++++++----- pkg/services/tree/service.go | 9 ++++----- 7 files changed, 29 insertions(+), 31 deletions(-) diff --git a/cmd/frostfs-cli/internal/client/client.go b/cmd/frostfs-cli/internal/client/client.go index ed9817b86..948d61f36 100644 --- a/cmd/frostfs-cli/internal/client/client.go +++ b/cmd/frostfs-cli/internal/client/client.go @@ -670,9 +670,8 @@ func SearchObjects(ctx context.Context, prm SearchObjectsPrm) (*SearchObjectsRes return nil, fmt.Errorf("read object list: %w", err) } - sort.Slice(list, func(i, j int) bool { - lhs, rhs := list[i].EncodeToString(), list[j].EncodeToString() - return strings.Compare(lhs, rhs) < 0 + slices.SortFunc(list, func(a, b oid.ID) int { + return strings.Compare(a.EncodeToString(), b.EncodeToString()) }) return &SearchObjectsRes{ diff --git a/cmd/frostfs-cli/internal/common/tracing.go b/cmd/frostfs-cli/internal/common/tracing.go index 30c2f2b1a..10863ed1e 100644 --- a/cmd/frostfs-cli/internal/common/tracing.go +++ b/cmd/frostfs-cli/internal/common/tracing.go @@ -2,7 +2,7 @@ package common import ( "context" - "sort" + "slices" "strings" "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/commonflags" @@ -45,15 +45,11 @@ func StartClientCommandSpan(cmd *cobra.Command) { }) commonCmd.ExitOnErr(cmd, "init tracing: %w", err) - var components sort.StringSlice + var components []string for c := cmd; c != nil; c = c.Parent() { components = append(components, c.Name()) } - for i, j := 0, len(components)-1; i < j; { - components.Swap(i, j) - i++ - j-- - } + slices.Reverse(components) operation := strings.Join(components, ".") ctx, span := tracing.StartSpanFromContext(cmd.Context(), operation) diff --git a/pkg/local_object_storage/pilorama/batch.go b/pkg/local_object_storage/pilorama/batch.go index 520c6dfb4..4c5238921 100644 --- a/pkg/local_object_storage/pilorama/batch.go +++ b/pkg/local_object_storage/pilorama/batch.go @@ -1,9 +1,9 @@ package pilorama import ( + "cmp" "encoding/binary" "slices" - "sort" "sync" "time" @@ -48,8 +48,8 @@ func (b *batch) run() { // Sorting without a mutex is ok, because we append to this slice only if timer is non-nil. // See (*boltForest).addBatch for details. - sort.Slice(b.operations, func(i, j int) bool { - return b.operations[i].Time < b.operations[j].Time + slices.SortFunc(b.operations, func(mi, mj *Move) int { + return cmp.Compare(mi.Time, mj.Time) }) b.operations = slices.CompactFunc(b.operations, func(x, y *Move) bool { return x.Time == y.Time }) diff --git a/pkg/local_object_storage/pilorama/boltdb.go b/pkg/local_object_storage/pilorama/boltdb.go index 09f2e1919..7bce1f340 100644 --- a/pkg/local_object_storage/pilorama/boltdb.go +++ b/pkg/local_object_storage/pilorama/boltdb.go @@ -10,7 +10,6 @@ import ( "os" "path/filepath" "slices" - "sort" "strconv" "sync" "time" @@ -1093,14 +1092,19 @@ func (t *boltForest) TreeSortedByFilename(ctx context.Context, cid cidSDK.ID, tr return res, last, metaerr.Wrap(err) } +func sortByFilename(nodes []NodeInfo) { + slices.SortFunc(nodes, func(a, b NodeInfo) int { + return bytes.Compare(a.Meta.GetAttr(AttributeFilename), b.Meta.GetAttr(AttributeFilename)) + }) +} + func sortAndCut(result []NodeInfo, last *string) []NodeInfo { var lastBytes []byte if last != nil { lastBytes = []byte(*last) } - sort.Slice(result, func(i, j int) bool { - return bytes.Compare(result[i].Meta.GetAttr(AttributeFilename), result[j].Meta.GetAttr(AttributeFilename)) == -1 - }) + sortByFilename(result) + for i := range result { if lastBytes == nil || bytes.Compare(lastBytes, result[i].Meta.GetAttr(AttributeFilename)) == -1 { return result[i:] diff --git a/pkg/local_object_storage/pilorama/forest.go b/pkg/local_object_storage/pilorama/forest.go index 78503bada..bb5c22e51 100644 --- a/pkg/local_object_storage/pilorama/forest.go +++ b/pkg/local_object_storage/pilorama/forest.go @@ -1,7 +1,6 @@ package pilorama import ( - "bytes" "context" "errors" "fmt" @@ -192,9 +191,7 @@ func (f *memoryForest) TreeSortedByFilename(_ context.Context, cid cid.ID, treeI return nil, start, nil } - sort.Slice(res, func(i, j int) bool { - return bytes.Compare(res[i].Meta.GetAttr(AttributeFilename), res[j].Meta.GetAttr(AttributeFilename)) == -1 - }) + sortByFilename(res) r := mergeNodeInfos(res) for i := range r { diff --git a/pkg/local_object_storage/pilorama/inmemory.go b/pkg/local_object_storage/pilorama/inmemory.go index c9f5df3b7..ce7b3db1e 100644 --- a/pkg/local_object_storage/pilorama/inmemory.go +++ b/pkg/local_object_storage/pilorama/inmemory.go @@ -1,6 +1,9 @@ package pilorama -import "sort" +import ( + "cmp" + "slices" +) // nodeInfo couples parent and metadata. type nodeInfo struct { @@ -131,10 +134,10 @@ func (t tree) getChildren(parent Node) []Node { } } - sort.Slice(children, func(i, j int) bool { - a := t.infoMap[children[i]] - b := t.infoMap[children[j]] - return a.Meta.Time < b.Meta.Time + slices.SortFunc(children, func(ci, cj uint64) int { + a := t.infoMap[ci] + b := t.infoMap[cj] + return cmp.Compare(a.Meta.Time, b.Meta.Time) }) return children } diff --git a/pkg/services/tree/service.go b/pkg/services/tree/service.go index 10c3b6ccc..8097d545c 100644 --- a/pkg/services/tree/service.go +++ b/pkg/services/tree/service.go @@ -5,7 +5,7 @@ import ( "context" "errors" "fmt" - "sort" + "slices" "sync" "sync/atomic" @@ -575,10 +575,9 @@ func sortByFilename(nodes []pilorama.NodeInfo, d GetSubTreeRequest_Body_Order_Di if len(nodes) == 0 { return nodes, nil } - less := func(i, j int) bool { - return bytes.Compare(nodes[i].Meta.GetAttr(pilorama.AttributeFilename), nodes[j].Meta.GetAttr(pilorama.AttributeFilename)) < 0 - } - sort.Slice(nodes, less) + slices.SortFunc(nodes, func(a, b pilorama.NodeInfo) int { + return bytes.Compare(a.Meta.GetAttr(pilorama.AttributeFilename), b.Meta.GetAttr(pilorama.AttributeFilename)) + }) return nodes, nil default: return nil, fmt.Errorf("unsupported order direction: %s", d.String())