*: adopt "slices" stdlib package for sorting #124363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nvanbenschoten
requested review from
abarganier,
nameisbhaskar,
vidit-bhat,
DrewKimball,
dt and
wenyihu6
and removed request for
a team
|
This change is |
nvanbenschoten
force-pushed
the
nvanbenschoten/slicesPkg
branch
2 times, most recently
from
104d9d7
to
7ee1c41
Compare
nvanbenschoten
force-pushed
the
nvanbenschoten/slicesPkg
branch
from
7ee1c41
to
2ce5985
Compare
kvoli
approved these changes
May 20, 2024
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, 12 of 12 files at r2, 1 of 1 files at r3, 20 of 20 files at r4, 23 of 23 files at r5, 1 of 1 files at r6, 6 of 6 files at r7, 38 of 38 files at r8, 11 of 11 files at r9, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @abarganier, @DrewKimball, @dt, @nameisbhaskar, @nvanbenschoten, @vidit-bhat, and @wenyihu6)
pkg/sql/sem/tree/datum.go line 4817 at r7 (raw file):
func (d *DTuple) sort(ctx CompareContext) { if !d.sorted { lessFn := func(a, b Datum) int {
nit: consider rename to sortFn or similar.
nvanbenschoten
force-pushed
the
nvanbenschoten/slicesPkg
branch
from
2ce5985
to
e8bc9e3
Compare
The "cmp" package was added to the stdlib in Go 1.21. Epic: None Release note: None
The "slices" package was added to the stdlib in Go 1.21. Epic: None Release note: None
This commit replaces some of usages of sort.IsSorted with uses of slices.IsSortedFunc, where doing so makes sense. As of Go 1.21, the docs on sort.IsSorted say: > Note: in many situations, the newer slices.IsSortedFunc function is > more ergonomic and runs faster. Epic: None Release note: None
This commit replaces some of usages of sort.SliceIsSorted with uses of slices.IsSortedFunc, where doing so makes sense. As of Go 1.21, the docs on sort.SliceIsSorted say: > Note: in many situations, the newer slices.IsSortedFunc function is > more ergonomic and runs faster. Epic: None Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/slicesPkg
branch
from
e8bc9e3
to
4057d65
Compare
This commit replaces some of usages of sort.Slice with uses of slices.SortFunc, where doing so makes sense. As of Go 1.21, the docs on sort.Slice say: > Note: in many situations, the newer slices.SortFunc function is > more ergonomic and runs faster. This commit only makes these changes in production kv code for now, because replacing the other 200 or so uses of sort.Slice would be a lot of churn with less benefit. Epic: None Release note: None
This commit replaces some of usages of sort.Slice with uses of slices.Sort, where doing so makes sense. As of Go 1.21, the docs on sort.Slice say: > Note: in many situations, the newer slices.SortFunc function is > more ergonomic and runs faster. Epic: None Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/slicesPkg
branch
from
4057d65
to
50acd41
Compare
nvanbenschoten
commented
May 20, 2024
There was a problem hiding this comment.
TFTR!
bors r=kvoli
Reviewable status:
pkg/sql/sem/tree/datum.go line 4817 at r7 (raw file):
Previously, kvoli (Austen) wrote…
nit: consider rename to
sortFnor similar.
Done.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adopts the new "slices" package for sorting, where doing so makes sense. This package was added to the Go standard library in Go 1.21. It uses generics to improve the ergonomics and performance of sorting and searching related tasks.
@kvoli assigning you as a reviewer because allocator-related code seems to be the biggest user of sorting around.
See individual commits.