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.

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

database/sql: performance regression in ConcurrentDBExec #22722

Closed
TocarIP opened this issue Nov 14, 2017 · 4 comments
Closed

database/sql: performance regression in ConcurrentDBExec #22722

TocarIP opened this issue Nov 14, 2017 · 4 comments

Comments

@TocarIP
Copy link
Contributor

@TocarIP TocarIP commented Nov 14, 2017

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

go version devel +ef0e2af Mon Nov 6 15:55:31 2017 +0000 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/nfs/site/home/itocar/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/localdisk/itocar/gopath/"
GORACE=""
GOROOT="/localdisk/itocar/golang"
GOTMPDIR=""
GOTOOLDIR="/localdisk/itocar/golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build447443283=/tmp/go-build -gno-record-gcc-switches"

/proc/cpuinfo:
model name : Intel(R) Xeon(R) CPU E5-2609 v3 @ 1.90GHz

What did you do?
Compared 1.9 performance with master on benches from database/sql package and found a regression:
ConcurrentDBExec-6 18.9ms ± 1% 22.5ms ± 1% +18.88% (p=0.001 n=7+7)

Bisecting points to 2 commits:
6a223b8 which is responsible for 12%:
ConcurrentDBExec-6 20.4ms ± 1% 23.0ms ± 1% +12.38% (p=0.000 n=8+8)

Commit message dosen;t mention performance implications, so I'm considering this a regression.

e972095 has caused the rest of regression,
I' have no idea why it affect this bench, perhaps this just an unfortunate side-effect.

When running this test with -count=10 i see that first few runs are faster and runs after ~5 are stable in performance, but the regression is present for both first and last runs.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 15, 2017

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Nov 15, 2017
@rsc
Copy link
Contributor

@rsc rsc commented Nov 22, 2017

FWIW I'm not convinced this benchmark is measuring anything particularly useful.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Nov 22, 2017

I'm also not concerned with this regression, at least at this level. The fake driver is doing more work then before because it now implements the connection resetter. A non implementing driver should not see any difference.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 28, 2017

Okay, closing. @TocarIP, thanks for filing in any case. Nobody's historically done the job you're doing (checking for performance regressions), and our automated tooling for it is busted, so please keep letting us know if you find these things.

@bradfitz bradfitz closed this Nov 28, 2017
@golang golang locked and limited conversation to collaborators Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.