Skip to content

Commit

Permalink
strings: remove overengineered Compare implementation
Browse files Browse the repository at this point in the history
The function is here ONLY for symmetry with package bytes.
This function should be used ONLY if it makes code clearer.
It is not here for performance. Remove any performance benefit.

If performance becomes an issue, the compiler should be fixed to
recognize the three-way compare (for all comparable types)
rather than encourage people to micro-optimize by using this function.

Change-Id: I71f4130bce853f7aef724c6044d15def7987b457
Reviewed-on: https://go-review.googlesource.com/3012
Reviewed-by: Rob Pike <r@golang.org>
  • Loading branch information
rsc committed Jan 19, 2015
1 parent e832043 commit fd4dc91
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 21 deletions.
3 changes: 0 additions & 3 deletions src/runtime/asm_386.s
Expand Up @@ -1431,9 +1431,6 @@ TEXT runtime·cmpstring(SB),NOSPLIT,$0-20
MOVL AX, ret+16(FP)
RET

TEXT strings·Compare(SB),NOSPLIT,$0
JMP runtime·cmpstring(SB)

TEXT bytes·Compare(SB),NOSPLIT,$0-28
MOVL s1+0(FP), SI
MOVL s1+4(FP), BX
Expand Down
3 changes: 0 additions & 3 deletions src/runtime/asm_amd64.s
Expand Up @@ -1364,9 +1364,6 @@ TEXT runtime·cmpstring(SB),NOSPLIT,$0-40
MOVQ AX, ret+32(FP)
RET

TEXT strings·Compare(SB),NOSPLIT,$0
JMP runtime·cmpstring(SB)

TEXT bytes·Compare(SB),NOSPLIT,$0-56
MOVQ s1+0(FP), SI
MOVQ s1+8(FP), BX
Expand Down
3 changes: 0 additions & 3 deletions src/runtime/asm_amd64p32.s
Expand Up @@ -832,9 +832,6 @@ TEXT runtime·cmpstring(SB),NOSPLIT,$0-20
MOVL AX, ret+16(FP)
RET

TEXT strings·Compare(SB),NOSPLIT,$0
JMP runtime·cmpstring(SB)

TEXT bytes·Compare(SB),NOSPLIT,$0-28
MOVL s1+0(FP), SI
MOVL s1+4(FP), BX
Expand Down
5 changes: 0 additions & 5 deletions src/runtime/noasm.go
Expand Up @@ -10,11 +10,6 @@ package runtime

import _ "unsafe" // for go:linkname

//go:linkname strings_Compare strings.Compare
func strings_Compare(s1, s2 string) int {
return cmpstring(s1, s2)
}

func cmpstring(s1, s2 string) int {
l := len(s1)
if len(s2) < l {
Expand Down
28 changes: 28 additions & 0 deletions src/strings/compare.go
@@ -0,0 +1,28 @@
// Copyright 2015 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package strings

// 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
}
7 changes: 0 additions & 7 deletions src/strings/strings_decl.go
Expand Up @@ -6,10 +6,3 @@ package strings

// IndexByte returns the index of the first instance of c in s, or -1 if c is not present in s.
func IndexByte(s string, c byte) int // ../runtime/asm_$GOARCH.s

// 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.
//
// In most cases it is simpler to use the built-in comparison operators
// ==, <, >, and so on.
func Compare(a, b string) int // ../runtime/noasm.go or ../runtime/asm_*.s

0 comments on commit fd4dc91

Please sign in to comment.