Repository navigation
strings, x/exp/slices: strings.Compare is more useful than its docs say, but also slow #58142
Copy link
Copy link
Closed
Labels
DocumentationIssues describing a change to documentation.Issues describing a change to documentation.FrozenDueToAgeNeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Description
What version of Go are you using (go version)?
go version devel go1.21-4f467f1082 Wed Jan 25 21:16:32 2023 +0000 linux/amd64
What did you do?
I benchmarked two binary searches on an array of strings:
sort.Search(len(strs), func(i int) bool { return key <= strs[i] })
slices.BinarySearchFunc(strs, key, strings.Compare)Turns out the former can be significantly faster. Seeing how strings.Compare is implemented, I get why:
Lines 7 to 28 in d42c08a
| // Compare returns an integer comparing two strings lexicographically. | |
| // The result will be 0 if a == b, -1 if a < b, and +1 if a > b. | |
| // | |
| // Compare is included only for symmetry with package bytes. | |
| // It is usually clearer and always faster to use the built-in | |
| // string comparison operators ==, <, >, and so on. | |
| func Compare(a, b string) int { | |
| // NOTE(rsc): This function does NOT call the runtime cmpstring function, | |
| // because we do not want to provide any performance justification for | |
| // using strings.Compare. Basically no one should use strings.Compare. | |
| // As the comment above says, it is here only for symmetry with package bytes. | |
| // If performance is important, the compiler should be changed to recognize | |
| // the pattern so that all code doing three-way comparisons, not just code | |
| // using strings.Compare, can benefit. | |
| if a == b { | |
| return 0 | |
| } | |
| if a < b { | |
| return -1 | |
| } | |
| return +1 | |
| } |
Should the decision to not optimize strings.Compare be reconsidered? Even if not, should its comments be changed to not recommend against its use? Because it appears to me as the sane way to work with strings and x/exp/slices.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
DocumentationIssues describing a change to documentation.Issues describing a change to documentation.FrozenDueToAgeNeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Activity
mknyszek commentedon Jan 30, 2023
CC @rsc who wrote the original note against optimizing
strings.Compare, and @ianlancetaylor for x/exp/slices.zigo101 commentedon Jan 31, 2023
related: #50167
rhcarvalho commentedon Aug 11, 2023
With
slicesin the standard library as of Go 1.21, I noticed a few places in our code base where we use the new genericcmp.Comparefunction in places wherestrings.Comparewould also work.Since
strings.Compareandcmp.Compareare similarly implemented, I would expect the two to have similar performance characteristics, inline with the note within thestrings.Comparesource code.ncruces commentedon Sep 28, 2023
cmp.Comparewill have worse performance for strings.cmp.Compareis implemented in such a way that the comparisons necessary to place NaNs before all other numbers are optimized away for types that are not floats.strings.Comparetakes advantage of the fact that==is fast for strings of unequal length.zigo101 commentedon Apr 6, 2024
This is done for #61725
ianlancetaylor commentedon Apr 7, 2024
Thanks for pointing that out.