Skip to content

Commit 66d3377

Browse files
Surya-7890Surya-7890
committed
refactor: replace sort.Sort with slices.SortFunc
This commit replaces all the instances of sort.Sort with slices.SortFunc. The godocs for sort.Sort recommends using slices.SortFunc, as it is faster and more efficient.
1 parent d50f243 commit 66d3377

File tree

12 files changed

+240
-153
lines changed

12 files changed

+240
-153
lines changed

‎internal/contract/version.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package contract
1919
import (
2020
"context"
2121
"fmt"
22-
"sort"
22+
"slices"
2323
"strings"
2424

2525
"github.com/pkg/errors"
@@ -123,7 +123,9 @@ func GetLatestContractAndAPIVersionFromContract(metadata metav1.Object, currentC
123123
labels := metadata.GetLabels()
124124

125125
sortedCompatibleContractVersions := kubeAwareAPIVersions(GetCompatibleVersions(currentContractVersion).UnsortedList())
126-
sort.Sort(sort.Reverse(sortedCompatibleContractVersions))
126+
slices.SortFunc(sortedCompatibleContractVersions, func(i, j string) int {
127+
return sortedCompatibleContractVersions.SortFunc(i, j, descending)
128+
})
127129

128130
for _, contractVersion := range sortedCompatibleContractVersions {
129131
contractGroupVersion := fmt.Sprintf("%s/%s", clusterv1.GroupVersion.Group, contractVersion)
@@ -136,7 +138,9 @@ func GetLatestContractAndAPIVersionFromContract(metadata metav1.Object, currentC
136138

137139
// Pick the latest version in the slice and validate it.
138140
kubeVersions := kubeAwareAPIVersions(strings.Split(supportedVersions, "_"))
139-
sort.Sort(kubeVersions)
141+
slices.SortFunc(kubeVersions, func(i, j string) int {
142+
return kubeVersions.SortFunc(i, j, ascending)
143+
})
140144
return contractVersion, kubeVersions[len(kubeVersions)-1], nil
141145
}
142146

@@ -188,8 +192,20 @@ func GetGKMetadata(ctx context.Context, c client.Reader, gk schema.GroupKind) (*
188192
// versions. e.g. v2, v1, v1beta2, v1beta1, v1alpha1.
189193
type kubeAwareAPIVersions []string
190194

191-
func (k kubeAwareAPIVersions) Len() int { return len(k) }
192-
func (k kubeAwareAPIVersions) Swap(i, j int) { k[i], k[j] = k[j], k[i] }
193-
func (k kubeAwareAPIVersions) Less(i, j int) bool {
194-
return k8sversion.CompareKubeAwareVersionStrings(k[i], k[j]) < 0
195+
const (
196+
ascending = iota
197+
descending
198+
)
199+
200+
func (k kubeAwareAPIVersions) SortFunc(i, j string, order int) int {
201+
if k8sversion.CompareKubeAwareVersionStrings(i, j) < 0 {
202+
if order == ascending {
203+
return -1
204+
}
205+
return 1
206+
}
207+
if order == ascending {
208+
return 1
209+
}
210+
return -1
195211
}

‎internal/controllers/machinedeployment/machinedeployment_rolling.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package machinedeployment
1818

1919
import (
2020
"context"
21-
"sort"
21+
"slices"
2222

2323
"github.com/pkg/errors"
2424
"k8s.io/klog/v2"
@@ -192,7 +192,9 @@ func (r *Reconciler) reconcileOldMachineSets(ctx context.Context, allMSs []*clus
192192
func (r *Reconciler) cleanupUnhealthyReplicas(ctx context.Context, oldMSs []*clusterv1.MachineSet, deployment *clusterv1.MachineDeployment, maxCleanupCount int32) ([]*clusterv1.MachineSet, int32, error) {
193193
log := ctrl.LoggerFrom(ctx)
194194

195-
sort.Sort(mdutil.MachineSetsByCreationTimestamp(oldMSs))
195+
slices.SortFunc(oldMSs, func(a, b *clusterv1.MachineSet) int {
196+
return mdutil.SortByCreationTimestamp(a, b, mdutil.ASCENDING)
197+
})
196198

197199
// Scale down all old MachineSets with any unhealthy replicas. MachineSet will honour Spec.DeletePolicy
198200
// for deleting Machines. Machines with a deletion timestamp, with a failure message or without a nodeRef
@@ -266,7 +268,9 @@ func (r *Reconciler) scaleDownOldMachineSetsForRollingUpdate(ctx context.Context
266268

267269
log.V(4).Info("Found available machines in deployment, scaling down old MSes", "count", availableMachineCount)
268270

269-
sort.Sort(mdutil.MachineSetsByCreationTimestamp(oldMSs))
271+
slices.SortFunc(oldMSs, func(a, b *clusterv1.MachineSet) int {
272+
return mdutil.SortByCreationTimestamp(a, b, mdutil.ASCENDING)
273+
})
270274

271275
totalScaledDown := int32(0)
272276
totalScaleDownCount := availableMachineCount - minAvailable

‎internal/controllers/machinedeployment/machinedeployment_sync.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package machinedeployment
1919
import (
2020
"context"
2121
"fmt"
22-
"sort"
22+
"slices"
2323
"time"
2424

2525
"github.com/pkg/errors"
@@ -424,9 +424,13 @@ func (r *Reconciler) scale(ctx context.Context, deployment *clusterv1.MachineDep
424424
// when scaling down, we should scale down older machine sets first.
425425
switch {
426426
case deploymentReplicasToAdd > 0:
427-
sort.Sort(mdutil.MachineSetsBySizeNewer(allMSs))
427+
slices.SortFunc(allMSs, func(a, b *clusterv1.MachineSet) int {
428+
return mdutil.SortBySize(a, b, mdutil.DESCENDING)
429+
})
428430
case deploymentReplicasToAdd < 0:
429-
sort.Sort(mdutil.MachineSetsBySizeOlder(allMSs))
431+
slices.SortFunc(allMSs, func(a, b *clusterv1.MachineSet) int {
432+
return mdutil.SortBySize(a, b, mdutil.ASCENDING)
433+
})
430434
}
431435

432436
// Iterate over all active machine sets and estimate proportions for each of them.
@@ -594,7 +598,9 @@ func (r *Reconciler) cleanupDeployment(ctx context.Context, oldMSs []*clusterv1.
594598
return nil
595599
}
596600

597-
sort.Sort(mdutil.MachineSetsByCreationTimestamp(cleanableMSes))
601+
slices.SortFunc(cleanableMSes, func(a, b *clusterv1.MachineSet) int {
602+
return mdutil.SortByCreationTimestamp(a, b, mdutil.ASCENDING)
603+
})
598604
log.V(4).Info("Looking to cleanup old machine sets for deployment")
599605

600606
for i := range cleanableMSCount {

‎internal/controllers/machinedeployment/mdutil/util.go‎

Lines changed: 81 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"context"
2222
"fmt"
2323
"reflect"
24-
"sort"
24+
"slices"
2525
"strconv"
2626
"strings"
2727
"time"
@@ -42,64 +42,93 @@ import (
4242
"sigs.k8s.io/cluster-api/util/conversion"
4343
)
4444

45-
// MachineSetsByDecreasingReplicas sorts the list of MachineSets in decreasing order of replicas,
46-
// using creation time (ascending order) and name (alphabetical) as tie breakers.
47-
type MachineSetsByDecreasingReplicas []*clusterv1.MachineSet
45+
// Sort functions in this file, use these constants to determine the sorting order.
46+
const (
47+
// Used to sort elements in Ascending order.
48+
// Also used in cases, where the order is from older to newer occurrences.
49+
ASCENDING = iota
4850

49-
func (o MachineSetsByDecreasingReplicas) Len() int { return len(o) }
50-
func (o MachineSetsByDecreasingReplicas) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
51-
func (o MachineSetsByDecreasingReplicas) Less(i, j int) bool {
52-
if o[i].Spec.Replicas == nil {
53-
return false
54-
}
55-
if o[j].Spec.Replicas == nil {
56-
return true
51+
// Used to sort elements in Descending order.
52+
// Also used in cases, where the order is from newer to older occurrences.
53+
DESCENDING
54+
)
55+
56+
// SortByCreationTimestamp sorts a list of MachineSet by creation timestamp, using their names as a tie breaker.
57+
func SortByCreationTimestamp(a, b *clusterv1.MachineSet, order int) int {
58+
if b.CreationTimestamp.Equal(&a.CreationTimestamp) {
59+
if b.Name > a.Name {
60+
if order == ASCENDING {
61+
return -1
62+
}
63+
return 1
64+
} else if b.Name < a.Name {
65+
if order == ASCENDING {
66+
return 1
67+
}
68+
return -1
69+
}
70+
return 0
5771
}
58-
if *o[i].Spec.Replicas == *o[j].Spec.Replicas {
59-
if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) {
60-
return o[i].Name < o[j].Name
72+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
73+
if order == ASCENDING {
74+
return -1
75+
}
76+
return 1
77+
} else if b.CreationTimestamp.Before(&a.CreationTimestamp) {
78+
if order == ASCENDING {
79+
return 1
6180
}
62-
return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp)
81+
return -1
6382
}
64-
return *o[i].Spec.Replicas > *o[j].Spec.Replicas
83+
return 0
6584
}
6685

67-
// MachineSetsByCreationTimestamp sorts a list of MachineSet by creation timestamp, using their names as a tie breaker.
68-
type MachineSetsByCreationTimestamp []*clusterv1.MachineSet
69-
70-
func (o MachineSetsByCreationTimestamp) Len() int { return len(o) }
71-
func (o MachineSetsByCreationTimestamp) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
72-
func (o MachineSetsByCreationTimestamp) Less(i, j int) bool {
73-
if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) {
74-
return o[i].Name < o[j].Name
86+
// SortBySize sorts a list of MachineSet by size in descending order, using their creation timestamp or name as a tie breaker.
87+
// By using the creation timestamp, this sorts from old to new machine sets if order is ASCENDING or new to old machine sets if order is DESCENDING.
88+
func SortBySize(a, b *clusterv1.MachineSet, order int) int {
89+
if *(a.Spec.Replicas) == *(b.Spec.Replicas) {
90+
if order == ASCENDING {
91+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
92+
return -1
93+
}
94+
return 1
95+
}
96+
if b.CreationTimestamp.Before(&a.CreationTimestamp) {
97+
return -1
98+
}
99+
return 1
75100
}
76-
return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp)
77-
}
78-
79-
// MachineSetsBySizeOlder sorts a list of MachineSet by size in descending order, using their creation timestamp or name as a tie breaker.
80-
// By using the creation timestamp, this sorts from old to new machine sets.
81-
type MachineSetsBySizeOlder []*clusterv1.MachineSet
82-
83-
func (o MachineSetsBySizeOlder) Len() int { return len(o) }
84-
func (o MachineSetsBySizeOlder) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
85-
func (o MachineSetsBySizeOlder) Less(i, j int) bool {
86-
if *(o[i].Spec.Replicas) == *(o[j].Spec.Replicas) {
87-
return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp)
101+
if *(a.Spec.Replicas) > *(b.Spec.Replicas) {
102+
return -1
88103
}
89-
return *(o[i].Spec.Replicas) > *(o[j].Spec.Replicas)
104+
return 1
90105
}
91106

92-
// MachineSetsBySizeNewer sorts a list of MachineSet by size in descending order, using their creation timestamp or name as a tie breaker.
93-
// By using the creation timestamp, this sorts from new to old machine sets.
94-
type MachineSetsBySizeNewer []*clusterv1.MachineSet
95-
96-
func (o MachineSetsBySizeNewer) Len() int { return len(o) }
97-
func (o MachineSetsBySizeNewer) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
98-
func (o MachineSetsBySizeNewer) Less(i, j int) bool {
99-
if *(o[i].Spec.Replicas) == *(o[j].Spec.Replicas) {
100-
return o[j].CreationTimestamp.Before(&o[i].CreationTimestamp)
107+
// SortByDecreasingReplicas sorts the list of MachineSets in decreasing order of replicas,
108+
// using creation time (ascending order) and name (alphabetical) as tie breakers.
109+
func SortByDecreasingReplicas(a, b *clusterv1.MachineSet) int {
110+
if a.Spec.Replicas == nil {
111+
return 1
112+
}
113+
if b.Spec.Replicas == nil {
114+
return -1
115+
}
116+
if *a.Spec.Replicas == *b.Spec.Replicas {
117+
if a.CreationTimestamp.Equal(&b.CreationTimestamp) {
118+
if a.Name < b.Name {
119+
return -1
120+
}
121+
return 1
122+
}
123+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
124+
return -1
125+
}
126+
return 1
127+
}
128+
if *a.Spec.Replicas > *b.Spec.Replicas {
129+
return -1
101130
}
102-
return *(o[i].Spec.Replicas) > *(o[j].Spec.Replicas)
131+
return 1
103132
}
104133

105134
// SetDeploymentRevision updates the revision for a deployment.
@@ -256,7 +285,9 @@ func FindOneActiveOrLatest(newMS *clusterv1.MachineSet, oldMSs []*clusterv1.Mach
256285
return nil
257286
}
258287

259-
sort.Sort(sort.Reverse(MachineSetsByCreationTimestamp(oldMSs)))
288+
slices.SortFunc(oldMSs, func(a, b *clusterv1.MachineSet) int {
289+
return SortByCreationTimestamp(a, b, DESCENDING)
290+
})
260291
allMSs := FilterActiveMachineSets(append(oldMSs, newMS))
261292

262293
switch len(allMSs) {
@@ -467,7 +498,7 @@ func FindNewMachineSet(deployment *clusterv1.MachineDeployment, msList []*cluste
467498
// having more than one new MachineSets that have the same template,
468499
// see https://github.com/kubernetes/kubernetes/issues/40415
469500
// We deterministically choose the oldest new MachineSet with matching template hash.
470-
sort.Sort(MachineSetsByDecreasingReplicas(msList))
501+
slices.SortFunc(msList, SortByDecreasingReplicas)
471502

472503
var matchingMachineSets []*clusterv1.MachineSet
473504
var diffs []string

‎internal/controllers/machinedeployment/mdutil/util_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package mdutil
1919
import (
2020
"fmt"
2121
"math/rand"
22-
"sort"
22+
"slices"
2323
"strconv"
2424
"testing"
2525
"time"
@@ -165,7 +165,7 @@ func TestMachineSetsByDecreasingReplicas(t *testing.T) {
165165
t.Run(tt.name, func(t *testing.T) {
166166
// sort the machine sets and verify the sorted list
167167
g := NewWithT(t)
168-
sort.Sort(MachineSetsByDecreasingReplicas(tt.machineSets))
168+
slices.SortFunc(tt.machineSets, SortByDecreasingReplicas)
169169
g.Expect(tt.machineSets).To(BeComparableTo(tt.want))
170170
})
171171
}

‎internal/controllers/machineset/machineset_delete_policy.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package machineset
1818

1919
import (
2020
"math"
21-
"sort"
21+
"slices"
2222

2323
"github.com/pkg/errors"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -94,16 +94,20 @@ type sortableMachines struct {
9494
priority deletePriorityFunc
9595
}
9696

97-
func (m sortableMachines) Len() int { return len(m.machines) }
98-
func (m sortableMachines) Swap(i, j int) { m.machines[i], m.machines[j] = m.machines[j], m.machines[i] }
99-
func (m sortableMachines) Less(i, j int) bool {
100-
priorityI, priorityJ := m.priority(m.machines[i]), m.priority(m.machines[j])
97+
func (m sortableMachines) SortFunc(i, j *clusterv1.Machine) int {
98+
priorityI, priorityJ := m.priority(i), m.priority(j)
10199
if priorityI == priorityJ {
102100
// In cases where the priority is identical, it should be ensured that the same machine order is returned each time.
103101
// Ordering by name is a simple way to do this.
104-
return m.machines[i].Name < m.machines[j].Name
102+
if i.Name < j.Name {
103+
return -1
104+
}
105+
return 1
105106
}
106-
return priorityJ < priorityI // high to low
107+
if priorityJ < priorityI { // high to low
108+
return -1
109+
}
110+
return 1
107111
}
108112

109113
func getMachinesToDeletePrioritized(filteredMachines []*clusterv1.Machine, diff int, fun deletePriorityFunc) []*clusterv1.Machine {
@@ -117,7 +121,7 @@ func getMachinesToDeletePrioritized(filteredMachines []*clusterv1.Machine, diff
117121
machines: filteredMachines,
118122
priority: fun,
119123
}
120-
sort.Sort(sortable)
124+
slices.SortFunc(sortable.machines, sortable.SortFunc)
121125

122126
return sortable.machines[:diff]
123127
}

‎internal/goproxy/goproxy.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"net/url"
2525
"path"
2626
"path/filepath"
27-
"sort"
27+
"slices"
2828
"strings"
2929
"time"
3030

@@ -137,7 +137,12 @@ func (g *Client) GetVersions(ctx context.Context, gomodulePath string) (semver.V
137137
return nil, fmt.Errorf("no versions found for go module %q", gomodulePath)
138138
}
139139

140-
sort.Sort(parsedVersions)
140+
slices.SortFunc(parsedVersions, func(i, j semver.Version) int {
141+
if i.LT(j) {
142+
return -1
143+
}
144+
return 1
145+
})
141146

142147
return parsedVersions, nil
143148
}

0 commit comments

Comments
 (0)