Currently, the GC uses a moving average of recent scan work ratios to
estimate the total scan work required by this cycle. This is in turn
used to compute how much scan work should be done by mutators when
they allocate in order to perform all expected scan work by the time
the allocated heap reaches the heap goal.
However, our current scan work estimate can be arbitrarily wrong if
the heap topography changes significantly from one cycle to the
next. For example, in the go1 benchmarks, at the beginning of each
benchmark, the heap is dominated by a 256MB no-scan object, so the GC
learns that the scan density of the heap is very low. In benchmarks
that then rapidly allocate pointer-dense objects, by the time of the
next GC cycle, our estimate of the scan work can be too low by a large
factor. This in turn lets the mutator allocate faster than the GC can
collect, allowing it to get arbitrarily far ahead of the scan work
estimate, which leads to very long GC cycles with very little mutator
assist that can overshoot the heap goal by large margins. This is
particularly easy to demonstrate with BinaryTree17:
$ GODEBUG=gctrace=1 ./go1.test -test.bench BinaryTree17
gc #1 @0.017s 2%: 0+0+0+0+0 ms clock, 0+0+0+0/0/0+0 ms cpu, 4->262->262 MB, 4 MB goal, 1 P
gc #2 @0.026s 3%: 0+0+0+0+0 ms clock, 0+0+0+0/0/0+0 ms cpu, 262->262->262 MB, 524 MB goal, 1 P
testing: warning: no tests to run
PASS
BenchmarkBinaryTree17 gc #3 @1.906s 0%: 0+0+0+0+7 ms clock, 0+0+0+0/0/0+7 ms cpu, 325->325->287 MB, 325 MB goal, 1 P (forced)
gc #4 @12.203s 20%: 0+0+0+10067+10 ms clock, 0+0+0+0/2523/852+10 ms cpu, 430->2092->1950 MB, 574 MB goal, 1 P
1 9150447353 ns/op
Change this estimate to instead use the *current* scannable heap
size. This has the advantage of being based solely on the current
state of the heap, not on past densities or reachable heap sizes, so
it isn't susceptible to falling behind during these sorts of phase
changes. This is strictly an over-estimate, but it's better to
over-estimate and get more assist than necessary than it is to
under-estimate and potentially spiral out of control. Experiments with
scaling this estimate back showed no obvious benefit for mutator
utilization, heap size, or assist time.
This new estimate has little effect for most benchmarks, including
most go1 benchmarks, x/benchmarks, and the 6g benchmark. It has a huge
effect for benchmarks that triggered the bad pacer behavior:
name old mean new mean delta
BinaryTree17 10.0s × (1.00,1.00) 3.5s × (0.98,1.01) -64.93% (p=0.000)
Fannkuch11 2.74s × (1.00,1.01) 2.65s × (1.00,1.00) -3.52% (p=0.000)
FmtFprintfEmpty 56.4ns × (0.99,1.00) 57.8ns × (1.00,1.01) +2.43% (p=0.000)
FmtFprintfString 187ns × (0.99,1.00) 185ns × (0.99,1.01) -1.19% (p=0.010)
FmtFprintfInt 184ns × (1.00,1.00) 183ns × (1.00,1.00) (no variance)
FmtFprintfIntInt 321ns × (1.00,1.00) 315ns × (1.00,1.00) -1.80% (p=0.000)
FmtFprintfPrefixedInt 266ns × (1.00,1.00) 263ns × (1.00,1.00) -1.22% (p=0.000)
FmtFprintfFloat 353ns × (1.00,1.00) 353ns × (1.00,1.00) -0.13% (p=0.035)
FmtManyArgs 1.21µs × (1.00,1.00) 1.19µs × (1.00,1.00) -1.33% (p=0.000)
GobDecode 9.69ms × (1.00,1.00) 9.59ms × (1.00,1.00) -1.07% (p=0.000)
GobEncode 7.89ms × (0.99,1.01) 7.74ms × (1.00,1.00) -1.92% (p=0.000)
Gzip 391ms × (1.00,1.00) 392ms × (1.00,1.00) ~ (p=0.522)
Gunzip 97.1ms × (1.00,1.00) 97.0ms × (1.00,1.00) -0.10% (p=0.000)
HTTPClientServer 55.7µs × (0.99,1.01) 56.7µs × (0.99,1.01) +1.81% (p=0.001)
JSONEncode 19.1ms × (1.00,1.00) 19.0ms × (1.00,1.00) -0.85% (p=0.000)
JSONDecode 66.8ms × (1.00,1.00) 66.9ms × (1.00,1.00) ~ (p=0.288)
Mandelbrot200 4.13ms × (1.00,1.00) 4.12ms × (1.00,1.00) -0.08% (p=0.000)
GoParse 3.97ms × (1.00,1.01) 4.01ms × (1.00,1.00) +0.99% (p=0.000)
RegexpMatchEasy0_32 114ns × (1.00,1.00) 115ns × (0.99,1.00) ~ (p=0.070)
RegexpMatchEasy0_1K 376ns × (1.00,1.00) 376ns × (1.00,1.00) ~ (p=0.900)
RegexpMatchEasy1_32 94.9ns × (1.00,1.00) 96.3ns × (1.00,1.01) +1.53% (p=0.001)
RegexpMatchEasy1_1K 568ns × (1.00,1.00) 567ns × (1.00,1.00) -0.22% (p=0.001)
RegexpMatchMedium_32 159ns × (1.00,1.00) 159ns × (1.00,1.00) ~ (p=0.178)
RegexpMatchMedium_1K 46.4µs × (1.00,1.00) 46.6µs × (1.00,1.00) +0.29% (p=0.000)
RegexpMatchHard_32 2.37µs × (1.00,1.00) 2.37µs × (1.00,1.00) ~ (p=0.722)
RegexpMatchHard_1K 71.1µs × (1.00,1.00) 71.2µs × (1.00,1.00) ~ (p=0.229)
Revcomp 565ms × (1.00,1.00) 562ms × (1.00,1.00) -0.52% (p=0.000)
Template 81.0ms × (1.00,1.00) 80.2ms × (1.00,1.00) -0.97% (p=0.000)
TimeParse 380ns × (1.00,1.00) 380ns × (1.00,1.00) ~ (p=0.148)
TimeFormat 405ns × (0.99,1.00) 385ns × (0.99,1.00) -5.00% (p=0.000)
Change-Id: I11274158bf3affaf62662e02de7af12d5fb789e4
Reviewed-on: https://go-review.googlesource.com/9696
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
This tracks the number of scannable bytes in the allocated heap. That
is, bytes that the garbage collector must scan before reaching the
last pointer field in each object.
This will be used to compute a more robust estimate of the GC scan
work.
Change-Id: I1eecd45ef9cdd65b69d2afb5db5da885c80086bb
Reviewed-on: https://go-review.googlesource.com/9695
Reviewed-by: Russ Cox <rsc@golang.org>
The garbage collector predicts how much "scan work" must be done in a
cycle to determine how much work should be done by mutators when they
allocate. Most code doesn't care what units the scan work is in: it
simply knows that a certain amount of scan work has to be done in the
cycle. Currently, the GC uses the number of pointer slots scanned as
the scan work on the theory that this is the bulk of the time spent in
the garbage collector and hence reflects real CPU resource usage.
However, this metric is difficult to estimate at the beginning of a
cycle.
Switch to counting the total number of bytes scanned, including both
pointer and scalar slots. This is still less than the total marked
heap since it omits no-scan objects and no-scan tails of objects. This
metric may not reflect absolute performance as well as the count of
scanned pointer slots (though it still takes time to scan scalar
fields), but it will be much easier to estimate robustly, which is
more important.
Change-Id: Ie3a5eeeb0384a1ca566f61b2f11e9ff3a75ca121
Reviewed-on: https://go-review.googlesource.com/9694
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, we only flush the per-P gcWork caches in gcMark, at the
beginning of mark termination. This is necessary to ensure that no
work is held up in these caches.
However, this flush happens after we update the GC controller state,
which depends on statistics about marked heap size and scan work that
are only updated by this flush. Hence, the controller is missing the
bulk of heap marking and scan work. This bug was introduced in commit
1b4025f, which introduced the per-P gcWork caches.
Fix this by flushing these caches before we update the GC controller
state. We continue to flush them at the beginning of mark termination
as well to be robust in case any write barriers happened between the
previous flush and entering mark termination, but this should be a
no-op.
Change-Id: I8f0f91024df967ebf0c616d1c4f0c339c304ebaa
Reviewed-on: https://go-review.googlesource.com/9646
Reviewed-by: Russ Cox <rsc@golang.org>
Ramp up the delay on subsequent attempts. Fast builders have the same delay.
Not a perfect fix, but should make it better. And this easy.
Fixes#9903 maybe
Fixes#10680 maybe
Change-Id: I967380c2cb8196e6da9a71116961229d37b36335
Reviewed-on: https://go-review.googlesource.com/9795
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Android has (had?) its own local DNS resolver daemon, also my fault:
007e987fee
And you access that via libc, not DNS.
Fixes#10714
Change-Id: Iaff752872ce19bb5c7771ab048fd50e3f72cb73c
Reviewed-on: https://go-review.googlesource.com/9793
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Improving the usability further.
Before:
$ go doc bytes.Read
doc: symbol Read not present in package bytes installed in "bytes"
$
After:
$ go doc bytes.Read
func (b *Buffer) Read(p []byte) (n int, err error)
Read reads the next len(p) bytes from the buffer or until the buffer is drained.
The return value n is the number of bytes read. If the buffer has no data to
return, err is io.EOF (unless len(p) is zero); otherwise it is nil.
func (r *Reader) Read(b []byte) (n int, err error)
$
Change-Id: I646511fada138bd09e9b39820da01a5ccef4a90f
Reviewed-on: https://go-review.googlesource.com/9656
Reviewed-by: Russ Cox <rsc@golang.org>
GOROOT is not dependably set.
When I first wrote this test, I thought it was a waste of time
because the function can't fail if the other environment functions
work, but I didn't want to add functionality without testing it.
Of course, the test broke, and I learned something: GOROOT is not
set on iOS or, to put it more broadly, the world continues to
surprise me with its complexity and horror, such as a version of
cat with syntax coloring.
In that vein, I built this test around smallpox.
Change-Id: Ifa6c218a927399d05c47954fdcaea1015e558fb6
Reviewed-on: https://go-review.googlesource.com/9791
Reviewed-by: Russ Cox <rsc@golang.org>
During development some tracing routines were added that are not
needed in the release. These included GCstarttimes, GCendtimes, and
GCprinttimes.
Fixes#10462
Change-Id: I0788e6409d61038571a5ae0cbbab793102df0a65
Reviewed-on: https://go-review.googlesource.com/9689
Reviewed-by: Austin Clements <austin@google.com>
Fixes#10221.
Change-Id: Ib23805494d8af1946360bfea767f9727e2504dc5
Reviewed-on: https://go-review.googlesource.com/7941
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Unfortunately Oracle Solaris does not have TCP_KEEPIDLE and
TCP_KEEPINTVL. TCP_KEEPIDLE is equivalent to TCP_KEEPALIVE_THRESHOLD,
but TCP_KEEPINTVL does not have a direct equivalent, so we don't set
TCP_KEEPINTVL any more.
Old Darwin versions also lack TCP_KEEPINTVL, but the code tries to set
it anyway so that it works on newer versions. We can't do that because
Oracle might assign the number illumos uses for TCP_KEEPINTVL to a
constant with a different meaning.
Unfortunately there's nothing we can do if we want to support both
illumos and Oracle Solaris with the same GOOS.
Updates #9614.
Change-Id: Id39eb5147f7afa8e951f886c0bf529d00f0e1bd4
Reviewed-on: https://go-review.googlesource.com/7690
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Before CL 8214 (use .plt instead of .got on Solaris) Solaris used a
dynamic linking scheme that didn't permit lazy binding. To speed program
startup, Go binaries only used it for a small number of symbols required
by the runtime. Other symbols were resolved on demand on first use, and
were cached for subsequent use. This required some moderately complex
code in the syscall package.
CL 8214 changed the way dynamic linking is implemented, and now lazy
binding is supported. As now all symbols are resolved lazily by the
dynamic loader, there is no need for the complex code in the syscall
package that did the same. This CL makes Go programs link directly
with the necessary shared libraries and deletes the lazy-loading code
implemented in Go.
Change-Id: Ifd7275db72de61b70647242e7056dd303b1aee9e
Reviewed-on: https://go-review.googlesource.com/9184
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Solaris, like Windows, NetBSD and OpenBSD, uses macros for stdin, stdout,
and stderr. Cgo can't access them without getters/setters written in
C. Because of this we disable affected tests like for the other platforms.
Updates #10715.
Change-Id: I3d33a5554b5ba209273dbdff992925a38a281b42
Reviewed-on: https://go-review.googlesource.com/8264
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
ELF normally requires this and Solaris runtime loader will crash if we
don't do it.
Fixes Solaris build.
Change-Id: I0482eed890aff2d346136ae7f9caf8f094f502ed
Reviewed-on: https://go-review.googlesource.com/8216
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The linker always uses .plt for externals, so libcFunc is now an actual
external symbol instead of a pointer to one.
Fixes most of the breakage introduced in previous CL.
Change-Id: I64b8c96f93127f2d13b5289b024677fd3ea7dbea
Reviewed-on: https://go-review.googlesource.com/8215
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Solaris requires all external procedures to be accessed through the
PLT. If 6l won't do it, /bin/ld will, so all the code written with .GOT
in mind won't work with the external linker.
This CL makes external linking work, opening the path to cgo support
on Solaris.
This CL breaks the Solaris build, this is fixed in subsequent CLs in
this series.
Change-Id: If370a79f49fdbe66d28b89fa463b4f3e91685f69
Reviewed-on: https://go-review.googlesource.com/8214
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
This change simplifies unnecessarily redundant error messages in tests.
There's no need to worry any more because package APIs now return
consistent, self-descriptive error values.
Alos renames ambiguous test functions and makes use of test tables.
Change-Id: I7b61027607c4ae2a3cf605d08d58cf449fa27eb2
Reviewed-on: https://go-review.googlesource.com/9662
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
This change makes TestDualStack{TCP,UDP}Listener work more properly by
attempting to book an available service port before testing.
Also simplifies error messages in tests.
Fixes#5001.
Change-Id: If13b0d0039878c9bd32061a0440664e4fa7abaf7
Reviewed-on: https://go-review.googlesource.com/9661
Reviewed-by: Ian Lance Taylor <iant@golang.org>
isn't (0, 0).
Also fix a s/b.Min.X/b.Max.X/ typo in bounds checking.
Fixes#10676
Change-Id: Ie5ff7ec20ca30367a8e65d32061959a2d8e089e9
Reviewed-on: https://go-review.googlesource.com/9712
Reviewed-by: Rob Pike <r@golang.org>
An ELF linker handles a PC-relative reference to an STT_FUNC defined in a
shared library by building a PLT entry and referring to that, so do the
same in 6l.
Fixes#10690
Change-Id: I061a96fd4400d957e301d0ac86760ce256910e1d
Reviewed-on: https://go-review.googlesource.com/9711
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Catch some malformed pipelines at parsing time.
The current code accepts pipelines such as:
{{12|.}}
{{"hello"|print|false}}
{{.|"blah blah"}}
Such pipelines generate panic in html/template at execution time.
Add an extra check to verify all the commands of the pipeline are executable
(except for the first one).
Fixes#10610
Change-Id: Id72236ba8f76a59fa284fe3d4c2cb073e50b51f1
Reviewed-on: https://go-review.googlesource.com/9626
Reviewed-by: Rob Pike <r@golang.org>
This makes the intermediate object file a little bigger but it doesn't waste
any space in the final shared library.
Fixes#10691
Change-Id: Ic51a571d60291f1ac2dad1b50dba4679643168ae
Reviewed-on: https://go-review.googlesource.com/9710
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This changes the action graph when shared libraries are involved to always have
an action for the shared library (which does nothing when the shared library
is up to date).
Change-Id: Ibbc70fd01cbb3f4e8c0ef96e62a151002d446144
Reviewed-on: https://go-review.googlesource.com/8934
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#10660
Fix the clang only builder by passing -extld down to the linker when needed.
The build passed on most hosts because gcc is almost always present. The bug
was verified by symlinking bin/false in place of gcc in my $PATH and running
the build.
Also, resolve a TODO and move the support logic into its own function.
Tested manually
env CC=clang-3.5 ./all.bash # linux/amd64
env CC=gcc-4.8 ./all.bash # linux/amd64
./all.bash # linux/amd64
./all.bash # darwin/amd64
Change-Id: I4e27a1119356e295500a0d19ad7a4ec14207bf10
Reviewed-on: https://go-review.googlesource.com/9526
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I just submitted the real fix for #10641.
This reverts commit 3120adc212.
Change-Id: I55051515f697e27ca887ed21c2ac985f0b9b062b
Reviewed-on: https://go-review.googlesource.com/9720
Reviewed-by: Joel Sing <jsing@google.com>
No semantic change.
Fixes#8708.
Change-Id: Ieda04a86a19bb69bfc2519d381a2f025e7cb8279
Reviewed-on: https://go-review.googlesource.com/9740
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I forgot there is already a ptrSize constant.
Rename field to avoid some confusion.
Change-Id: I098fdcc8afc947d6c02c41c6e6de24624cc1c8ff
Reviewed-on: https://go-review.googlesource.com/9700
Reviewed-by: Austin Clements <austin@google.com>
The old one was inferior.
Fixes#10695.
Change-Id: Ia7fb88c9ceb1b10197b77a54f729865385288d98
Reviewed-on: https://go-review.googlesource.com/9709
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
When a parse error occurred, the lexing goroutine would lay idle.
It's not likely a problem but if the program is for some reason
accepting badly formed data repeatedly, it's wasteful.
The solution is easy: Just drain the input on error. We know this
will succeed because the input is always a string and is therefore
guaranteed finite.
With debugging prints in the package tests I've shown this is effective,
shutting down 79 goroutines that would otherwise linger, out of 123 total.
Fixes#10574.
Change-Id: I8aa536e327b219189a7e7f604a116fa562ae1c39
Reviewed-on: https://go-review.googlesource.com/9658
Reviewed-by: Russ Cox <rsc@golang.org>
Freezetheworld still has stuff to do when gomaxprocs=1.
In particular, signals can come in on other Ms (like the GC M, say)
and the single user M is still running.
Fixes#10546
Change-Id: I2f07f17d1c81e93cf905df2cb087112d436ca7e7
Reviewed-on: https://go-review.googlesource.com/9551
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
