Skip to content

Commit

Permalink
prefer slices.SortFunc to sort.Sort
Browse files Browse the repository at this point in the history
In many situations, the newer `slices.SortFunc` function is more ergonomic and runs faster than `sort.Sort`. Using `slices.SortFunc` also avoids having to define custom slice types to make slices of objects sortable.
  • Loading branch information
gammazero committed Jan 17, 2025
1 parent 8ca0ca2 commit 4a8bdf4
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 83 deletions.
13 changes: 5 additions & 8 deletions bitswap/client/wantlist/wantlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
package wantlist

import (
"sort"
"cmp"
"slices"

pb "github.com/ipfs/boxo/bitswap/message/pb"

Expand Down Expand Up @@ -34,12 +35,6 @@ func NewRefEntry(c cid.Cid, p int32) Entry {
}
}

type entrySlice []Entry

func (es entrySlice) Len() int { return len(es) }
func (es entrySlice) Swap(i, j int) { es[i], es[j] = es[j], es[i] }
func (es entrySlice) Less(i, j int) bool { return es[i].Priority > es[j].Priority }

// New generates a new raw Wantlist
func New() *Wantlist {
return &Wantlist{
Expand Down Expand Up @@ -125,7 +120,9 @@ func (w *Wantlist) Entries() []Entry {
for _, e := range w.set {
es = append(es, e)
}
sort.Sort(entrySlice(es))
slices.SortFunc(es, func(a, b Entry) int {
return cmp.Compare(b.Priority, a.Priority)
})
w.cached = es
return es[0:len(es):len(es)]
}
4 changes: 2 additions & 2 deletions bitswap/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"errors"
"fmt"
"sort"
"slices"
"sync"
"time"

Expand Down Expand Up @@ -383,7 +383,7 @@ func (bs *Server) Stat() (Stat, error) {
for i, p := range peers {
peersStr[i] = p.String()
}
sort.Strings(peersStr)
slices.Sort(peersStr)
s.Peers = peersStr

return s, nil
Expand Down
4 changes: 2 additions & 2 deletions files/serialfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"io"
"os"
"path/filepath"
"sort"
"slices"
"strings"
"testing"
)
Expand Down Expand Up @@ -96,7 +96,7 @@ testInputs:
}
}

sort.Strings(expectedPaths)
slices.Sort(expectedPaths)

