From af9d0b995f162988d15e9f3df2d488f1dcdbf2cd Mon Sep 17 00:00:00 2001 From: Vishnu Kothakapu Date: Thu, 20 Aug 2026 23:31:55 +0530 Subject: [PATCH] Performance: Upgrade sort.Slice to slices.SortFunc across codebase This commit modernizes the codebase by replacing older, reflection-based sort.Slice and sort.SliceIsSorted calls with the modern slices.Sort and slices.SortFunc introduced in Go 1.21. This provides better performance and type safety by utilizing generics rather than runtime reflection. Signed-off-by: Vishnu Kothakapu --- cmd/podman/farm/list.go | 7 ++++--- cmd/podman/networks/list.go | 7 ++++--- cmd/podman/system/connection/list.go | 6 +++--- cmd/quadlet/main.go | 10 ++++++---- libpod/boltdb_state_internal.go | 23 +++++++++++------------ libpod/kube.go | 3 +-- libpod/pod.go | 4 ++-- pkg/rootless/rootless.go | 6 ++++-- pkg/specgen/generate/ports.go | 18 +++++++++--------- pkg/util/utils.go | 19 ++++++++++--------- test/e2e/images_test.go | 21 ++++++++++----------- test/e2e/pod_ps_test.go | 4 ++-- test/e2e/ps_test.go | 15 ++++++++------- 13 files changed, 74 insertions(+), 69 deletions(-) diff --git a/cmd/podman/farm/list.go b/cmd/podman/farm/list.go index 480181c972..e1b479c7fd 100644 --- a/cmd/podman/farm/list.go +++ b/cmd/podman/farm/list.go @@ -1,10 +1,11 @@ package farm import ( + "cmp" "errors" "fmt" "os" - "sort" + "slices" "github.com/spf13/cobra" "go.podman.io/common/pkg/completion" @@ -68,8 +69,8 @@ func list(cmd *cobra.Command, args []string) error { return err } - sort.Slice(farms, func(i, j int) bool { - return farms[i].Name < farms[j].Name + slices.SortFunc(farms, func(a, b config.Farm) int { + return cmp.Compare(a.Name, b.Name) }) rpt := report.New(os.Stdout, cmd.Name()) diff --git a/cmd/podman/networks/list.go b/cmd/podman/networks/list.go index 29d8b26e84..401c45d882 100644 --- a/cmd/podman/networks/list.go +++ b/cmd/podman/networks/list.go @@ -1,9 +1,10 @@ package network import ( + "cmp" "fmt" "os" - "sort" + "slices" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -72,8 +73,8 @@ func networkList(cmd *cobra.Command, _ []string) error { return err } // sort the networks to make sure the order is deterministic - sort.Slice(responses, func(i, j int) bool { - return responses[i].Name < responses[j].Name + slices.SortFunc(responses, func(a, b types.Network) int { + return cmp.Compare(a.Name, b.Name) }) switch { diff --git a/cmd/podman/system/connection/list.go b/cmd/podman/system/connection/list.go index fcfb3178ee..c886a27c47 100644 --- a/cmd/podman/system/connection/list.go +++ b/cmd/podman/system/connection/list.go @@ -1,10 +1,10 @@ package connection import ( + "cmp" "fmt" "os" "slices" - "sort" "github.com/spf13/cobra" "go.podman.io/common/pkg/completion" @@ -105,8 +105,8 @@ func inspect(cmd *cobra.Command, args []string) error { return nil } - sort.Slice(rows, func(i, j int) bool { - return rows[i].Name < rows[j].Name + slices.SortFunc(rows, func(a, b config.Connection) int { + return cmp.Compare(a.Name, b.Name) }) rpt := report.New(os.Stdout, cmd.Name()) diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index cbf34b016c..de8fa5e276 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -1,11 +1,13 @@ package main import ( + "cmp" "errors" "flag" "fmt" "os" "path/filepath" + "slices" "sort" "strings" "unicode" @@ -510,16 +512,16 @@ func process() bool { // Sort unit files according to potential inter-dependencies, with Volume and Network units // taking precedence over all others. - sort.Slice(units, func(i, j int) bool { - getOrder := func(i int) int { - ext := filepath.Ext(units[i].Filename) + slices.SortFunc(units, func(a, b *parser.UnitFile) int { + getOrder := func(filename string) int { + ext := filepath.Ext(filename) order, ok := quadlet.SupportedExtensions[ext] if !ok { return 0 } return order } - return getOrder(i) < getOrder(j) + return cmp.Compare(getOrder(a.Filename), getOrder(b.Filename)) }) // Generate the PodsInfoMap to allow containers to link to their pods and add themselves to the pod's containers list diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index d656cc316e..8bb4438b58 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -3,9 +3,10 @@ package libpod import ( + "cmp" "fmt" "os" - "sort" + "slices" "strings" "github.com/sirupsen/logrus" @@ -337,9 +338,7 @@ func ocicniPortsToNetTypesPorts(ports []types.OCICNIPortMapping) []types.PortMap newPorts := make([]types.PortMapping, 0, len(ports)) // first sort the ports - sort.Slice(ports, func(i, j int) bool { - return compareOCICNIPorts(ports[i], ports[j]) - }) + slices.SortFunc(ports, compareOCICNIPorts) // we already check if the slice is empty so we can use the first element currentPort := types.PortMapping{ @@ -378,18 +377,18 @@ func ocicniPortsToNetTypesPorts(ports []types.OCICNIPortMapping) []types.PortMap // 4) container port // //nolint:staticcheck // OCICNIPortMapping is deprecated; kept for backwards-compatible DB migration -func compareOCICNIPorts(i, j types.OCICNIPortMapping) bool { - if i.HostIP != j.HostIP { - return i.HostIP < j.HostIP +func compareOCICNIPorts(i, j types.OCICNIPortMapping) int { + if c := cmp.Compare(i.HostIP, j.HostIP); c != 0 { + return c } - if i.Protocol != j.Protocol { - return i.Protocol < j.Protocol + if c := cmp.Compare(i.Protocol, j.Protocol); c != 0 { + return c } - if i.HostPort != j.HostPort { - return i.HostPort < j.HostPort + if c := cmp.Compare(i.HostPort, j.HostPort); c != 0 { + return c } - return i.ContainerPort < j.ContainerPort + return cmp.Compare(i.ContainerPort, j.ContainerPort) } diff --git a/libpod/kube.go b/libpod/kube.go index 7fd362e798..f1a79bc095 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -11,7 +11,6 @@ import ( "os" "reflect" "slices" - "sort" "strconv" "strings" "time" @@ -586,7 +585,7 @@ func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, po // Let's sort the containers in order of created time // This will ensure that the init containers are defined in the correct order in the kube yaml - sort.Slice(containers, func(i, j int) bool { return containers[i].CreatedTime().Before(containers[j].CreatedTime()) }) + slices.SortFunc(containers, func(a, b *Container) int { return a.CreatedTime().Compare(b.CreatedTime()) }) for _, ctr := range containers { if ctr.IsInfra() { diff --git a/libpod/pod.go b/libpod/pod.go index d5a1413496..32ef3ccb35 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "maps" - "sort" + "slices" "strings" "time" @@ -506,7 +506,7 @@ func (p *Pod) initContainers() ([]*Container, error) { return nil, err } // Sort the pod containers by created time - sort.Slice(cons, func(i, j int) bool { return cons[i].CreatedTime().Before(cons[j].CreatedTime()) }) + slices.SortFunc(cons, func(a, b *Container) int { return a.CreatedTime().Compare(b.CreatedTime()) }) // Iterate sorted containers and add ids for any init containers for _, c := range cons { if len(c.config.InitContainerType) > 0 { diff --git a/pkg/rootless/rootless.go b/pkg/rootless/rootless.go index 024a4afcba..d39fa46d15 100644 --- a/pkg/rootless/rootless.go +++ b/pkg/rootless/rootless.go @@ -1,10 +1,12 @@ package rootless import ( + "cmp" "errors" "fmt" "os" "path/filepath" + "slices" "sort" "sync" @@ -167,8 +169,8 @@ func MaybeSplitMappings(mappings []spec.LinuxIDMapping, availableMappings []user var overflow spec.LinuxIDMapping overflow.Size = 0 consumed := 0 - sort.Slice(availableMappings, func(i, j int) bool { - return availableMappings[i].ID > availableMappings[j].ID + slices.SortFunc(availableMappings, func(a, b user.IDMap) int { + return cmp.Compare(b.ID, a.ID) }) for { cur := overflow diff --git a/pkg/specgen/generate/ports.go b/pkg/specgen/generate/ports.go index 397f5f6a30..e251aebf66 100644 --- a/pkg/specgen/generate/ports.go +++ b/pkg/specgen/generate/ports.go @@ -3,10 +3,10 @@ package generate import ( + "cmp" "fmt" "net" "slices" - "sort" "strconv" "strings" @@ -228,17 +228,17 @@ func ParsePortMapping(portMappings []types.PortMapping, exposePorts map[uint16][ } // 1. sort the ports by host port // use a small hack to make sure ports with host port 0 are sorted last - sort.Slice(ports, func(i, j int) bool { - if ports[i].hostPort == ports[j].hostPort { - return ports[i].containerPort < ports[j].containerPort + slices.SortFunc(ports, func(a, b tempMapping) int { + if a.hostPort == b.hostPort { + return cmp.Compare(a.containerPort, b.containerPort) } - if ports[i].hostPort == 0 { - return false + if a.hostPort == 0 { + return 1 } - if ports[j].hostPort == 0 { - return true + if b.hostPort == 0 { + return -1 } - return ports[i].hostPort < ports[j].hostPort + return cmp.Compare(a.hostPort, b.hostPort) }) allUsedContainerPorts := allUsedContainerPortsMap[protocol] diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 1cf35f5080..83f4ec3ba7 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -1,6 +1,7 @@ package util import ( + "cmp" "errors" "fmt" "io/fs" @@ -10,7 +11,7 @@ import ( "os/user" "path/filepath" "regexp" - "sort" + "slices" "strconv" "strings" "syscall" @@ -551,16 +552,16 @@ func breakInsert(mapping []idtools.IDMap, extension idtools.IDMap) (result []idt // containing all integers found in fullRanges and not found in usedRanges. func getAvailableIDRanges(fullRanges, usedRanges [][2]int) (availableRanges [][2]int) { // Sort them - sort.Slice(fullRanges, func(i, j int) bool { - return fullRanges[i][0] < fullRanges[j][0] + slices.SortFunc(fullRanges, func(a, b [2]int) int { + return cmp.Compare(a[0], b[0]) }) if len(usedRanges) == 0 { return fullRanges } - sort.Slice(usedRanges, func(i, j int) bool { - return usedRanges[i][0] < usedRanges[j][0] + slices.SortFunc(usedRanges, func(a, b [2]int) int { + return cmp.Compare(a[0], b[0]) }) // To traverse usedRanges @@ -637,8 +638,8 @@ func getAvailableIDRangesFromMappings(idmap []idtools.IDMap, parentMapping []rus // Returns the filled idmap. func fillIDMap(idmap []idtools.IDMap, availableRanges [][2]int) (output []idtools.IDMap) { idmapByCid := append([]idtools.IDMap{}, idmap...) - sort.Slice(idmapByCid, func(i, j int) bool { - return idmapByCid[i].ContainerID < idmapByCid[j].ContainerID + slices.SortFunc(idmapByCid, func(a, b idtools.IDMap) int { + return cmp.Compare(a.ContainerID, b.ContainerID) }) if len(availableRanges) == 0 { @@ -778,8 +779,8 @@ func ParseIDMap(mapSpec []string, mapSetting string, parentMapping []ruser.IDMap // entries that are consecutive. func sortAndMergeConsecutiveMappings(idmap []idtools.IDMap) (finalIDMap []idtools.IDMap) { idmapByCid := append([]idtools.IDMap{}, idmap...) - sort.Slice(idmapByCid, func(i, j int) bool { - return idmapByCid[i].ContainerID < idmapByCid[j].ContainerID + slices.SortFunc(idmapByCid, func(a, b idtools.IDMap) int { + return cmp.Compare(a.ContainerID, b.ContainerID) }) for i, mapPiece := range idmapByCid { if i == 0 { diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go index b432b29f1d..49faebabf9 100644 --- a/test/e2e/images_test.go +++ b/test/e2e/images_test.go @@ -3,9 +3,10 @@ package integration import ( + "cmp" "encoding/json" "fmt" - "sort" + "slices" "strings" "github.com/docker/go-units" @@ -322,24 +323,22 @@ WORKDIR /test } sortedArr := sortValueTest("created", 0, "CreatedAt") - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] > sortedArr[j] })).To(BeTrue()) + Expect(slices.IsSortedFunc(sortedArr, func(a, b string) int { return cmp.Compare(b, a) })).To(BeTrue()) sortedArr = sortValueTest("id", 0, "ID") - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue()) + Expect(slices.IsSorted(sortedArr)).To(BeTrue()) sortedArr = sortValueTest("repository", 0, "Repository") - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue()) + Expect(slices.IsSorted(sortedArr)).To(BeTrue()) sortedArr = sortValueTest("size", 0, "Size") - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { - size1, _ := units.FromHumanSize(sortedArr[i]) - size2, _ := units.FromHumanSize(sortedArr[j]) - return size1 < size2 + Expect(slices.IsSortedFunc(sortedArr, func(a, b string) int { + size1, _ := units.FromHumanSize(a) + size2, _ := units.FromHumanSize(b) + return cmp.Compare(size1, size2) })).To(BeTrue()) sortedArr = sortValueTest("tag", 0, "Tag") - Expect(sort.SliceIsSorted(sortedArr, - func(i, j int) bool { return sortedArr[i] < sortedArr[j] })). - To(BeTrue()) + Expect(slices.IsSorted(sortedArr)).To(BeTrue()) sortValueTest("badvalue", 125, "Tag") sortValueTest("id", 125, "badvalue") diff --git a/test/e2e/pod_ps_test.go b/test/e2e/pod_ps_test.go index 42c32ee270..ece8215276 100644 --- a/test/e2e/pod_ps_test.go +++ b/test/e2e/pod_ps_test.go @@ -4,7 +4,7 @@ package integration import ( "fmt" - "sort" + "slices" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -151,7 +151,7 @@ var _ = Describe("Podman ps", func() { sortedArr := session.OutputToStringArray() - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue(), "slice is sorted") + Expect(slices.IsSorted(sortedArr)).To(BeTrue(), "slice is sorted") }) It("podman pod ps --ctr-names", func() { diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go index 6d4fd3f02b..e6c72151ca 100644 --- a/test/e2e/ps_test.go +++ b/test/e2e/ps_test.go @@ -3,9 +3,10 @@ package integration import ( + "cmp" "fmt" "regexp" - "sort" + "slices" "strconv" "github.com/docker/go-units" @@ -507,18 +508,18 @@ var _ = Describe("Podman ps", func() { // TODO: This may be broken - the test was running without the // ability to perform any sorting for months and succeeded // without error. - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { + Expect(slices.IsSortedFunc(sortedArr, func(a, b string) int { r := regexp.MustCompile(`^\S+\s+\(virtual (\S+)\)`) - matches1 := r.FindStringSubmatch(sortedArr[i]) - matches2 := r.FindStringSubmatch(sortedArr[j]) + matches1 := r.FindStringSubmatch(a) + matches2 := r.FindStringSubmatch(b) // sanity check in case an oddly formatted size appears if len(matches1) < 2 || len(matches2) < 2 { - return sortedArr[i] < sortedArr[j] + return cmp.Compare(a, b) } size1, _ := units.FromHumanSize(matches1[1]) size2, _ := units.FromHumanSize(matches2[1]) - return size1 < size2 + return cmp.Compare(size1, size2) })).To(BeTrue(), "slice is sorted") }) @@ -538,7 +539,7 @@ var _ = Describe("Podman ps", func() { Expect(session.OutputToString()).ToNot(ContainSubstring("COMMAND")) sortedArr := session.OutputToStringArray() - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue(), "slice is sorted") + Expect(slices.IsSorted(sortedArr)).To(BeTrue(), "slice is sorted") }) It("podman --pod", func() {