Skip to content

refactor: use slices.SortFunc instead of sort.Slice #1191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

alexandear
Copy link
Collaborator

@alexandear alexandear commented Dec 13, 2024

This PR replaces sort.Slice with slices.SortFunc in Friendly.printStatistics. See https://go-review.googlesource.com/c/go/+/626038 for reasons.

The PR adds TestFriendly_printStatistics to check that sorting works as expected, as it was not covered by any test before.

@alexandear alexandear force-pushed the refactor-friendly-printstatistics branch from b99eaa6 to 426b35e Compare December 13, 2024 12:43
@denisvmedia
Copy link
Collaborator

denisvmedia commented Dec 13, 2024

I made an isolated benchmark, and the new method is faster:

package main

import (
	"cmp"
	"math/rand"
	"sort"
	"slices"
	"testing"
	"time"
)

type statEntry struct {
	failures int
}

func BenchmarkSortSlice(b *testing.B) {
	data := generateTestData(100000)

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		temp := make([]statEntry, len(data))
		copy(temp, data)

		sort.Slice(temp, func(i, j int) bool {
			return temp[i].failures > temp[j].failures
		})
	}
}

func BenchmarkSlicesSortFunc(b *testing.B) {
	data := generateTestData(100000)

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		temp := make([]statEntry, len(data))
		copy(temp, data)

		slices.SortFunc(temp, func(a, b statEntry) int {
			return -cmp.Compare(a.failures, b.failures)
		})
	}
}

func generateTestData(size int) []statEntry {
	rand.Seed(time.Now().UnixNano())
	data := make([]statEntry, size)
	for i := 0; i < size; i++ {
		data[i] = statEntry{failures: rand.Intn(1000000)}
	}
	return data
}
user@mac % go test -bench=. -benchtime=10s
goos: darwin
goarch: arm64
pkg: benchit
cpu: Apple M3 Pro
BenchmarkSortSlice-12         	    1440	   8164691 ns/op
BenchmarkSlicesSortFunc-12    	    1690	   7128573 ns/op
PASS
ok  	benchit	25.571s

@chavacava chavacava merged commit 7cbd3d1 into mgechev:master Dec 13, 2024
5 checks passed
@alexandear alexandear deleted the refactor-friendly-printstatistics branch December 13, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants