Skip to content

Commit de10b55

Browse files
committed
storage: use slices, maps packages
This commit replaces some of usages of the sort package with uses of slices.{Sort,SortFunc,Sorted}, where doing so makes sense. As of Go 1.21, the docs on sort.Slice say: > Note: in many situations, the newer slices.SortFunc function is > more ergonomic and runs faster. Epic: none Release note: none
1 parent c74f6a3 commit de10b55

11 files changed

+50
-76
lines changed

pkg/storage/bench_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"math/rand"
1414
"os"
1515
"path/filepath"
16-
"sort"
16+
"slices"
1717
"testing"
1818
"time"
1919

@@ -2421,14 +2421,14 @@ func BenchmarkMVCCScannerWithIntentsAndVersions(b *testing.B) {
24212421
if err != nil {
24222422
b.Fatal(err)
24232423
}
2424-
sort.Slice(kvPairs, func(i, j int) bool {
2425-
cmp := EngineComparer.Compare(kvPairs[i].key, kvPairs[j].key)
2426-
if cmp == 0 {
2424+
slices.SortFunc(kvPairs, func(i, j kvPair) int {
2425+
v := EngineComparer.Compare(i.key, j.key)
2426+
if v == 0 {
24272427
// Should not happen since we resolve in a different batch from the
24282428
// one where we wrote the intent.
2429-
b.Fatalf("found equal user keys in same batch")
2429+
b.Fatal("found equal user keys in same batch")
24302430
}
2431-
return cmp < 0
2431+
return v
24322432
})
24332433
sstFileName := fmt.Sprintf("tmp-ingest-%d", i)
24342434
sstFile, err := eng.Env().Create(sstFileName, fs.UnspecifiedWriteCategory)

pkg/storage/engine_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"os"
1717
"path/filepath"
1818
"reflect"
19-
"sort"
19+
"slices"
2020
"strconv"
2121
"strings"
2222
"testing"
@@ -763,7 +763,7 @@ func TestEngineScan1(t *testing.T) {
763763
for i, t := range testCases {
764764
sortedKeys[i] = string(t.key.Key)
765765
}
766-
sort.Strings(sortedKeys)
766+
slices.Sort(sortedKeys)
767767

768768
keyvals, err := Scan(context.Background(), engine, roachpb.Key("chinese"), roachpb.Key("german"), 0)
769769
if err != nil {
@@ -1311,7 +1311,7 @@ func TestEngineFS(t *testing.T) {
13111311
if err != nil {
13121312
break
13131313
}
1314-
sort.Strings(result)
1314+
slices.Sort(result)
13151315
got := strings.Join(result, ",")
13161316
want := s[3]
13171317
if got != want {
@@ -1460,7 +1460,7 @@ func TestFS(t *testing.T) {
14601460
t.Helper()
14611461

14621462
got, err := e.List(dir)
1463-
sort.Strings(got)
1463+
slices.Sort(got)
14641464
require.NoError(t, err)
14651465
if !reflect.DeepEqual(got, want) {
14661466
t.Fatalf("e.List(%q) = %#v, want %#v", dir, got, want)
@@ -1610,7 +1610,7 @@ func TestScanLocks(t *testing.T) {
16101610
for k := range locks {
16111611
keys = append(keys, roachpb.Key(k))
16121612
}
1613-
sort.Slice(keys, func(i, j int) bool { return bytes.Compare(keys[i], keys[j]) < 0 })
1613+
slices.SortFunc(keys, roachpb.Key.Compare)
16141614

16151615
testcases := map[string]struct {
16161616
from roachpb.Key

pkg/storage/fs/file_registry.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ import (
99
"context"
1010
"fmt"
1111
"io"
12+
"maps"
1213
"os"
1314
"path/filepath"
14-
"sort"
15+
"slices"
1516
"strconv"
1617
"strings"
1718

@@ -191,9 +192,7 @@ func (r *FileRegistry) Load(ctx context.Context) error {
191192
obsoleteFiles = append(obsoleteFiles, fileNum)
192193
}
193194
}
194-
sort.Slice(obsoleteFiles, func(i, j int) bool {
195-
return obsoleteFiles[i] < obsoleteFiles[j]
196-
})
195+
slices.Sort(obsoleteFiles)
197196
r.writeMu.obsoleteRegistryFiles = make([]string, 0, r.NumOldRegistryFiles+1)
198197
for _, f := range obsoleteFiles {
199198
r.writeMu.obsoleteRegistryFiles = append(r.writeMu.obsoleteRegistryFiles, makeRegistryFilename(f))
@@ -306,11 +305,7 @@ func (r *FileRegistry) maybeElideEntries(ctx context.Context) error {
306305
// recursively List each directory and walk two lists of sorted
307306
// filenames. We should test a store with many files to see how much
308307
// the current approach slows node start.
309-
filenames := make([]string, 0, len(r.writeMu.mu.entries))
310-
for filename := range r.writeMu.mu.entries {
311-
filenames = append(filenames, filename)
312-
}
313-
sort.Strings(filenames)
308+
filenames := slices.Sorted(maps.Keys(r.writeMu.mu.entries))
314309

315310
batch := &enginepb.RegistryUpdateBatch{}
316311
for _, filename := range filenames {

pkg/storage/fs/file_registry_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ package fs
77

88
import (
99
"bytes"
10+
"cmp"
1011
"context"
1112
"fmt"
1213
"io"
1314
"os"
14-
"sort"
15+
"slices"
1516
"strings"
1617
"testing"
1718
"time"
@@ -392,8 +393,8 @@ func TestFileRegistry(t *testing.T) {
392393
entry: entry,
393394
})
394395
}
395-
sort.Slice(fileEntries, func(i, j int) bool {
396-
return fileEntries[i].name < fileEntries[j].name
396+
slices.SortFunc(fileEntries, func(a, b fileEntry) int {
397+
return cmp.Compare(a.name, b.name)
397398
})
398399
var b bytes.Buffer
399400
for _, fe := range fileEntries {

pkg/storage/intent_interleaving_iter_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ package storage
77

88
import (
99
"bytes"
10+
"cmp"
1011
"context"
1112
"flag"
1213
"fmt"
1314
"math/rand"
14-
"sort"
15+
"slices"
1516
"strings"
1617
"testing"
1718

@@ -555,15 +556,8 @@ func generateRandomData(
555556
timestamps = append(timestamps, rng.Intn(1<<20)+1)
556557
}
557558
// Sort in descending order and make unique.
558-
sort.Sort(sort.Reverse(sort.IntSlice(timestamps)))
559-
last := 0
560-
for j := 1; j < len(timestamps); j++ {
561-
if timestamps[j] != timestamps[last] {
562-
last++
563-
timestamps[last] = timestamps[j]
564-
}
565-
}
566-
timestamps = timestamps[:last+1]
559+
slices.SortFunc(timestamps, func(a, b int) int { return cmp.Compare(b, a) })
560+
timestamps = slices.Compact(timestamps)
567561
for i, ts := range timestamps {
568562
// Intent.
569563
meta := enginepb.MVCCMetadata{

pkg/storage/metamorphic/operations.go

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"fmt"
1111
"math"
1212
"path/filepath"
13-
"sort"
13+
"slices"
1414
"strconv"
1515
"strings"
1616

@@ -1526,21 +1526,10 @@ var opGenerators = []opGenerator{
15261526
keys = append(keys, key)
15271527
}
15281528
// SST Writer expects keys in sorted order, so sort them first.
1529-
sort.Slice(keys, func(i, j int) bool {
1530-
return keys[i].Less(keys[j])
1531-
})
1529+
slices.SortFunc(keys, storage.MVCCKey.Compare)
15321530
// An sstable intended for ingest cannot have the same key appear
15331531
// multiple times. Remove any duplicates.
1534-
n := len(keys)
1535-
for i := 1; i < n; {
1536-
if keys[i-1].Equal(keys[i]) {
1537-
copy(keys[i:], keys[i+1:])
1538-
n--
1539-
} else {
1540-
i++
1541-
}
1542-
}
1543-
keys = keys[:n]
1532+
keys = slices.CompactFunc(keys, storage.MVCCKey.Equal)
15441533

15451534
return &ingestOp{
15461535
m: m,

pkg/storage/mvcc.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"io"
1414
"math"
1515
"runtime"
16-
"sort"
16+
"slices"
1717
"sync"
1818
"time"
1919

@@ -6335,10 +6335,10 @@ func MVCCGarbageCollect(
63356335

63366336
// Sort the slice to both determine the bounds and ensure that we're seeking
63376337
// in increasing order.
6338-
sort.Slice(keys, func(i, j int) bool {
6339-
iKey := MVCCKey{Key: keys[i].Key, Timestamp: keys[i].Timestamp}
6340-
jKey := MVCCKey{Key: keys[j].Key, Timestamp: keys[j].Timestamp}
6341-
return iKey.Less(jKey)
6338+
slices.SortFunc(keys, func(a, b kvpb.GCRequest_GCKey) int {
6339+
aKey := MVCCKey{Key: a.Key, Timestamp: a.Timestamp}
6340+
bKey := MVCCKey{Key: b.Key, Timestamp: b.Timestamp}
6341+
return aKey.Compare(bKey)
63426342
})
63436343

63446344
// Bound the iterator appropriately for the set of keys we'll be garbage
@@ -6649,8 +6649,8 @@ func MVCCGarbageCollectRangeKeys(
66496649
}
66506650
}
66516651

6652-
sort.Slice(rks, func(i, j int) bool {
6653-
return rks[i].Compare(rks[j].MVCCRangeKey) < 0
6652+
slices.SortFunc(rks, func(i, j CollectableGCRangeKey) int {
6653+
return i.MVCCRangeKey.Compare(j.MVCCRangeKey)
66546654
})
66556655

66566656
// Validate that keys are non-overlapping.

pkg/storage/mvcc_history_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import (
99
"bytes"
1010
"context"
1111
"fmt"
12+
"maps"
1213
"math"
1314
"regexp"
15+
"slices"
1416
"sort"
1517
"strconv"
1618
"strings"
@@ -301,12 +303,7 @@ func TestMVCCHistories(t *testing.T) {
301303

302304
// Unreplicated locks.
303305
if len(e.unreplLocks) > 0 {
304-
var ks []string
305-
for k := range e.unreplLocks {
306-
ks = append(ks, k)
307-
}
308-
sort.Strings(ks)
309-
for _, k := range ks {
306+
for _, k := range slices.Sorted(maps.Keys(e.unreplLocks)) {
310307
info := e.unreplLocks[k]
311308
buf.Printf("lock (%s): %v/%s -> %+v\n",
312309
lock.Unreplicated, k, info.str, info.txn)

pkg/storage/mvcc_key_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
"math"
1313
"math/rand"
1414
"reflect"
15-
"sort"
15+
"slices"
1616
"testing"
1717
"testing/quick"
1818

@@ -57,7 +57,7 @@ func TestMVCCKeys(t *testing.T) {
5757
sortKeys := make(mvccKeys, len(keys))
5858
copy(sortKeys, keys)
5959
shuffle.Shuffle(sortKeys)
60-
sort.Sort(sortKeys)
60+
slices.SortFunc(sortKeys, MVCCKey.Compare)
6161
if !reflect.DeepEqual(sortKeys, keys) {
6262
t.Errorf("expected keys to sort in order %s, but got %s", keys, sortKeys)
6363
}
@@ -978,8 +978,8 @@ func BenchmarkMVCCRangeKeyStack_Clone(b *testing.B) {
978978
}
979979
stack.Versions = append(stack.Versions, version)
980980
}
981-
sort.Slice(stack.Versions, func(i, j int) bool {
982-
return stack.Versions[i].Timestamp.Less(stack.Versions[j].Timestamp)
981+
slices.SortFunc(stack.Versions, func(i, j MVCCRangeKeyVersion) int {
982+
return i.Timestamp.Compare(j.Timestamp)
983983
})
984984
return stack
985985
}
@@ -1082,7 +1082,7 @@ func rangeKeyVersions(v map[int]MVCCValue) MVCCRangeKeyVersions {
10821082
for i := range v {
10831083
timestamps = append(timestamps, i)
10841084
}
1085-
sort.Ints(timestamps)
1085+
slices.Sort(timestamps)
10861086
for i, ts := range timestamps {
10871087
versions[len(versions)-1-i] = rangeKeyVersion(ts, v[ts])
10881088
}

pkg/storage/mvcc_stats_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ package storage
88
import (
99
"context"
1010
"fmt"
11+
"maps"
1112
"math/rand"
12-
"sort"
13+
"slices"
1314
"testing"
1415

1516
"github.com/cockroachdb/cockroach/pkg/keys"
@@ -1845,10 +1846,7 @@ func (s *randomTest) step(t *testing.T) {
18451846
s.cycle++
18461847

18471848
if s.actionNames == nil {
1848-
for name := range s.actions {
1849-
s.actionNames = append(s.actionNames, name)
1850-
}
1851-
sort.Strings(s.actionNames)
1849+
s.actionNames = slices.Sorted(maps.Keys(s.actions))
18521850
}
18531851
actName := s.actionNames[s.rng.Intn(len(s.actionNames))]
18541852

0 commit comments

Comments
 (0)