Skip to content
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.

Sign up for GitHub

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

Jump to bottom

performance issue about empty string check with str == "" in go1.14.6 #40330

Closed
songjiayang opened this issue Jul 21, 2020 · 4 comments
Closed

performance issue about empty string check with str == "" in go1.14.6 #40330

songjiayang opened this issue Jul 21, 2020 · 4 comments
Labels
FrozenDueToAge Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@songjiayang
Copy link

What version of Go are you using (go version)?

$ go version

go version go1.14.6 darwin/amd64

Does this issue reproduce with the latest release?

yes, it does

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE=""

GOARCH="amd64"

GOBIN=""

GOCACHE="/Users/songjiayang/Library/Caches/go-build"

GOENV="/Users/songjiayang/Library/Application Support/go/env"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GOINSECURE=""

GOOS="darwin"

GOPATH="/Users/songjiayang/.gvm/pkgsets/go1.14.6/global"

GOPROXY="https://goproxy.io,direct"

GOROOT="/Users/songjiayang/.gvm/gos/go1.14.6"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/Users/songjiayang/.gvm/gos/go1.14.6/pkg/tool/darwin_amd64"

GCCGO="gccgo"

AR="ar"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD="/Users/songjiayang/workspace/golang/benchmark/go.mod"

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/zb/p5mbxpss5j19k76c9kd3wy5jxfrpjg/T/go-build631736981=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I run benchmark to test empty string check, the code is :

package benchmark

import (
	"log"
	"testing"
)

func BenchmarkStringEmptyCheckWithDefault(b *testing.B) {
	var foo string
	for i := 0; i < b.N; i++ {
		if foo != "" {
			log.Panicf("empty string should be default")
		}
	}
}

func BenchmarkStringEmptyCheckWithLen(b *testing.B) {
	var foo string
	for i := 0; i < b.N; i++ {
		if len(foo) != 0 {
			log.Panicf("empty string length should be 0")
		}
	}
}

run benchmark with go1.14.6:

go test -bench=BenchmarkStringEmptyCheck

goos: darwin
goarch: amd64
pkg: benchmark
BenchmarkStringEmptyCheckWithDefault-8   	1000000000	         0.613 ns/op
BenchmarkStringEmptyCheckWithLen-8       	1000000000	         0.309 ns/op
PASS
ok  	benchmark	1.040s

run it with go1.10.8 :

goos: darwin
goarch: amd64
BenchmarkStringEmptyCheckWithDefault-8   	2000000000	         0.30 ns/op
BenchmarkStringEmptyCheckWithLen-8       	2000000000	         0.30 ns/op
PASS

What did you expect to see?

running benchmark with go1.14.6 should behavior like go1.10.8.

What did you see instead?

but in go1.14.6, str == "" is slower than len(str) ==0 to check empty string.
@seankhliao
Copy link
Member

This appears to be something between 1.14.5 and 1.14.6

» go1.14.5 test -bench .
goos: linux
goarch: amd64
pkg: go.seankhliao.com/testrepo-113
BenchmarkStringEmptyCheckWithDefault-4   	1000000000	         0.317 ns/op
BenchmarkStringEmptyCheckWithLen-4       	1000000000	         0.323 ns/op
PASS
ok  	go.seankhliao.com/testrepo-113	0.714s

» go1.14.6 test -bench .
goos: linux
goarch: amd64
pkg: go.seankhliao.com/testrepo-113
BenchmarkStringEmptyCheckWithDefault-4   	1000000000	         0.642 ns/op
BenchmarkStringEmptyCheckWithLen-4       	1000000000	         0.635 ns/op
PASS
ok  	go.seankhliao.com/testrepo-113	1.421s

@songjiayang
Copy link
Author

songjiayang commented Jul 21, 2020 •
edited

@seankhliao I test go 1.14.5 with OS darwin, str == "" is slower than len(str) == 0 as go1.14.6.

@martisch
Copy link
Contributor

martisch commented Jul 21, 2020 •
edited

Those benchmarks are not very meaningful. Both are optimized to an empty loop:

go tool objdump -s StringEmpty test.test 
TEXT _/Users/martisch/test.BenchmarkStringEmptyCheckWithDefault(SB) /Users/martisch/test/main_test.go
  main_test.go:10	0x110abc0		488b442408		MOVQ 0x8(SP), AX	
  main_test.go:10	0x110abc5		31c9			XORL CX, CX		
  main_test.go:10	0x110abc7		eb03			JMP 0x110abcc		
  main_test.go:10	0x110abc9		48ffc1			INCQ CX			
  main_test.go:10	0x110abcc		48398870010000		CMPQ CX, 0x170(AX)	
  main_test.go:10	0x110abd3		7ff4			JG 0x110abc9		
  main_test.go:10	0x110abd5		c3			RET			

TEXT _/Users/martisch/test.BenchmarkStringEmptyCheckWithLen(SB) /Users/martisch/test/main_test.go
  main_test.go:19	0x110abe0		488b442408		MOVQ 0x8(SP), AX	
  main_test.go:19	0x110abe5		31c9			XORL CX, CX		
  main_test.go:19	0x110abe7		eb03			JMP 0x110abec		
  main_test.go:19	0x110abe9		48ffc1			INCQ CX			
  main_test.go:19	0x110abec		48398870010000		CMPQ CX, 0x170(AX)	
  main_test.go:19	0x110abf3		7ff4			JG 0x110abe9		
  main_test.go:19	0x110abf5		c3			RET

The difference in performance might be explained by branch alignment/code position differences due to patches in go1.14. Both run at the same speed of 1 clock cycle per iteration on my machine (darwin/amd64 Intel Ice Lake) with go tip (11f92e9), go1.14.6 and go1.14.5.

For others: see if changing the order of the benchmark functions in the source file makes a difference.

I do not think there is anything todo or can be improved between the versions as they both generate the same code.

@martisch martisch added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. Performance labels Jul 21, 2020
@martisch
Copy link
Contributor

Closing given that the emitted instructions are the same and it is not consistently reproducible across machines. Feel free to reopen with a new example if there is a measurable change in a more complex benchmark or whole program runtime between sub versions of go1.14 that can be classified as performance regressions.

@golang golang locked and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@martisch @songjiayang @gopherbot @seankhliao