stat, err := os.Stat(tmppath)
if err != nil {
Expand Down
30 changes: 13 additions & 17 deletions filestore/util.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package filestore

import (
"cmp"
"context"
"fmt"
"sort"
"slices"
"strings"

pb "github.com/ipfs/boxo/filestore/pb"

Expand Down Expand Up @@ -177,7 +179,7 @@ func listAllFileOrder(ctx context.Context, fs *Filestore, verify bool) (func(con
return nil, err
}

var entries listEntries
var entries []*listEntry

Check warning on line 182 in filestore/util.go

View check run for this annotation

Codecov / codecov/patch

filestore/util.go#L182

Added line #L182 was not covered by tests

for {
v, ok := qr.NextSync()
Expand All @@ -199,7 +201,15 @@ func listAllFileOrder(ctx context.Context, fs *Filestore, verify bool) (func(con
})
}
}
sort.Sort(entries)
slices.SortFunc(entries, func(a, b *listEntry) int {
if a.filePath == b.filePath {
if a.offset == b.offset {
return strings.Compare(a.dsKey, b.dsKey)
}
return cmp.Compare(a.offset, b.offset)

Check warning on line 209 in filestore/util.go

View check run for this annotation

Codecov / codecov/patch

filestore/util.go#L204-L209

Added lines #L204 - L209 were not covered by tests
}
return strings.Compare(a.filePath, b.filePath)

Check warning on line 211 in filestore/util.go

View check run for this annotation

Codecov / codecov/patch

filestore/util.go#L211

Added line #L211 was not covered by tests
})

i := 0
return func(ctx context.Context) *ListRes {
Expand Down Expand Up @@ -243,20 +253,6 @@ type listEntry struct {
err error
}

type listEntries []*listEntry

func (l listEntries) Len() int { return len(l) }
func (l listEntries) Swap(i, j int) { l[i], l[j] = l[j], l[i] }
func (l listEntries) Less(i, j int) bool {
if l[i].filePath == l[j].filePath {
if l[i].offset == l[j].offset {
return l[i].dsKey < l[j].dsKey
}
return l[i].offset < l[j].offset
}
return l[i].filePath < l[j].filePath
}

func mkListRes(m mh.Multihash, d *pb.DataObj, err error) *ListRes {
status := StatusOk
errorMsg := ""
Expand Down
5 changes: 2 additions & 3 deletions gateway/headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package gateway

import (
"net/http"
"sort"
"slices"
)

// Headers is an HTTP middleware that sets the configured headers in all requests.
Expand Down Expand Up @@ -106,7 +106,6 @@ func cleanHeaderSet(headers []string) []string {
result = append(result, k)
}

// Sort
sort.Strings(result)
slices.Sort(result)
return result
}
16 changes: 5 additions & 11 deletions ipld/merkledag/coding.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package merkledag
import (
"errors"
"fmt"
"sort"
"slices"
"strings"

pb "github.com/ipfs/boxo/ipld/merkledag/pb"
Expand All @@ -24,14 +24,6 @@ const _ = pb.DoNotUpgradeFileEverItWillChangeYourHashes
// for now, we use a PBNode intermediate thing.
// because native go objects are nice.

// pbLinkSlice is a slice of pb.PBLink, similar to LinkSlice but for sorting the
// PB form
type pbLinkSlice []*pb.PBLink

func (pbls pbLinkSlice) Len() int { return len(pbls) }
func (pbls pbLinkSlice) Swap(a, b int) { pbls[a], pbls[b] = pbls[b], pbls[a] }
func (pbls pbLinkSlice) Less(a, b int) bool { return *pbls[a].Name < *pbls[b].Name }

// unmarshal decodes raw data into a *Node instance.
// The conversion uses an intermediate PBNode.
func unmarshal(encodedBytes []byte) (*ProtoNode, error) {
Expand Down Expand Up @@ -148,7 +140,9 @@ func (n *ProtoNode) GetPBNode() *pb.PBNode {
// Ensure links are sorted prior to encode, regardless of `linksDirty`. They
// may not have come sorted if we deserialized a badly encoded form that
// didn't have links already sorted.
sort.Stable(pbLinkSlice(pbn.Links))
slices.SortStableFunc(pbn.Links, func(a, b *pb.PBLink) int {
return strings.Compare(*a.Name, *b.Name)
})

if len(n.data) > 0 {
pbn.Data = n.data
Expand All @@ -163,7 +157,7 @@ func (n *ProtoNode) EncodeProtobuf(force bool) ([]byte, error) {
if n.linksDirty {
// there was a mutation involving links, make sure we sort before we build
// and cache a `Node` form that captures the current state
sort.Stable(LinkSlice(n.links))
n.sortLinks()
n.linksDirty = false
}
n.cached = cid.Undef
Expand Down
24 changes: 12 additions & 12 deletions ipld/merkledag/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
"errors"
"fmt"
"math"
"sort"
"slices"
"strings"

blocks "github.com/ipfs/go-block-format"
cid "github.com/ipfs/go-cid"
Expand Down Expand Up @@ -147,13 +148,6 @@ func checkHasher(indicator uint64, sizeHint int) error {
return err
}

// LinkSlice is a slice of format.Links
type LinkSlice []*format.Link

func (ls LinkSlice) Len() int { return len(ls) }
func (ls LinkSlice) Swap(a, b int) { ls[a], ls[b] = ls[b], ls[a] }
func (ls LinkSlice) Less(a, b int) bool { return ls[a].Name < ls[b].Name }

// NodeWithData builds a new Protonode with the given data.
func NodeWithData(d []byte) *ProtoNode {
return &ProtoNode{data: d}
Expand Down Expand Up @@ -289,7 +283,7 @@ func (n *ProtoNode) Copy() format.Node {
// Sort links regardless of linksDirty state, this may have come from a
// serialized form that had badly sorted links, in which case linksDirty
// will not be true.
sort.Stable(LinkSlice(nnode.links))
nnode.sortLinks()
}

nnode.builder = n.builder
Expand Down Expand Up @@ -424,7 +418,7 @@ func checkLink(lnk *format.Link) error {
func (n *ProtoNode) MarshalJSON() ([]byte, error) {
if n.linksDirty {
// there was a mutation involving links, make sure we sort
sort.Stable(LinkSlice(n.links))
n.sortLinks()
n.linksDirty = false
n.encoded = nil
}
Expand Down Expand Up @@ -489,7 +483,7 @@ func (n *ProtoNode) Multihash() mh.Multihash {
func (n *ProtoNode) Links() []*format.Link {
if n.linksDirty {
// there was a mutation involving links, make sure we sort
sort.Stable(LinkSlice(n.links))
n.sortLinks()
n.linksDirty = false
n.encoded = nil
}
Expand Down Expand Up @@ -541,7 +535,7 @@ func (n *ProtoNode) Tree(p string, depth int) []string {

if n.linksDirty {
// there was a mutation involving links, make sure we sort
sort.Stable(LinkSlice(n.links))
n.sortLinks()
n.linksDirty = false
n.encoded = nil
}
Expand All @@ -553,6 +547,12 @@ func (n *ProtoNode) Tree(p string, depth int) []string {
return out
}

func (n *ProtoNode) sortLinks() {
slices.SortStableFunc(n.links, func(a, b *format.Link) int {
return strings.Compare(a.Name, b.Name)
})
}

func ProtoNodeConverter(b blocks.Block, nd ipld.Node) (legacy.UniversalNode, error) {
pbNode, ok := nd.(dagpb.PBNode)
if !ok {
Expand Down
15 changes: 11 additions & 4 deletions ipld/unixfs/hamt/hamt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (
"fmt"
"math/rand"
"os"
"sort"
"slices"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -60,6 +61,12 @@ func makeDirWidth(ds ipld.DAGService, size, width int) ([]string, *Shard, error)
return dirs, s, nil
}

func sortLinks(links []*ipld.Link) {
slices.SortStableFunc(links, func(a, b *ipld.Link) int {
return strings.Compare(a.Name, b.Name)
})
}

func assertLink(s *Shard, name string, found bool) error {
_, err := s.Find(context.Background(), name)
switch err {
Expand All @@ -85,8 +92,8 @@ func assertLinksEqual(linksA []*ipld.Link, linksB []*ipld.Link) error {
return errors.New("links arrays are different sizes")
}

sort.Stable(dag.LinkSlice(linksA))
sort.Stable(dag.LinkSlice(linksB))
sortLinks(linksA)
sortLinks(linksB)
for i, a := range linksA {
b := linksB[i]
if a.Name != b.Name {
Expand Down Expand Up @@ -513,7 +520,7 @@ func TestRemoveElemsAfterMarshal(t *testing.T) {
}
ctx := context.Background()

sort.Strings(dirs)
slices.Sort(dirs)

err = s.Remove(ctx, dirs[0])
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions ipld/unixfs/io/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"fmt"
"math"
"sort"
"slices"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -438,9 +438,9 @@ func getAllLinksSortedByName(d Directory) ([]*ipld.Link, error) {
return entries, nil
}

func sortLinksByName(l []*ipld.Link) {
sort.SliceStable(l, func(i, j int) bool {
return strings.Compare(l[i].Name, l[j].Name) == -1 // FIXME: Is this correct?
func sortLinksByName(links []*ipld.Link) {
slices.SortStableFunc(links, func(a, b *ipld.Link) int {
return strings.Compare(a.Name, b.Name)
})
}

Expand Down
14 changes: 8 additions & 6 deletions ipns/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package ipns

import (
"bytes"
"cmp"
"errors"
"fmt"
"sort"
"slices"
"strings"
"time"

ipns_pb "github.com/ipfs/boxo/ipns/pb"
Expand Down Expand Up @@ -321,12 +323,12 @@ func createNode(value path.Path, seq uint64, eol time.Time, ttl time.Duration) (
m[cborTTLKey] = basicnode.NewInt(int64(ttl))
keys = append(keys, cborTTLKey)

sort.Slice(keys, func(i, j int) bool {
li, lj := len(keys[i]), len(keys[j])
if li == lj {
return keys[i] < keys[j]
slices.SortFunc(keys, func(a, b string) int {
la, lb := len(a), len(b)
if la == lb {
return strings.Compare(a, b)
}
return li < lj
return cmp.Compare(la, lb)
})

newNd := basicnode.Prototype__Map{}.NewBuilder()
Expand Down
10 changes: 5 additions & 5 deletions keystore/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
"sort"
"slices"
"testing"

ci "github.com/libp2p/go-libp2p/core/crypto"
Expand Down Expand Up @@ -60,7 +60,7 @@ func TestKeystoreBasics(t *testing.T) {
t.Fatal(err)
}

sort.Strings(l)
slices.Sort(l)
if l[0] != "bar" || l[1] != "foo" {
t.Fatal("wrong entries listed")
}
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestInvalidKeyFiles(t *testing.T) {
t.Fatal(err)
}

sort.Strings(l)
slices.Sort(l)
if len(l) != 1 {
t.Fatal("wrong entry count")
}
Expand Down Expand Up @@ -256,8 +256,8 @@ func assertDirContents(dir string, exp []string) error {
names = append(names, decodedName)
}

sort.Strings(names)
sort.Strings(exp)
slices.Sort(names)
slices.Sort(exp)
if len(names) != len(exp) {
return errors.New("directory had wrong number of entries in it")
}
Expand Down
Loading

0 comments on commit 4a8bdf4

Please sign in to comment.