Also replaces verbs for error message from %s to %v. In general, low
level IO APIs return an error value containing non-string types and
there's no guarantee that all the types implement fmt.Stringer
interface.
Change-Id: I8a6e2a80d5c721c772a83b9556bac16556eaa771
Reviewed-on: https://go-review.googlesource.com/73931
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In CL 50510, the Content-Type header started to be set in Redirect when
request method is GET. (Prior to that, it wasn't set at all, which is
what said CL was fixing.) However, according to HTTP specification,
the expected response for a HEAD request is identical to that of a
GET request, but without the response body.
This CL updates the behavior to set the Content-Type header for HEAD
method in addition to GET.
This actually allows a simpler implementation than before. This change
largely reverts CL 50510, and applies the simpler implementation.
Add a test for Content-Type header and body for GET, HEAD requests.
Updates CL 50510.
Change-Id: If33ea3f4bbc5246bb5dc751458004828cfe681b9
Reviewed-on: https://go-review.googlesource.com/65190
Run-TryBot: Dmitri Shuralyov <shurcool@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
It is easy to miss the documentation information that no arguments
in the Notify function means that the Notify will catch all possible signals.
So the example was added with explicit comment above the Notify usage.
Fixes#22257
Change-Id: Ia6a16dd4a419f7c77d89020ca5db85979b5b474e
Reviewed-on: https://go-review.googlesource.com/74730
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The counter part, writeInt in cmd/internal/obj, writes int64s.
So the reader side should also read int64s. This may cause a
larger range of values being accepted, some of which should
not be that large. This is probably ok: for example, for
size/index/length, the very large value (due to corruption)
may be well past the end and causes other errors. And we did
not do much bound check anyway.
One exmaple where this matters is ARM32's object file. For one
type of relocation it encodes the instruction into Reloc.Add
field (which itself may be problematic and worth fix) and the
instruction encoding overflows int32, causing ARM32 object
file being rejected by goobj (and so objdump and nm) before.
Unskip ARM32 object file tests in goobj, nm, and objdump.
Updates #19811.
Change-Id: Ia46c2b68df5f1c5204d6509ceab6416ad6372315
Reviewed-on: https://go-review.googlesource.com/69010
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
An initial draft of the Newton code for Float.Sqrt was structured like
this:
for condition
// do Newton iteration..
prec *= 2
since prec, at the end of the loop, was double the precision used in
the last Newton iteration, the termination condition was set to
2*limit. The code was later rewritten in the form
for condition
prec *= 2
// do Newton iteration..
but condition was not updated, and it's still 2*limit, which is about
double what we actually need, and is triggering the execution of an
additional, and unnecessary, Newton iteration.
This change adjusts the Newton termination condition to the (correct)
value of z.prec, plus 32 guard bits as a safety margin.
name old time/op new time/op delta
FloatSqrt/64-4 798ns ± 3% 802ns ± 3% ~ (p=0.458 n=8+8)
FloatSqrt/128-4 1.65µs ± 1% 1.65µs ± 1% ~ (p=0.290 n=8+8)
FloatSqrt/256-4 3.10µs ± 1% 2.10µs ± 0% -32.32% (p=0.000 n=8+7)
FloatSqrt/1000-4 8.83µs ± 1% 4.91µs ± 2% -44.39% (p=0.000 n=8+8)
FloatSqrt/10000-4 107µs ± 1% 40µs ± 1% -62.68% (p=0.000 n=8+8)
FloatSqrt/100000-4 2.91ms ± 1% 0.96ms ± 1% -67.13% (p=0.000 n=8+8)
FloatSqrt/1000000-4 240ms ± 1% 80ms ± 1% -66.66% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
FloatSqrt/64-4 416B ± 0% 416B ± 0% ~ (all equal)
FloatSqrt/128-4 720B ± 0% 720B ± 0% ~ (all equal)
FloatSqrt/256-4 1.34kB ± 0% 0.82kB ± 0% -39.29% (p=0.000 n=8+8)
FloatSqrt/1000-4 5.09kB ± 0% 2.50kB ± 0% -50.94% (p=0.000 n=8+8)
FloatSqrt/10000-4 45.9kB ± 0% 23.5kB ± 0% -48.81% (p=0.000 n=8+8)
FloatSqrt/100000-4 533kB ± 0% 251kB ± 0% -52.90% (p=0.000 n=8+8)
FloatSqrt/1000000-4 9.21MB ± 0% 4.61MB ± 0% -49.98% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
FloatSqrt/64-4 9.00 ± 0% 9.00 ± 0% ~ (all equal)
FloatSqrt/128-4 13.0 ± 0% 13.0 ± 0% ~ (all equal)
FloatSqrt/256-4 15.0 ± 0% 12.0 ± 0% -20.00% (p=0.000 n=8+8)
FloatSqrt/1000-4 24.0 ± 0% 19.0 ± 0% -20.83% (p=0.000 n=8+8)
FloatSqrt/10000-4 40.0 ± 0% 35.0 ± 0% -12.50% (p=0.000 n=8+8)
FloatSqrt/100000-4 66.0 ± 0% 55.0 ± 0% -16.67% (p=0.000 n=8+8)
FloatSqrt/1000000-4 143 ± 0% 122 ± 0% -14.69% (p=0.000 n=8+8)
Change-Id: I4868adb7f8960f2ca20e7792734c2e6211669fc0
Reviewed-on: https://go-review.googlesource.com/75010
Reviewed-by: Robert Griesemer <gri@golang.org>
Recurse into structs/arrays of one element when
assigning names.
Test incorporated into existing end-to-end debugger test,
hand-verified that it fails without this CL.
Fixes#19868
Revives CL 40010
Old-Change-Id: I0266e58af975fb64cfa17922be383b70f0a7ea96
Change-Id: I122ac2375931477769ec8d763607c1ec42d78a7f
Reviewed-on: https://go-review.googlesource.com/71731
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Dsymutil, an utility used on macOS when externally linking executables,
does not support base address selector entries in debug_ranges.
To work around this deficiency this commit removes base address
selectors from debug_ranges and emits instead a list composed only of
compile unit relative addresses.
A new type of relocation is introduced, R_ADDRCUOFF, similar to
R_ADDROFF, that relocates an address to its offset from the low_pc of
the symbol's compile unit.
Fixes#21945
Change-Id: Ie991f9bc1afda2b49ac5d734eb41c37d3a37e554
Reviewed-on: https://go-review.googlesource.com/72371
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
After this CL, "go vet" can be guaranteed to have complete type information
about the packages being checked, even if cgo or swig is in use,
which will in turn make it reasonable for vet checks to insist on type
information. It also fixes vet's understanding of unusual import paths
like relative paths and vendored packages.
For now "go tool vet" will continue to cope without type information,
but the eventual plan is for "go tool vet" to query the go command for
what it needs, and also to be able to query alternate build systems
like bazel. But that's future work.
Fixes#4889.
Fixes#12556 (if not already fixed).
Fixes#15182.
Fixes#16086.
Fixes#17571.
Change-Id: I932626ee7da649b302cd269b82eb6fe5d7b9f0f2
Reviewed-on: https://go-review.googlesource.com/74750
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL adds support for accepting package config from
the go command. Paired with CL 74356 this lets us make
sure vet has complete information about package sources.
This fixes many issues (see CL 74356 for the list), including
mishandling of cgo and vendoring.
Change-Id: Ia4a1dce6f9b1b0a8ef5fdf9005a20a8b294969f1
Reviewed-on: https://go-review.googlesource.com/74355
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Also, support spaces in go binaries locations, and document
GOROOT_BOOTSTRAP at the top.
Change-Id: I643d22df57aad9a2200cc256edd20e8c811bc70d
Reviewed-on: https://go-review.googlesource.com/74951
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The check of uintptr(newcap) > maxSliceCap(et.size) in addition
to capmem > _MaxMem is needed to prevent a reproducible overflow
on 32bit architectures.
On 64bit platforms this problem is less likely to occur as allocation
of a sufficiently large array or slice to be append is likely to
already exhaust available memory before the call to append can be made.
Example program that without the fix in this CL does segfault on 386:
type T [1<<27 + 1]int64
var d T
var s []T
func main() {
s = append(s, d, d, d, d)
print(len(s), "\n")
}
Fixes#21586
Change-Id: Ib4185435826ef43df71ba0f789e19f5bf9a347e6
Reviewed-on: https://go-review.googlesource.com/55133
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
testing.Skip{,f} will exit the test via runtime.Goexit. Thus, the
successive return is never reached and can be removed.
Change-Id: I1e399f3d5db753ece1ffba648850427e1b4be300
Reviewed-on: https://go-review.googlesource.com/74990
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
commentText is only called if g != nil in ParseGo, so the check inside
commentText is redundant and can be deleted.
Change-Id: I130c18b738527c96bc59950b354a50b9e23f92e9
Reviewed-on: https://go-review.googlesource.com/74871
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is basically a mini-bootstrap, to reach a fixed point.
Change-Id: I88abad3d3ac961c3d11a48cb64d625d458684ef7
Reviewed-on: https://go-review.googlesource.com/74792
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Otherwise the new numbered directories like b028/ appear in the objects,
and they can change from run to run.
Fixes#22514.
Change-Id: I8d0cf65f3622e48b2547d5757febe0ee1301e2ed
Reviewed-on: https://go-review.googlesource.com/74791
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This makes 'go install cmd/compile' in one directory produce
a different binary from running it in another directory,
which is problematic for reproducible builds.
Change-Id: If26685d2e45d2695413b472142b49694716575fa
Reviewed-on: https://go-review.googlesource.com/74790
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
When a SyntaxError occurs, report the current offset within the stream.
The code already accounted for the offset within the current buffer
being scanned. By including how much data was already scanned, the
current offset can be computed.
Fixes#22478
Change-Id: I91ecd4cad0b85a5c1556bc597f3ee914e769af01
Reviewed-on: https://go-review.googlesource.com/74251
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add a DisallowUnknownFields flag to Decoder.
DisallowUnknownFields causes the Decoder to return an error when
the the decoding destination is a struct and the input contains
object keys which do not match any non-ignored, public field the
destination, including keys whose value is set to null.
Note: this fix has already been worked on in 27231, which seems
to be abandoned. This version is a slightly simpler implementation
and is up to date with the master branch.
Fixes#15314
Change-Id: I987a5857c52018df334f4d1a2360649c44a7175d
Reviewed-on: https://go-review.googlesource.com/74830
Reviewed-by: Joe Tsai <joetsai@google.com>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since Go1.8, different types of GC mark workers were annotated and the
annotation strings were recorded during StartTrace. This change fixes
two issues around the use of traceString from StartTrace here.
1) "failed to parse trace: no consistent ordering of events possible"
This issue is a result of a missing 'batch' event entry. For efficient
tracing, tracer maintains system allocated buffers and once a buffer
is full, it is Flushed out for writing. Moreover, tracing assumes all
the records in the same buffer (batch) are already ordered and implements
more optimization in encoding and defers the completing order
reconstruction till the trace parsing time. Thus, when a Flush happens
and a new buffer is used, the new buffer should contain an event to
indicate the start of a new batch. Before this CL, the batch entry was
written only by traceEvent only when the buffer position is 0 and
wasn't written when flush occurs during traceString.
This CL fixes it by moving the batch entry write to the traceFlush.
2) crash during tracing due to invalid memory access, or during parsing
due to duplicate string entries
This issue is a result of memory allocation during traceString calls.
Execution tracer traces some memory allocation activities. Before this
CL, traceString took the buffer address (*traceBuf) and mutated the buffer.
If memory tracing occurs in the meantime from the same P, the allocation
tracing (traceEvent) will take the same buffer address through the pointer
to the buffer address (**traceBuf), and mutate the buffer.
As a result, one of the followings can happen:
- the allocation record is overwritten by the following trace string
record (data loss)
- if buffer flush occurs during the allocation tracing, traceString
will attempt to write the string record to the old buffer and
eventually causes invalid memory access crash.
- or flush on the same buffer can occur twice (once from the memory
allocation, and once from the string record write), and in this case
the trace can contain the same data twice and the parse will complain
about duplicate string record entries.
This CL fixes the second issue by making the traceString take
**traceBuf (*traceBufPtr).
Change-Id: I24f629758625b38e1916fbfc7d7be6ea210586af
Reviewed-on: https://go-review.googlesource.com/50873
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently, both the background mark worker and the goal GC CPU are
both fixed at 25%. The trigger controller's goal is to achieve the
goal CPU usage, and with the previous commit it can actually achieve
this. But this means there are *no* assists, which sounds ideal but
actually causes problems for the trigger controller. Since the
controller can't lower CPU usage below the background mark worker CPU,
it saturates at the CPU goal and no longer gets feedback, which
translates into higher variability in heap growth.
This commit fixes this by allowing assists 5% CPU beyond the 25% fixed
background mark. This avoids saturating the trigger controller, since
it can now get feedback from both sides of the CPU goal. This leads to
low variability in both CPU usage and heap growth, at the cost of
reintroducing a low rate of mark assists.
We also experimented with 20% background plus 5% assist, but 25%+5%
clearly performed better in benchmarks.
Updates #14951.
Updates #14812.
Updates #18534.
Combined with the previous CL, this significantly improves tail
mutator utilization in the x/bechmarks garbage benchmark. On a sample
trace, it increased the 99.9%ile mutator utilization at 10ms from 26%
to 59%, and at 5ms from 17% to 52%. It reduced the 99.9%ile zero
utilization window from 2ms to 700µs. It also helps the mean mutator
utilization: it increased the 10s mutator utilization from 83% to 94%.
The minimum mutator utilization is also somewhat improved, though
there is still some unknown artifact that causes a miniscule fraction
of mutator assists to take 5--10ms (in fact, there was exactly one
10ms mutator assist in my sample trace).
This has no significant effect on the throughput of the
github.com/dr2chase/bent benchmarks-50.
This has little effect on the go1 benchmarks (and the slight overall
improvement makes up for the slight overall slowdown from the previous
commit):
name old time/op new time/op delta
BinaryTree17-12 2.40s ± 0% 2.41s ± 1% +0.26% (p=0.010 n=18+19)
Fannkuch11-12 2.95s ± 0% 2.93s ± 0% -0.62% (p=0.000 n=18+15)
FmtFprintfEmpty-12 42.2ns ± 0% 42.3ns ± 1% +0.37% (p=0.001 n=15+14)
FmtFprintfString-12 67.9ns ± 2% 67.2ns ± 3% -1.03% (p=0.002 n=20+18)
FmtFprintfInt-12 75.6ns ± 3% 76.8ns ± 2% +1.59% (p=0.000 n=19+17)
FmtFprintfIntInt-12 123ns ± 1% 124ns ± 1% +0.77% (p=0.000 n=17+14)
FmtFprintfPrefixedInt-12 148ns ± 1% 150ns ± 1% +1.28% (p=0.000 n=20+20)
FmtFprintfFloat-12 212ns ± 0% 211ns ± 1% -0.67% (p=0.000 n=16+17)
FmtManyArgs-12 499ns ± 1% 500ns ± 0% +0.23% (p=0.004 n=19+16)
GobDecode-12 6.49ms ± 1% 6.51ms ± 1% +0.32% (p=0.008 n=19+19)
GobEncode-12 5.47ms ± 0% 5.43ms ± 1% -0.68% (p=0.000 n=19+20)
Gzip-12 220ms ± 1% 216ms ± 1% -1.66% (p=0.000 n=20+19)
Gunzip-12 38.8ms ± 0% 38.5ms ± 0% -0.80% (p=0.000 n=19+20)
HTTPClientServer-12 78.5µs ± 1% 78.1µs ± 1% -0.53% (p=0.008 n=20+19)
JSONEncode-12 12.2ms ± 0% 11.9ms ± 0% -2.38% (p=0.000 n=17+19)
JSONDecode-12 52.3ms ± 0% 53.3ms ± 0% +1.84% (p=0.000 n=19+20)
Mandelbrot200-12 3.69ms ± 0% 3.69ms ± 0% -0.19% (p=0.000 n=19+19)
GoParse-12 3.17ms ± 1% 3.19ms ± 1% +0.61% (p=0.000 n=20+20)
RegexpMatchEasy0_32-12 73.7ns ± 0% 73.2ns ± 1% -0.66% (p=0.000 n=17+20)
RegexpMatchEasy0_1K-12 238ns ± 0% 239ns ± 0% +0.32% (p=0.000 n=17+16)
RegexpMatchEasy1_32-12 69.1ns ± 1% 69.2ns ± 1% ~ (p=0.669 n=19+13)
RegexpMatchEasy1_1K-12 365ns ± 1% 367ns ± 1% +0.49% (p=0.000 n=19+19)
RegexpMatchMedium_32-12 104ns ± 1% 105ns ± 1% +1.33% (p=0.000 n=16+20)
RegexpMatchMedium_1K-12 33.6µs ± 3% 34.1µs ± 4% +1.67% (p=0.001 n=20+20)
RegexpMatchHard_32-12 1.67µs ± 1% 1.62µs ± 1% -2.78% (p=0.000 n=18+17)
RegexpMatchHard_1K-12 50.3µs ± 2% 48.7µs ± 1% -3.09% (p=0.000 n=19+18)
Revcomp-12 384ms ± 0% 386ms ± 0% +0.59% (p=0.000 n=19+19)
Template-12 61.1ms ± 1% 60.5ms ± 1% -1.02% (p=0.000 n=19+20)
TimeParse-12 307ns ± 0% 303ns ± 1% -1.23% (p=0.000 n=19+15)
TimeFormat-12 323ns ± 0% 323ns ± 0% -0.12% (p=0.011 n=15+20)
[Geo mean] 47.1µs 47.0µs -0.20%
https://perf.golang.org/search?q=upload:20171030.4
It slightly improve the performance the x/benchmarks:
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.29ms ± 3% 2.22ms ± 2% -2.97% (p=0.000 n=18+18)
Garbage/benchmem-MB=64-12 2.24ms ± 2% 2.21ms ± 2% -1.64% (p=0.000 n=18+18)
HTTP-12 12.6µs ± 1% 12.6µs ± 1% ~ (p=0.690 n=19+17)
JSON-12 11.3ms ± 2% 11.3ms ± 1% ~ (p=0.163 n=17+18)
and fixes some of the heap size bloat caused by the previous commit:
name old peak-RSS-bytes new peak-RSS-bytes delta
Garbage/benchmem-MB=1024-12 1.88G ± 2% 1.77G ± 2% -5.52% (p=0.000 n=20+18)
Garbage/benchmem-MB=64-12 248M ± 8% 226M ± 5% -8.93% (p=0.000 n=20+20)
HTTP-12 47.0M ±27% 47.2M ±12% ~ (p=0.512 n=20+20)
JSON-12 206M ±11% 206M ±10% ~ (p=0.841 n=20+20)
https://perf.golang.org/search?q=upload:20171030.5
Combined with the change to add a soft goal in the previous commit,
the achieves a decent performance improvement on the garbage
benchmark:
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.40ms ± 4% 2.22ms ± 2% -7.40% (p=0.000 n=19+18)
Garbage/benchmem-MB=64-12 2.23ms ± 1% 2.21ms ± 2% -1.06% (p=0.000 n=19+18)
HTTP-12 12.5µs ± 1% 12.6µs ± 1% ~ (p=0.330 n=20+17)
JSON-12 11.1ms ± 1% 11.3ms ± 1% +1.87% (p=0.000 n=16+18)
https://perf.golang.org/search?q=upload:20171030.6
Change-Id: If04ddb57e1e58ef2fb9eec54c290eb4ae4bea121
Reviewed-on: https://go-review.googlesource.com/59971
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, GC pacing is based on a single hard heap limit computed
based on GOGC. In order to achieve this hard limit, assist pacing
makes the conservative assumption that the entire heap is live.
However, in the steady state (with GOGC=100), only half of the heap is
live. As a result, the garbage collector works twice as hard as
necessary and finishes half way between the trigger and the goal.
Since this is a stable state for the trigger controller, this repeats
from cycle to cycle. Matters are even worse if GOGC is higher. For
example, if GOGC=200, only a third of the heap is live in steady
state, so the GC will work three times harder than necessary and
finish only a third of the way between the trigger and the goal.
Since this causes the garbage collector to consume ~50% of the
available CPU during marking instead of the intended 25%, about 25% of
the CPU goes to mutator assists. This high mutator assist cost causes
high mutator latency variability.
This commit improves the situation by separating the heap goal into
two goals: a soft goal and a hard goal. The soft goal is set based on
GOGC, just like the current goal is, and the hard goal is set at a 10%
larger heap than the soft goal. Prior to the soft goal, assist pacing
assumes the heap is in steady state (e.g., only half of it is live).
Between the soft goal and the hard goal, assist pacing switches to the
current conservative assumption that the entire heap is live.
In benchmarks, this nearly eliminates mutator assists. However, since
background marking is fixed at 25% CPU, this causes the trigger
controller to saturate, which leads to somewhat higher variability in
heap size. The next commit will address this.
The lower CPU usage of course leads to longer mark cycles, though
really it means the mark cycles are as long as they should have been
in the first place. This does, however, lead to two potential
down-sides compared to the current pacing policy: 1. the total
overhead of the write barrier is higher because it's enabled more of
the time and 2. the heap size may be larger because there's more
floating garbage. We addressed 1 by significantly improving the
performance of the write barrier in the preceding commits. 2 can be
demonstrated in intense GC benchmarks, but doesn't seem to be a
problem in any real applications.
Updates #14951.
Updates #14812 (fixes?).
Fixes#18534.
This has no significant effect on the throughput of the
github.com/dr2chase/bent benchmarks-50.
This has little overall throughput effect on the go1 benchmarks:
name old time/op new time/op delta
BinaryTree17-12 2.41s ± 0% 2.40s ± 0% -0.22% (p=0.007 n=20+18)
Fannkuch11-12 2.95s ± 0% 2.95s ± 0% +0.07% (p=0.003 n=17+18)
FmtFprintfEmpty-12 41.7ns ± 3% 42.2ns ± 0% +1.17% (p=0.002 n=20+15)
FmtFprintfString-12 66.5ns ± 0% 67.9ns ± 2% +2.16% (p=0.000 n=16+20)
FmtFprintfInt-12 77.6ns ± 2% 75.6ns ± 3% -2.55% (p=0.000 n=19+19)
FmtFprintfIntInt-12 124ns ± 1% 123ns ± 1% -0.98% (p=0.000 n=18+17)
FmtFprintfPrefixedInt-12 151ns ± 1% 148ns ± 1% -1.75% (p=0.000 n=19+20)
FmtFprintfFloat-12 210ns ± 1% 212ns ± 0% +0.75% (p=0.000 n=19+16)
FmtManyArgs-12 501ns ± 1% 499ns ± 1% -0.30% (p=0.041 n=17+19)
GobDecode-12 6.50ms ± 1% 6.49ms ± 1% ~ (p=0.234 n=19+19)
GobEncode-12 5.43ms ± 0% 5.47ms ± 0% +0.75% (p=0.000 n=20+19)
Gzip-12 216ms ± 1% 220ms ± 1% +1.71% (p=0.000 n=19+20)
Gunzip-12 38.6ms ± 0% 38.8ms ± 0% +0.66% (p=0.000 n=18+19)
HTTPClientServer-12 78.1µs ± 1% 78.5µs ± 1% +0.49% (p=0.035 n=20+20)
JSONEncode-12 12.1ms ± 0% 12.2ms ± 0% +1.05% (p=0.000 n=18+17)
JSONDecode-12 53.0ms ± 0% 52.3ms ± 0% -1.27% (p=0.000 n=19+19)
Mandelbrot200-12 3.74ms ± 0% 3.69ms ± 0% -1.17% (p=0.000 n=18+19)
GoParse-12 3.17ms ± 1% 3.17ms ± 1% ~ (p=0.569 n=19+20)
RegexpMatchEasy0_32-12 73.2ns ± 1% 73.7ns ± 0% +0.76% (p=0.000 n=18+17)
RegexpMatchEasy0_1K-12 239ns ± 0% 238ns ± 0% -0.27% (p=0.000 n=13+17)
RegexpMatchEasy1_32-12 69.0ns ± 2% 69.1ns ± 1% ~ (p=0.404 n=19+19)
RegexpMatchEasy1_1K-12 367ns ± 1% 365ns ± 1% -0.60% (p=0.000 n=19+19)
RegexpMatchMedium_32-12 105ns ± 1% 104ns ± 1% -1.24% (p=0.000 n=19+16)
RegexpMatchMedium_1K-12 34.1µs ± 2% 33.6µs ± 3% -1.60% (p=0.000 n=20+20)
RegexpMatchHard_32-12 1.62µs ± 1% 1.67µs ± 1% +2.75% (p=0.000 n=18+18)
RegexpMatchHard_1K-12 48.8µs ± 1% 50.3µs ± 2% +3.07% (p=0.000 n=20+19)
Revcomp-12 386ms ± 0% 384ms ± 0% -0.57% (p=0.000 n=20+19)
Template-12 59.9ms ± 1% 61.1ms ± 1% +2.01% (p=0.000 n=20+19)
TimeParse-12 301ns ± 2% 307ns ± 0% +2.11% (p=0.000 n=20+19)
TimeFormat-12 323ns ± 0% 323ns ± 0% ~ (all samples are equal)
[Geo mean] 47.0µs 47.1µs +0.23%
https://perf.golang.org/search?q=upload:20171030.1
Likewise, the throughput effect on the x/benchmarks is minimal (and
reasonably positive on the garbage benchmark with a large heap):
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.40ms ± 4% 2.29ms ± 3% -4.57% (p=0.000 n=19+18)
Garbage/benchmem-MB=64-12 2.23ms ± 1% 2.24ms ± 2% +0.59% (p=0.016 n=19+18)
HTTP-12 12.5µs ± 1% 12.6µs ± 1% ~ (p=0.326 n=20+19)
JSON-12 11.1ms ± 1% 11.3ms ± 2% +2.15% (p=0.000 n=16+17)
It does increase the heap size of the garbage benchmarks, but seems to
have relatively little impact on more realistic programs. Also, we'll
gain some of this back with the next commit.
name old peak-RSS-bytes new peak-RSS-bytes delta
Garbage/benchmem-MB=1024-12 1.21G ± 1% 1.88G ± 2% +55.59% (p=0.000 n=19+20)
Garbage/benchmem-MB=64-12 168M ± 3% 248M ± 8% +48.08% (p=0.000 n=18+20)
HTTP-12 45.6M ± 9% 47.0M ±27% ~ (p=0.925 n=20+20)
JSON-12 193M ±11% 206M ±11% +7.06% (p=0.001 n=20+20)
https://perf.golang.org/search?q=upload:20171030.2
Change-Id: Ic78904135f832b4d64056cbe734ab979f5ad9736
Reviewed-on: https://go-review.googlesource.com/59970
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Previously some of the AuxInt are uint32, which may not fit into
int32. This CL convert them to int32. This does not change the
generated code, but make ssacheck happy.
Pass "toolstash -cmp" for std cmd on ARM.
Fixes#22499.
Change-Id: Ib072d3c14962388bfeb0766c861995d00b4fa7c4
Reviewed-on: https://go-review.googlesource.com/74770
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
All of these had a return or break in the else body, so flipping the
condition means we can unindent and simplify.
Change-Id: If93e97504480d18a0dac3f2c8ffe57ab8bcb929c
Reviewed-on: https://go-review.googlesource.com/74190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, anytime we exported a function or method declaration
(which includes methods for every type transitively exported), we
included the inline function bodies, if any. However, in many cases,
it's impossible (or at least very unlikely) for the importing package
to call the method.
For example:
package p
type T int
func (t T) M() { t.u() }
func (t T) u() {}
func (t T) v() {}
T.M and T.u are inlineable, and they're both reachable through calls
to T.M, which is exported. However, t.v is also inlineable, but cannot
be reached.
Exception: if p.T is embedded in another type q.U, p.T.v will be
promoted to q.U.v, and the generated wrapper function could have
inlined the call to p.T.v. However, in practice, this doesn't happen,
and a missed inlining opportunity doesn't affect correctness.
To implement this, this CL introduces an extra flood fill pass before
exporting to mark inline bodies that are actually reachable, so the
exporter can skip over methods like t.v.
This reduces Kubernetes build time (as measured by "time go build -a
k8s.io/kubernetes/cmd/...") on an HP Z620 measurably:
== before ==
real 0m44.658s
user 11m19.136s
sys 0m53.844s
== after ==
real 0m41.702s
user 10m29.732s
sys 0m50.908s
It also significantly cuts down the cost of enabling mid-stack
inlining (-l=4):
== before (-l=4) ==
real 1m19.236s
user 20m6.528s
sys 1m17.328s
== after (-l=4) ==
real 0m59.100s
user 13m12.808s
sys 0m58.776s
Updates #19348.
Change-Id: Iade58233ca42af823a1630517a53848b5d3c7a7e
Reviewed-on: https://go-review.googlesource.com/74110
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The test was skipped because it did not work on AMD64 with
frame pointer enabled, and accidentally skipped on other
architectures. Now frame pointer is the default on AMD64.
Update the test to work with frame pointer. Now the test
is skipped only when frame pointer is NOT enabled on AMD64.
Fixes#18317.
Change-Id: I724cb6874e562f16e67ce5f389a1d032a2003115
Reviewed-on: https://go-review.googlesource.com/68610
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Since copy function can figure out how many bytes of data to copy when
two slices have different length, it is not necessary to check how many
bytes need to copy each time before copying the data.
Change-Id: I5151ddfe46af5575566fe9c9a2648e111575ec3d
Reviewed-on: https://go-review.googlesource.com/71090
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On PPC64 (and a few other architectures), accessing global
requires multiple instructions and use of temp register.
The compiler emits a single MOV prog, and the assembler
expands it to multiple instructions. If globals are accessed
multiple times, each time it generates a reload of the temp
register. As this is done by the assembler, the compiler
cannot optimize it.
This CL makes the compiler not fold address of global into load
and store. If a global is accessed multiple times, or multiple
fields of a struct are accessed, the compiler can CSE the
address. Currently, this doesn't help the case where different
globals are accessed, even though they may be close to each
other in the address space (which we don't know at compile time).
It helps a little bit in go1 benchmark:
name old time/op new time/op delta
BinaryTree17-2 4.84s ± 1% 4.84s ± 1% ~ (p=0.796 n=10+10)
Fannkuch11-2 4.10s ± 0% 4.08s ± 0% -0.58% (p=0.000 n=9+8)
FmtFprintfEmpty-2 97.9ns ± 1% 96.8ns ± 1% -1.08% (p=0.000 n=10+10)
FmtFprintfString-2 147ns ± 0% 147ns ± 1% ~ (p=0.129 n=9+10)
FmtFprintfInt-2 152ns ± 0% 152ns ± 0% ~ (p=0.294 n=10+8)
FmtFprintfIntInt-2 218ns ± 1% 217ns ± 0% -0.64% (p=0.000 n=10+8)
FmtFprintfPrefixedInt-2 263ns ± 1% 256ns ± 0% -2.77% (p=0.000 n=10+8)
FmtFprintfFloat-2 375ns ± 1% 368ns ± 0% -1.95% (p=0.000 n=10+7)
FmtManyArgs-2 849ns ± 0% 850ns ± 0% ~ (p=0.621 n=8+9)
GobDecode-2 12.3ms ± 1% 12.2ms ± 1% -0.94% (p=0.003 n=10+10)
GobEncode-2 10.3ms ± 1% 10.5ms ± 1% +2.03% (p=0.000 n=10+10)
Gzip-2 414ms ± 1% 414ms ± 0% ~ (p=0.842 n=9+10)
Gunzip-2 66.3ms ± 0% 66.4ms ± 0% ~ (p=0.077 n=9+9)
HTTPClientServer-2 66.3µs ± 5% 66.4µs ± 1% ~ (p=0.661 n=10+9)
JSONEncode-2 23.9ms ± 1% 23.9ms ± 1% ~ (p=0.905 n=10+9)
JSONDecode-2 119ms ± 1% 116ms ± 0% -2.65% (p=0.000 n=10+10)
Mandelbrot200-2 5.11ms ± 0% 4.92ms ± 0% -3.71% (p=0.000 n=10+10)
GoParse-2 5.81ms ± 1% 5.84ms ± 1% ~ (p=0.052 n=10+10)
RegexpMatchEasy0_32-2 315ns ± 0% 317ns ± 0% +0.67% (p=0.000 n=10+10)
RegexpMatchEasy0_1K-2 658ns ± 0% 638ns ± 0% -3.01% (p=0.000 n=9+9)
RegexpMatchEasy1_32-2 315ns ± 1% 317ns ± 0% +0.56% (p=0.000 n=9+9)
RegexpMatchEasy1_1K-2 935ns ± 0% 926ns ± 0% -0.96% (p=0.000 n=9+9)
RegexpMatchMedium_32-2 394ns ± 0% 396ns ± 1% +0.46% (p=0.001 n=10+10)
RegexpMatchMedium_1K-2 65.1µs ± 0% 64.5µs ± 0% -0.90% (p=0.000 n=9+9)
RegexpMatchHard_32-2 3.16µs ± 0% 3.17µs ± 0% +0.35% (p=0.000 n=10+9)
RegexpMatchHard_1K-2 89.4µs ± 0% 89.3µs ± 0% ~ (p=0.136 n=9+9)
Revcomp-2 703ms ± 2% 694ms ± 2% -1.41% (p=0.009 n=10+10)
Template-2 107ms ± 1% 107ms ± 1% ~ (p=0.053 n=9+10)
TimeParse-2 526ns ± 0% 524ns ± 0% -0.34% (p=0.002 n=9+9)
TimeFormat-2 534ns ± 0% 504ns ± 1% -5.51% (p=0.000 n=10+10)
[Geo mean] 93.8µs 93.1µs -0.70%
It also helps in the case mentioned in issue #17110, main.main
in package math's test. Now it generates 4 loads of R31 instead
of 10, for the same piece of code.
This causes a slight increase of binary size: cmd/go increases
0.66%.
If this is a good idea, we should do it on other architectures
where accessing global is expensive.
Updates #17110.
Change-Id: I2687af6eafc04f2a57c19781ec300c33567094b6
Reviewed-on: https://go-review.googlesource.com/68250
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
The new RoundToEven function can be implemented as a single FIDBR
instruction on s390x.
name old time/op new time/op delta
RoundToEven 5.32ns ± 1% 0.86ns ± 1% -83.86% (p=0.000 n=10+10)
Change-Id: Iaf597e57a0d1085961701e3c75ff4f6f6dcebb5f
Reviewed-on: https://go-review.googlesource.com/74350
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Noted in CL 73212 review by crawshaw.
Neglected to update CL 73212 before submitting.
Also fix printing of target goos/goarch for cross-compile build.
Change-Id: If702f23071a4456810f1de6abb9115b38933c5c1
Reviewed-on: https://go-review.googlesource.com/74631
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Every time I see an error that begins `missing argument for Fprintf("%s")`
my mental type-checker goes off, since obviously "%s" is not a valid first
argument to Fprintf. Writing Printf("%s") to report an error in Printf("hello %s")
is almost as confusing.
This CL rewords the errors reported by vet's printf check to be more
consistent with each other, avoid placing context like "in printf call"
in the middle of the message, and to avoid the imprecisions above by
not quoting the format string at all.
Before:
bad.go:9: no formatting directive in Printf call
bad.go:10: missing argument for Printf("%s"): format reads arg 1, have only 0 args
bad.go:11: wrong number of args for format in Printf call: 1 needed but 2 args
bad.go:12: bad syntax for printf argument index: [1]
bad.go:13: index value [0] for Printf("%[0]s"); indexes start at 1
bad.go:14: missing argument for Printf("%[2]s"): format reads arg 2, have only 1 args
bad.go:15: bad syntax for printf argument index: [abc]
bad.go:16: unrecognized printf verb 'z'
bad.go:17: arg "hello" for * in printf format not of type int
bad.go:18: arg fmt.Sprint in printf call is a function value, not a function call
bad.go:19: arg fmt.Sprint in Print call is a function value, not a function call
bad.go:20: arg "world" for printf verb %d of wrong type: string
bad.go:21: missing argument for Printf("%q"): format reads arg 2, have only 1 args
bad.go:22: first argument to Print is os.Stderr
bad.go:23: Println call ends with newline
bad.go:32: arg r in Sprint call causes recursive call to String method
bad.go:34: arg r for printf causes recursive call to String method
After:
bad.go:9: Printf call has arguments but no formatting directives
bad.go:10: Printf format %s reads arg #1, but have only 0 args
bad.go:11: Printf call needs 1 args but has 2 args
bad.go:12: Printf format %[1 is missing closing ]
bad.go:13: Printf format has invalid argument index [0]
bad.go:14: Printf format has invalid argument index [2]
bad.go:15: Printf format has invalid argument index [abc]
bad.go:16: Printf format %.234z has unknown verb z
bad.go:17: Printf format %.*s uses non-int "hello" as argument of *
bad.go:18: Printf format %s arg fmt.Sprint is a func value, not called
bad.go:19: Print arg fmt.Sprint is a func value, not called
bad.go:20: Printf format %d has arg "world" of wrong type string
bad.go:21: Printf format %q reads arg #2, but have only 1 args
bad.go:22: Print does not take io.Writer but has first arg os.Stderr
bad.go:23: Println args end with redundant newline
bad.go:32: Sprint arg r causes recursive call to String method
bad.go:34: Sprintf format %s with arg r causes recursive String method call
Change-Id: I5719f0fb9f2cd84df8ad4c7754ab9b79c691b060
Reviewed-on: https://go-review.googlesource.com/74352
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The support in this CL assumes that something at a higher level than
the toolchain-specific importers is taking care of converting imports
in source code into canonical import paths before invoking the
toolchain-specific importers. That kind of "what does an import mean"
as opposed to "find me the import data for this specific path"
should be provided by higher-level layers.
That's a different layering than the default behavior but matches the
current layering in the compiler and linker and works with the metadata
planned for generation by the go command for package management.
It should also eventually allow the importer code to stop concerning
itself with source directories and vendor import translation and maybe
deprecate ImporterFrom in favor of Importer once again. But that's all
in the future. For now, just make non-nil lookups work, and test that.
Fixes#13847.
Adds #22550.
Change-Id: I048c6a384492e634988a7317942667689ae680ff
Reviewed-on: https://go-review.googlesource.com/74354
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
For #9346#22135 explicitly state under layout constants
that they are not valid time values for Parse. Also add
examples of parsing valid RFC3339 values and the layout
to the example for time.Parse.
Fix capitalisation of time.Parse and Time.Format.
For #20869 include RFC3339 in the list of layouts that do
not accept all the time formats allowed by RFCs (lowercase z).
This does not fully address #20869.
Fixes#9346Fixes#22135
Change-Id: Ia4c13e5745de583db5ef7d5b1688d7768bc42c1b
Reviewed-on: https://go-review.googlesource.com/74231
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: Ifa0384722dd879af7f5edb7b7aaac5ede3cff46d
Reviewed-on: https://go-review.googlesource.com/74690
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The compiler's instrumentation pass has some out-of-date comments
about the write barrier and some confusing comments about
typedslicecopy. Update these comments and add a comment to
typedslicecopy explaining why it's manually instrumented while none of
the other operations are.
Change-Id: I024e5361d53f1c3c122db0c85155368a30cabd6b
Reviewed-on: https://go-review.googlesource.com/74430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When run `go doc -u http.connectMethod`, the whole table is treated as
a single long line. This commit inserts `\t` at the begining of each line,
so the table can be displayed properly in `go doc`.
Change-Id: I6408efd31f84c113e81167d62e1791643000d629
Reviewed-on: https://go-review.googlesource.com/74651
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Hide in the source code instead of in the separate whitelist.
Removes the only printf false positive in the standard library.
Change-Id: I99285e67588c7c93bd56d59ee768a03be7c301e7
Reviewed-on: https://go-review.googlesource.com/74590
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The httpresponse.go module wants to be able to tell if a particular type t
is net/http.Response (and also net/http.Client). It does this by importing
net/http, looking up Response, and then comparing that saved type against
each t.
Instead of doing an eager import of net/http, wait until we have a type t
to ask a question about, and then just look to see if that t is http.Response.
This kind of lazy check does not require assuming that net/http is available
or will be important (perhaps the check is disabled in this run, or perhaps
other conditions that lead to the comparison are not satisfied).
Not loading these kinds of types at startup time will scale better.
Change-Id: Ibb00623901a96e725a4ff6f231e6d15127979dfd
Reviewed-on: https://go-review.googlesource.com/74353
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The signal-to-noise ratio is too low.
Stop printing the name of every package.
Can still get the old output with make.bash -v.
Change-Id: Ib2c82e037166e6d2ddc31ae2a4d29af5becce574
Reviewed-on: https://go-review.googlesource.com/74351
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This cuts 6 seconds off all.bash with the new go command.
Not a ton, but also an easy 6 seconds to grab.
The -tags=use_go_run in the misc/cgo tests is just some
go command flag that will make run.go use go run,
but without making everything look stale.
(Those tests have relative imports,
so go tool compile+link is not enough.)
Change-Id: I43bf4bb661d3adde2b2d4aad5e8f64b97bc69ba9
Reviewed-on: https://go-review.googlesource.com/73994
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The content-based staleness code means that
go run -gcflags=-l helloworld.go
recompiles all of helloworld.go's dependencies with -gcflags=-l,
whereas before it would have assumed installed packages were
up-to-date. In this test, that means every race iteration rebuilds
the runtime and maybe a few other packages. Instead, install them
to a temporary location for reuse.
This speeds the test from 17s to 9s on my MacBook Pro.
Change-Id: Ied136ce72650261083bb19cc7dee38dac0ad05ca
Reviewed-on: https://go-review.googlesource.com/73992
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This cuts 23 seconds from all.bash on my MacBook Pro.
Change-Id: Ibc4d7c01660b9e9ebd088dd55ba993f0d7ec6aa3
Reviewed-on: https://go-review.googlesource.com/73991
Reviewed-by: Ian Lance Taylor <iant@golang.org>
