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

runtime: new heap limits negatively affect performance #22692

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

runtime: new heap limits negatively affect performance #22692

TocarIP opened this issue Nov 13, 2017 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Go1.12

Comments

@TocarIP
Copy link
Contributor

TocarIP commented Nov 13, 2017 •
edited

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?

While comparing 1.9 performance with trunk, I've noticed that a number of benchmarks regressed vs 1.9 after commit: 03eb948

strings/HTMLEscapeOld 463ns ± 0% 521ns ± 1% +12.59% (p=0.000 n=8+8)
bufio/ReaderEmpty 1.06µs ± 0% 1.63µs ± 1% +54.43% (p=0.000 n=7+8)
html/UnescapeSparse 3.46µs ± 0% 3.84µs ± 0% +10.97% (p=0.000 n=8+8)
html/template/CSSValueFilter 329ns ± 0% 370ns ± 1% +12.47% (p=0.000 n=8+8)

also with -cpu=1 there is

net/IPMaskString 617ns ± 1% 714ns ±10% +15.66% (p=0.000 n=7+8)

I'm not sure whether this is an expected result (commit message doesn't have any double digits regressions), so I'm opening this.
@TocarIP
Copy link
Contributor Author

TocarIP commented Nov 13, 2017

cc @aclements

@aclements
Copy link
Member

Interesting. This is not expected behavior. However, my guess is that there's nothing special about these particular benchmarks. That commit certainly will change GC scheduling, and it may simply be that they typically didn't get a GC scheduled during them before and now do.

@TocarIP, could you try running each of these benchmarks in isolation and for a longer duration before and after that commit? That should help narrow down whether they're just being affected by scheduling artifacts from neighboring benchmarks.

(Thanks for keeping an eye on all the benchmarks!)

@aclements aclements added this to the Go1.10 milestone Nov 13, 2017
@TocarIP
Copy link
Contributor Author

TocarIP commented Nov 13, 2017

I don't have data at hand, but it is reproducible by running them in an isolation on that commit (and previous). It could be scheduling artifact, but (at least on my machine) it is stable. I'll try it on other machine later

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 15, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 10, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 10, 2018
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
5 participants
@rsc @aclements @ianlancetaylor @gopherbot @TocarIP