Martin Möhrmann [Sat, 2 Sep 2017 16:46:59 +0000 (18:46 +0200)]
cmd/compile: specialize map creation for small hint sizes
Handle make(map[any]any) and make(map[any]any, hint) where
hint <= BUCKETSIZE special to allow for faster map initialization
and to improve binary size by using runtime calls with fewer arguments.
Given hint is smaller or equal to BUCKETSIZE in which case
overLoadFactor(hint, 0) is false and no buckets would be allocated by makemap:
* If hmap needs to be allocated on the stack then only hmap's hash0
field needs to be initialized and no call to makemap is needed.
* If hmap needs to be allocated on the heap then a new special
makehmap function will allocate hmap and intialize hmap's
hash0 field.
Reduces size of the godoc by ~36kb.
AMD64
name old time/op new time/op delta
NewEmptyMap 16.6ns ± 2% 5.5ns ± 2% -66.72% (p=0.000 n=10+10)
NewSmallMap 64.8ns ± 1% 56.5ns ± 1% -12.75% (p=0.000 n=9+10)
Updates #6853
Change-Id: I624e90da6775afaa061178e95db8aca674f44e9b
Reviewed-on: https://go-review.googlesource.com/61190
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Blixt [Wed, 1 Nov 2017 20:25:24 +0000 (16:25 -0400)]
time: fix incorrect "zero pad" comment in example
The comment currently implies that a zero will be added, but the
underscore is used to add a space for single-digit dates.
Change-Id: Ib3bac8a16bc2d1fcb26ab3bb7ad172b89e1a4a24
Reviewed-on: https://go-review.googlesource.com/75230 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Wed, 1 Nov 2017 16:06:52 +0000 (17:06 +0100)]
runtime/pprof: use switch for GOOS check in testCPUProfile
Since CL 33071, testCPUProfile is only one user of the badOS map.
Replace it by the corresponding switch, with the "plan9" case removed
because it is already checked earlier in the same function.
Change-Id: Id647b8ee1fd37516bb702b35b3c9296a4f56b61b
Reviewed-on: https://go-review.googlesource.com/75110
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Wed, 1 Nov 2017 23:57:50 +0000 (19:57 -0400)]
cmd/go: add cache verification mode
If GODEBUG=gocacheverify=1, then instead of using the cache to
avoid computations, the go command will do the computations and
double-check that they match any existing cache entries.
This is handled entirely in the cache implementation; there's no
complexity added to any of the cache usage sites.
(As of this CL there aren't any cache usage sites, but soon there will be.)
Also change GOCMDDEBUGHASH to the more usual GODEBUG=gocachehash=1.
Russ Cox [Wed, 1 Nov 2017 23:22:56 +0000 (19:22 -0400)]
cmd/dist: set GOCACHE during make.bash/run.bash
Use a build cache separate from the default user cache,
one that will be wiped out during startup, so that make.bash
continues to start from a clean slate.
The cache is stored in $GOCACHE, which is printed by go env and
defaults to a subdirectory named "go-build" in the standard user cache
directory for the host operating system.
This CL only implements the cache. Future CLs will store data in it.
griesemer [Wed, 1 Nov 2017 21:24:06 +0000 (14:24 -0700)]
go/types: avoid repeated "declared but not used" errors for closure variables
At the end of type-checking a function or closure, unused local variables
are reported by looking at all variables in the function scope and its
nested children scopes. If a nested scope belonged to a nested function
(closure), that scope would be searched twice, leading to multiple error
messages for unused variables.
This CL introduces an internal-only marker to identify function scopes
so that they can be ignored where needed.
Fixes #22524.
Change-Id: If58cc17b2f0615a16f33ea262f50dffd0e86d0f0
Reviewed-on: https://go-review.googlesource.com/75251 Reviewed-by: Alan Donovan <adonovan@google.com>
Evan Jones [Wed, 1 Nov 2017 16:44:14 +0000 (12:44 -0400)]
syscall: use setattrlist for UtimesNano on Darwin for ns resolution
Mac OS X 10.13 introduced APFS which stores nanosecond resolution
timestamps. The implementation of os.Stat already returns full
resolution timestamps, but os.Chtimes only sets timestamps with
microsecond resolution.
Fix this by using setattrlist on Darwin, which takes a struct timeval
with nanosecond resolution. This is what Mac OS X 10.13 appears uses
to implement utimensat, according to dtruss.
Fixes #22528
Change-Id: I397dabef6b2b73a081382999aa4c4405ab8c6015
Reviewed-on: https://go-review.googlesource.com/74952
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tim Cooper [Wed, 27 Sep 2017 22:46:58 +0000 (19:46 -0300)]
crypto, hash: implement BinaryMarshaler, BinaryUnmarshaler in hash implementations
The marshal method allows the hash's internal state to be serialized and
unmarshaled at a later time, without having the re-write the entire stream
of data that was already written to the hash.
Fixes #20573
Change-Id: I40bbb84702ac4b7c5662f99bf943cdf4081203e5
Reviewed-on: https://go-review.googlesource.com/66710 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Leigh McCulloch [Fri, 27 Oct 2017 07:04:57 +0000 (07:04 +0000)]
encoding/xml: ignore whitespace in values and attrs
Whitespace is ignored in bool values and attrs. It is convenient and
relatively safe since whitespace around a bool value is often
unimportant. The same logic can be applied to numeric values of types
int, uint, and float.
Fixes #22146
Change-Id: Ie0462def90304af144b8e2e72d85b644857c27cc
Reviewed-on: https://go-review.googlesource.com/73891 Reviewed-by: Sam Whited <sam@samwhited.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Sam Whited <sam@samwhited.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mikio Hara [Fri, 27 Oct 2017 09:13:17 +0000 (18:13 +0900)]
net/smtp: don't call testing.T.Fatal{,f} from goroutines not running Test function
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>
Dmitri Shuralyov [Thu, 21 Sep 2017 05:53:13 +0000 (01:53 -0400)]
net/http: set Content-Type header for HEAD as well
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>
Radek Sohlich [Tue, 31 Oct 2017 16:59:29 +0000 (17:59 +0100)]
os/signal: improve documentation for the Notify function
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>
Cherry Zhang [Fri, 6 Oct 2017 19:00:02 +0000 (15:00 -0400)]
cmd/internal/goobj: accept int64 in readInt
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.
Alberto Donizetti [Wed, 1 Nov 2017 11:36:56 +0000 (12:36 +0100)]
math/big: avoid unnecessary Newton iteration in Float.Sqrt
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.
Joe Kyo [Tue, 31 Oct 2017 09:24:44 +0000 (09:24 +0000)]
net/http: fix typo in doc string
Change-Id: I4542f6c095a35a4dec03c67c45a75a155197eb56
Reviewed-on: https://go-review.googlesource.com/74650 Reviewed-by: Tom Bergan <tombergan@google.com>
Alessandro Arzilli [Sat, 21 Oct 2017 10:45:23 +0000 (12:45 +0200)]
compile, link: remove base address selector from DWARF range lists
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.
Russ Cox [Tue, 31 Oct 2017 16:59:47 +0000 (12:59 -0400)]
cmd/go: pass package config to vet during "go vet"
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.
Change-Id: I932626ee7da649b302cd269b82eb6fe5d7b9f0f2
Reviewed-on: https://go-review.googlesource.com/74750 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Mon, 30 Oct 2017 15:16:09 +0000 (11:16 -0400)]
cmd/vet: accept package config from go command
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>
Martin Möhrmann [Sat, 12 Aug 2017 15:37:13 +0000 (17:37 +0200)]
runtime: protect growslice against newcap*et.size overflow
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:
Russ Cox [Tue, 31 Oct 2017 19:42:17 +0000 (15:42 -0400)]
cmd/link: do not store compilation directory in DWARF info
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.
Michael Fraenkel [Sun, 29 Oct 2017 00:50:57 +0000 (20:50 -0400)]
encoding/json: Include the offset of a SyntaxError
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>
Ivan Bertona [Tue, 31 Oct 2017 20:16:38 +0000 (13:16 -0700)]
encoding/json: disallow unknown fields in Decoder
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>
Hana (Hyang-Ah) Kim [Mon, 24 Jul 2017 15:51:03 +0000 (11:51 -0400)]
runtime/trace: fix corrupted trace during StartTrace
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).
runtime: allow 5% mutator assist over 25% background mark
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):
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.
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.
Cherry Zhang [Tue, 31 Oct 2017 15:48:13 +0000 (11:48 -0400)]
cmd/compile: on ARM, make sure *const's AuxInt fit into int32
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.
Daniel Martí [Sat, 28 Oct 2017 16:35:27 +0000 (17:35 +0100)]
all: unindent some if bodies by exiting early
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>
Cherry Zhang [Thu, 5 Oct 2017 20:14:14 +0000 (16:14 -0400)]
test: fix and re-enable nosplit.go
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.
Joe Kyo [Mon, 16 Oct 2017 06:25:14 +0000 (07:25 +0100)]
crypto/tls: remove bookkeeping code from pHash function
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.
Cherry Zhang [Wed, 4 Oct 2017 18:33:10 +0000 (14:33 -0400)]
cmd/compile: don't fold address of global into load/store on PPC64
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 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.
Russ Cox [Sun, 29 Oct 2017 19:18:52 +0000 (15:18 -0400)]
cmd/vet: tighten printf format error messages
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
Russ Cox [Mon, 30 Oct 2017 15:13:27 +0000 (11:13 -0400)]
go/importer: support lookup in importer.For
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.
Kenny Grant [Sat, 28 Oct 2017 22:03:15 +0000 (23:03 +0100)]
time: document that valid layouts are not valid Parse values
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 #9346
Fixes #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>
Ramazan AYYILDIZ [Tue, 31 Oct 2017 10:19:48 +0000 (11:19 +0100)]
strings: add examples for specialCase
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>
Joe Kyo [Tue, 31 Oct 2017 09:47:27 +0000 (09:47 +0000)]
net/http: display connect methods table properly in `go doc`
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>
Russ Cox [Mon, 30 Oct 2017 14:47:30 +0000 (10:47 -0400)]
cmd/vet: do not import net/http at startup
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>
Russ Cox [Fri, 27 Oct 2017 18:11:21 +0000 (14:11 -0400)]
test/run: use go tool compile + link instead of go run when possible
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>
Russ Cox [Fri, 27 Oct 2017 17:45:50 +0000 (13:45 -0400)]
runtime/race: install alternate packages to temp dir
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>
Russ Cox [Fri, 27 Oct 2017 17:30:09 +0000 (13:30 -0400)]
runtime: shorten tests in all.bash
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>
Russ Cox [Fri, 27 Oct 2017 17:07:38 +0000 (13:07 -0400)]
cmd/dist: log timing to $GOBUILDTIMELOGFILE
We can't make all.bash faster if we can't measure it.
Measure it.
Change-Id: Ia5da791d4cfbfa1fd9a8e905b3188f63819ade73
Reviewed-on: https://go-review.googlesource.com/73990 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Thu, 12 Oct 2017 00:39:28 +0000 (20:39 -0400)]
cmd/go: switch to entirely content-based staleness determination
This CL changes the go command to base all its rebuilding decisions
on the content of the files being processed and not their file system
modification times. It also eliminates the special handling of release
toolchains, which were previously considered always up-to-date
because modification time order could not be trusted when unpacking
a pre-built release.
The go command previously tracked "build IDs" as a backup to
modification times, to catch changes not reflected in modification times.
For example, if you remove one .go file in a package with multiple .go
files, there is no modification time remaining in the system that indicates
that the installed package is out of date. The old build ID was the hash
of a list of file names and a few other factors, expected to change if
those factors changed.
This CL moves to using this kind of build ID as the only way to
detect staleness, making sure that the build ID hash includes all
possible factors that need to influence the rebuild decision.
One such factor is the compiler flags. As of this CL, if you run
go build -gcflags -N cmd/gofmt
you will get a gofmt where every package is built with -N,
regardless of what may or may not be installed already.
Another such factor is the linker flags. As of this CL, if you run
go install myprog
go install -ldflags=-s myprog
the second go install will now correctly build a new myprog with
the updated linker flags. (Previously the installed myprog appeared
up-to-date, because the ldflags were not included in the build ID.)
Because we have more precise information we can also validate whether
the target of a "go test -c" operation is already the right binary and
therefore can avoid a rebuild.
This CL sets us up for having a more general build artifact cache,
maybe even a step toward not having a pkg directory with .a files,
but this CL does not take that step. For now the result of go install
is the same as it ever was; we just do a better job of what needs to
be installed.
This CL does slow down builds a small amount by reading all the
dependent source files in full. (The go command already read the
beginning of every dependent source file to discover build tags
and imports.) On my MacBook Pro, before this CL all.bash takes
3m58s, while after this CL and a few optimizations stacked above it
all.bash takes 4m28s. Given that CL 73850 cut 1m43s off the all.bash
time earlier today, we can afford adding 30s back for now.
More optimizations are planned that should make the go command
more efficient than it was even before this CL.
Russ Cox [Mon, 30 Oct 2017 23:53:58 +0000 (19:53 -0400)]
cmd/compile: change ssa test to avoid go run -gcflags=-d=ssa/check/on
In the new content-based staleness world, setting -gcflags like this
recompiles all the packages involved in running the program, not just
the "stale" ones. So go run -gcflags=-d=ssa/check/on recompiles
runtime with those flags too, which is not what the test is trying
to check.
Russ Cox [Mon, 30 Oct 2017 19:28:11 +0000 (15:28 -0400)]
all: respect $GO_GCFLAGS during run.bash
If the go install doesn't use the same flags as the main build
it can overwrite the installed standard library, leading to
flakiness and slow future tests.
Force uses of 'go install' etc to propagate $GO_GCFLAGS
or disable them entirely, to avoid problems.
As I understand it, the main place this happens is the ssacheck builder.
If there are other uses that need to run some of the now-disabled
tests we can reenable fixed tests in followup CLs.
Change-Id: Ib860a253539f402f8a96a3c00ec34f0bbf137c9a
Reviewed-on: https://go-review.googlesource.com/74470 Reviewed-by: David Crawshaw <crawshaw@golang.org>
Allows code that operates on a FlagSet to know the name and error
handling behavior of the FlagSet without having to call FlagSet.Init.
Fixes #17628
Fixes #21888
Change-Id: Ib0fe4c8885f9ccdacf5a7fb761d5ecb23f3bb055
Reviewed-on: https://go-review.googlesource.com/70391
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jason Wangsadinata [Mon, 30 Oct 2017 16:04:03 +0000 (23:04 +0700)]
container/ring: fix example_test.go
The Len method is a linear operation. CL 73090 used Len to iterate over
a ring, resulting in a quadratic time operation.
Change-Id: Ib69c19190ba648311e6c345d8cb26292b50121ee
Reviewed-on: https://go-review.googlesource.com/74390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Matthew Dempsky [Mon, 30 Oct 2017 19:16:30 +0000 (12:16 -0700)]
cmd/compile: skip compiling wrappers for imported defined types
When compiling a package that defines a type T with method T.M, we
already compile and emit the wrapper method (*T).M. There's no need
for every package that uses T to do the same.
Change-Id: I3ca2659029907570f8b98d66111686435fad7ed0
Reviewed-on: https://go-review.googlesource.com/74412
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
Mark Theunissen [Tue, 25 Jul 2017 10:47:39 +0000 (12:47 +0200)]
net/url: preserve leading slashes when resolving path
When doing resolvePath, if there are multiple leading slashes in the
target, preserve them. This prevents an issue where the Go http.Client
cleans up multiple leading slashes in the Location header in a
redirect, resulting in a redirection to the incorrect target.
David Chase [Mon, 30 Oct 2017 15:39:29 +0000 (11:39 -0400)]
cmd/compile: adjust expectations of test for issue 18902
The test for #18902 reads the assembly stream to be sure
that the line number does not change too often (this is an
indication that debugging the code will be unpleasant and
that the compiler is probably getting line numbers "wrong").
It checks that it is getting "enough" input, but the
compiler has gotten enough better since the test was written
that it now fails for lack of enough input. The old
threshould was 200 instructions, the new one is 150 (the
minimum observed input is on arm64 with 184 instructions).
Keith Randall [Sat, 28 Oct 2017 17:14:08 +0000 (10:14 -0700)]
cmd/compile: fix runtime.KeepAlive
KeepAlive needs to introduce a use of the spill of the
value it is keeping alive. Without that, we don't guarantee
that the spill dominates the KeepAlive.
This bug was probably introduced with the code to move spills
down to the dominator of the restores, instead of always spilling
just after the value itself (CL 34822).
Russ Cox [Mon, 30 Oct 2017 18:15:01 +0000 (14:15 -0400)]
cmd/dist: force non-devel version for cross-build buildlets
If the compiler has a non-devel version it will report that version
to the go command for use as the "compiler ID" instead of using
the content ID of the binary. This in turn allows the go command
to see the compiled-for-amd64 arm compiler and the compiled-for-arm
arm compiler as having the same ID, so that packages cross-compiled
from amd64 look up-to-date when copied to the arm system
during the linux-arm buildlets and trybots.
Michael Munday [Sun, 17 Sep 2017 16:20:35 +0000 (17:20 +0100)]
math: optimize dim and remove s390x assembly implementation
By calculating dim directly, rather than calling max, we can simplify
the generated code significantly. The compiler now reports that dim
is easily inlineable, but it can't be inlined because there is still
an assembly stub for Dim.
Since dim is now very simple I no longer think it is worth having
assembly implementations of it. I have therefore removed the s390x
assembly. Removing the other assembly for Dim is #21913.
name old time/op new time/op delta
Dim 4.29ns ± 0% 3.53ns ± 0% -17.62% (p=0.000 n=9+8)
Change-Id: Ic38a6b51603cbc661dcdb868ecf2b1947e9f399e
Reviewed-on: https://go-review.googlesource.com/64194
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Terin Stock [Tue, 17 Oct 2017 00:34:35 +0000 (17:34 -0700)]
net/http/pprof: attach handlers using http.HandleFunc
Simplify how pprof attaches the handlers to the DefaultMux by using
http.HandleFunc instead of manually wrapping the handlers in
a http.HandlerFunc.
Change-Id: I65db262ebb2e29e4b6f30df9d2688f5daf782c29
Reviewed-on: https://go-review.googlesource.com/71251 Reviewed-by: Sam Whited <sam@samwhited.com> Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Sam Whited <sam@samwhited.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Fri, 27 Oct 2017 17:48:08 +0000 (13:48 -0400)]
runtime: use buffered write barrier for bulkBarrierPreWrite
This modifies bulkBarrierPreWrite to use the buffered write barrier
instead of the eager write barrier. This reduces the number of system
stack switches and sanity checks by a factor of the buffer size
(currently 256). This affects both typedmemmove and typedmemclr.
Since this is purely a runtime change, it applies to all arches
(unlike the pointer write barrier).
name old time/op new time/op delta
BulkWriteBarrier-12 7.33ns ± 6% 4.46ns ± 9% -39.10% (p=0.000 n=20+19)
Austin Clements [Fri, 27 Oct 2017 19:30:19 +0000 (15:30 -0400)]
runtime: simplify and optimize typedslicecopy
Currently, typedslicecopy meticulously performs a typedmemmove on
every element of the slice. This probably used to be necessary because
we only had an individual element's type, but now we use the heap
bitmap, so we only need to know whether the type has any pointers and
how big it is. Hence, this CL rewrites typedslicecopy to simply
perform one bulk barrier and one memmove.
This also has a side-effect of eliminating two unnecessary write
barriers per slice element that were coming from updates to dstp and
srcp, which were stored in the parent stack frame. However, most of
the win comes from eliminating the loops.
name old time/op new time/op delta
BulkWriteBarrier-12 7.83ns ±10% 7.33ns ± 6% -6.45% (p=0.000 n=20+20)
Austin Clements [Thu, 26 Oct 2017 16:33:04 +0000 (12:33 -0400)]
cmd/compile: compiler support for buffered write barrier
This CL implements the compiler support for calling the buffered write
barrier added by the previous CL.
Since the buffered write barrier is only implemented on amd64 right
now, this still supports the old, eager write barrier as well. There's
little overhead to supporting both and this way a few tests in
test/fixedbugs that expect to have liveness maps at write barrier
calls can easily opt-in to the old, eager barrier.
This significantly improves the performance of the write barrier:
name old time/op new time/op delta
WriteBarrier-12 73.5ns ±20% 19.2ns ±27% -73.90% (p=0.000 n=19+18)
It also reduces the size of binaries because the write barrier call is
more compact:
(Amusingly, this also reduces compiler allocations by 0.75%, which,
combined with the better write barrier, speeds up the compiler overall
by 2.10%. See the perf link.)
It slightly improves the performance of most of the go1 benchmarks and
improves the performance of the x/benchmarks:
Austin Clements [Thu, 26 Oct 2017 16:21:16 +0000 (12:21 -0400)]
runtime: buffered write barrier implementation
This implements runtime support for buffered write barriers on amd64.
The buffered write barrier has a fast path that simply enqueues
pointers in a per-P buffer. Unlike the current write barrier, this
fast path is *not* a normal Go call and does not require the compiler
to spill general-purpose registers or put arguments on the stack. When
the buffer fills up, the write barrier takes the slow path, which
spills all general purpose registers and flushes the buffer. We don't
allow safe-points or stack splits while this frame is active, so it
doesn't matter that we have no type information for the spilled
registers in this frame.
One minor complication is cgocheck=2 mode, which uses the write
barrier to detect Go pointers being written to non-Go memory. We
obviously can't buffer this, so instead we set the buffer to its
minimum size, forcing the write barrier into the slow path on every
call. For this specific case, we pass additional information as
arguments to the flush function. This also requires enabling the cgo
write barrier slightly later during runtime initialization, after Ps
(and the per-P write barrier buffers) have been initialized.
The code in this CL is not yet active. The next CL will modify the
compiler to generate calls to the new write barrier.
This reduces the average cost of the write barrier by roughly a factor
of 4, which will pay for the cost of having it enabled more of the
time after we make the GC pacer less aggressive. (Benchmarks will be
in the next CL.)
Austin Clements [Fri, 27 Oct 2017 19:20:21 +0000 (15:20 -0400)]
runtime: make systemstack tail call if already switched
Currently systemstack always calls its argument, even if we're already
on the system stack. Unfortunately, traceback with _TraceJump stops at
the first systemstack it sees, which often cuts off runtime stacks
early in profiles.
Fix this by performing a tail call if we're already on the system
stack. This eliminates it from the traceback entirely, so it won't
stop prematurely (or all get mushed into a single node in the profile
graph).