Skip to content

Commit 5b73737

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 5b73737

File tree

12 files changed

+239
-153
lines changed

12 files changed

+239
-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.NEW_TO_OLD)
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.OLD_TO_NEW)
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: 79 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,91 @@ 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+
const (
46+
ASCENDING = iota
47+
DESCENDING
48+
OLD_TO_NEW
49+
NEW_TO_OLD
50+
)
4851

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
52+
// MachineSetsByCreationTimestamp sorts a list of MachineSet by creation timestamp, using their names as a tie breaker.
53+
// type MachineSetsByCreationTimestamp []*clusterv1.MachineSet
54+
func SortByCreationTimestamp(a, b *clusterv1.MachineSet, order int) int {
55+
if b.CreationTimestamp.Equal(&a.CreationTimestamp) {
56+
if b.Name > a.Name {
57+
if order == ASCENDING {
58+
return -1
59+
}
60+
return 1
61+
} else if b.Name < a.Name {
62+
if order == ASCENDING {
63+
return 1
64+
}
65+
return -1
66+
}
67+
return 0
5768
}
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
69+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
70+
if order == ASCENDING {
71+
return -1
6172
}
62-
return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp)
73+
return 1
74+
} else if b.CreationTimestamp.Before(&a.CreationTimestamp) {
75+
if order == ASCENDING {
76+
return 1
77+
}
78+
return -1
6379
}
64-
return *o[i].Spec.Replicas > *o[j].Spec.Replicas
80+
return 0
6581
}
6682

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
83+
// SortBySize sorts a list of MachineSet by size in descending order, using their creation timestamp or name as a tie breaker.
84+
// By using the creation timestamp, this sorts from old to new machine sets if order is OLD_TO_NEW or new to old machine sets if order is NEW_TO_OLD.
85+
func SortBySize(a, b *clusterv1.MachineSet, order int) int {
86+
if *(a.Spec.Replicas) == *(b.Spec.Replicas) {
87+
if order == OLD_TO_NEW {
88+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
89+
return -1
90+
}
91+
return 1
92+
} else {
93+
if b.CreationTimestamp.Before(&a.CreationTimestamp) {
94+
return -1
95+
}
96+
return 1
97+
}
7598
}
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)
99+
if *(a.Spec.Replicas) > *(b.Spec.Replicas) {
100+
return -1
88101
}
89-
return *(o[i].Spec.Replicas) > *(o[j].Spec.Replicas)
102+
return 1
90103
}
91104

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)
105+
// MachineSetsByDecreasingReplicas sorts the list of MachineSets in decreasing order of replicas,
106+
// using creation time (ascending order) and name (alphabetical) as tie breakers.
107+
func SortByDecreasingReplicas(a, b *clusterv1.MachineSet) int {
108+
if a.Spec.Replicas == nil {
109+
return 1
110+
}
111+
if b.Spec.Replicas == nil {
112+
return -1
113+
}
114+
if *a.Spec.Replicas == *b.Spec.Replicas {
115+
if a.CreationTimestamp.Equal(&b.CreationTimestamp) {
116+
if a.Name < b.Name {
117+
return -1
118+
}
119+
return 1
120+
}
121+
if a.CreationTimestamp.Before(&b.CreationTimestamp) {
122+
return -1
123+
}
124+
return 1
125+
}
126+
if *a.Spec.Replicas > *b.Spec.Replicas {
127+
return -1
101128
}
102-
return *(o[i].Spec.Replicas) > *(o[j].Spec.Replicas)
129+
return 1
103130
}
104131

105132
// SetDeploymentRevision updates the revision for a deployment.
@@ -256,7 +283,9 @@ func FindOneActiveOrLatest(newMS *clusterv1.MachineSet, oldMSs []*clusterv1.Mach
256283
return nil
257284
}
258285

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

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

472501
var matchingMachineSets []*clusterv1.MachineSet
473502
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: 8 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,13 @@ 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+
})
146+
fmt.Println("========== here ==========", parsedVersions)
141147

142148
return parsedVersions, nil
143149
}

0 commit comments

Comments
 (0)