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).
cmd/compile,cmd/internal/obj/ppc64: make math.Abs,math.Copysign instrinsics on ppc64x
This adds support for math Abs, Copysign to be instrinsics on ppc64x.
New instruction FCPSGN is added to generate fcpsgn. Some new
rules are added to improve the int<->float conversions that are
generated mainly due to the Float64bits and Float64frombits in
the math package. PPC64.rules is also modified as suggested
in the review for CL 63290.
Others that used Copysign also saw smaller improvements.
I attempted to make this work using rules since that
seems to be preferred, but due to the use of Float64bits and
Float64frombits in these functions, several rules had to be added and
even then not all cases were matched. Using rules became too
complicated and seemed too fragile for these.
Lynn Boger [Fri, 27 Oct 2017 14:06:40 +0000 (10:06 -0400)]
runtime: use -buildmode=pie in testCgoPprofPIE instead of -extldflags=-pie
Errors occur in runtime test testCgoPprofPIE when the test
is built by passing -pie to the external linker with code
that was not built as PIC. This occurs on ppc64le because
non-PIC is the default, and fails only on newer distros
where the address range used for programs is high enough
to cause relocation overflow. This test should be built
with -buildmode=pie since that correctly generates PIC
with -pie.
Related issues are #21954 and #22126.
Updates #22459
Change-Id: Ib641440bc9f94ad2b97efcda14a4b482647be8f7
Reviewed-on: https://go-review.googlesource.com/73970
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Hugues Bruant [Wed, 25 Oct 2017 04:57:51 +0000 (21:57 -0700)]
cmd/compile: fix incorrect go:noinline usage
This pragma is not actually honored by the compiler.
The tests implicitly relied on the inliner being unable
to inline closures with captured variables, which will
soon change.
Fixes #22208
Change-Id: I13abc9c930b9156d43ec216f8efb768952a29439
Reviewed-on: https://go-review.googlesource.com/73211 Reviewed-by: Michael Munday <mike.munday@ibm.com>
Russ Cox [Sat, 28 Oct 2017 18:23:21 +0000 (14:23 -0400)]
cmd/go: adjust default GOROOT to prefer runtime.GOROOT() spelling
If runtime.GOROOT() and the os.Executable method for finding GOROOT
find the same directory but with different spellings, prefer the spelling
returned by runtime.GOROOT().
This avoids an inconsistency if "pwd" returns one spelling but a
different spelling is used in $PATH (and therefore in os.Executable()).
make.bash runs with GOROOT=$(cd .. && pwd); the goal is to allow
the resulting toolchain to use that default setting (unless moved)
even if the directory spelling is different in $PATH.
Change-Id: If96b28b9e8697f4888f153a400b40bbf58a9128b
Reviewed-on: https://go-review.googlesource.com/74250
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>
recordspan has two remaining write barriers from writing to the
pointer to the backing store of h.allspans. However, h.allspans is
always backed by off-heap memory, so let the compiler know this.
Unfortunately, this isn't quite as clean as most go:notinheap uses
because we can't directly name the backing store of a slice, but we
can get it done with some judicious casting.
For #22460.
Change-Id: I296f92fa41cf2cb6ae572b35749af23967533877
Reviewed-on: https://go-review.googlesource.com/73414 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Tue, 24 Oct 2017 21:11:32 +0000 (17:11 -0400)]
cmd/compile: elide write barriers for copy of notinheap pointers
Currently copy and append for types containing only scalars and
notinheap pointers still get compiled to have write barriers, even
though those write barriers are unnecessary. Fix these to use
HasHeapPointer instead of just Haspointer so that they elide write
barriers when possible.
This fixes the unnecessary write barrier in runtime.recordspan when it
grows the h.allspans slice. This is important because recordspan gets
called (*very* indirectly) from (*gcWork).tryGet, which is
go:nowritebarrierrec. Unfortunately, the compiler's analysis has no
hope of seeing this because it goes through the indirect call
fixalloc.first, but I saw it happen.
Austin Clements [Tue, 24 Oct 2017 20:34:33 +0000 (16:34 -0400)]
cmd/compile: make HasHeapPointer recursive
Currently (*Type).HasHeapPointer only ignores pointers go:notinheap
types if the type itself is a pointer to a go:notinheap type. However,
if it's some other type that contains pointers where all of those
pointers are go:notinheap, it will conservatively return true. As a
result, we'll use write barriers where they aren't needed, for example
calling typedmemmove instead of just memmove on structs that contain
only go:notinheap pointers.
Fix this by making HasHeapPointer walk the whole type looking for
pointers that aren't marked go:notinheap.
Austin Clements [Tue, 24 Oct 2017 21:10:02 +0000 (17:10 -0400)]
cmd/compile: report typedslicecopy write barriers
Most write barrier calls are inserted by SSA, but copy and append are
lowered to runtime.typedslicecopy during walk. Fix these to set
Func.WBPos and emit the "write barrier" warning, as done for the write
barriers inserted by SSA. As part of this, we refactor setting WBPos
and emitting this warning into the frontend so it can be shared by
both walk and SSA.
Adam Langley [Sat, 14 Oct 2017 18:43:17 +0000 (11:43 -0700)]
crypto/{ecdsa,rsa}: rename argument to PrivateKey.Sign.
The crypto.Signer interface takes pre-hased messages for ECDSA and RSA,
but the argument in the implementations was called “msg”, not “digest”,
which is confusing.
This change renames them to help clarify the intended use.
Austin Clements [Sun, 22 Oct 2017 20:36:27 +0000 (16:36 -0400)]
cmd/compile: improve coverage of nowritebarrierrec check
The current go:nowritebarrierrec checker has two problems that limit
its coverage:
1. It doesn't understand that systemstack calls its argument, which
means there are several cases where we fail to detect prohibited write
barriers.
2. It only observes calls in the AST, so calls constructed during
lowering by SSA aren't followed.
This CL completely rewrites this checker to address these issues.
The current checker runs entirely after walk and uses visitBottomUp,
which introduces several problems for checking across systemstack.
First, visitBottomUp itself doesn't understand systemstack calls, so
the callee may be ordered after the caller, causing the checker to
fail to propagate constraints. Second, many systemstack calls are
passed a closure, which is quite difficult to resolve back to the
function definition after transformclosure and walk have run. Third,
visitBottomUp works exclusively on the AST, so it can't observe calls
created by SSA.
To address these problems, this commit splits the check into two
phases and rewrites it to use a call graph generated during SSA
lowering. The first phase runs before transformclosure/walk and simply
records systemstack arguments when they're easy to get. Then, it
modifies genssa to record static call edges at the point where we're
lowering to Progs (which is the latest point at which position
information is conveniently available). Finally, the second phase runs
after all functions have been lowered and uses a direct BFS walk of
the call graph (combining systemstack calls with static calls) to find
prohibited write barriers and construct nice error messages.
Austin Clements [Sun, 22 Oct 2017 22:10:08 +0000 (18:10 -0400)]
runtime: allow write barriers in gchelper
We're about to start tracking nowritebarrierrec through systemstack
calls, which detects that we're calling markroot (which has write
barriers) from gchelper, which is called from the scheduler during STW
apparently without a P.
But it turns out that func helpgc, which wakes up blocked Ms to run
gchelper, installs a P for gchelper to use. This means there *is* a P
when gchelper runs, so it is allowed to have write barriers. Tell the
compiler this by marking gchelper go:yeswritebarrierrec. Also,
document the call to gchelper so I don't have to spend another half a
day puzzling over how on earth this could possibly work before
discovering the spooky action-at-a-distance in helpgc.
Austin Clements [Sun, 22 Oct 2017 22:10:08 +0000 (18:10 -0400)]
runtime: eliminate write barriers from persistentalloc
We're about to start tracking nowritebarrierrec through systemstack
calls, which will reveal write barriers in persistentalloc prohibited
by various callers.
The pointers manipulated by persistentalloc are always to off-heap
memory, so this removes these write barriers statically by introducing
a new go:notinheap type to represent generic off-heap memory.
Austin Clements [Sun, 22 Oct 2017 22:10:08 +0000 (18:10 -0400)]
runtime: allow write barriers in startpanic_m
We're about to start tracking nowritebarrierrec through systemstack
calls, which will reveal write barriers in startpanic_m prohibited by
various callers.
We actually can allow write barriers here because the write barrier is
a no-op when we're panicking. Let the compiler know.
Austin Clements [Thu, 19 Oct 2017 23:57:46 +0000 (19:57 -0400)]
runtime: mark gcWork methods nowritebarrierrec
Currently most of these are marked go:nowritebarrier as a hint, but
it's actually important that these not invoke write barriers
recursively. The danger is that some gcWork method would invoke the
write barrier while the gcWork is in an inconsistent state and that
the write barrier would in turn invoke some other gcWork method, which
would crash or permanently corrupt the gcWork. Simply marking the
write barrier itself as go:nowritebarrierrec isn't sufficient to
prevent this if the write barrier doesn't use the outer method.
Thankfully, this doesn't cause any build failures, so we were getting
this right. :)
Austin Clements [Mon, 23 Oct 2017 01:37:05 +0000 (21:37 -0400)]
runtime: remove write barriers from newstack, gogo
Currently, newstack and gogo have write barriers for maintaining the
context register saved in g.sched.ctxt. This is troublesome, because
newstack can be called from go:nowritebarrierrec places that can't
allow write barriers. It happens to be benign because g.sched.ctxt
will always be nil on entry to newstack *and* it so happens the
incoming ctxt will also always be nil in these contexts (I
think/hope), but this is playing with fire. It's also desirable to
mark newstack go:nowritebarrierrec to prevent any other, non-benign
write barriers from creeping in, but we can't do that right now
because of this one write barrier.
Fix all of this by observing that g.sched.ctxt is really just a saved
live pointer register. Hence, we can shade it when we scan g's stack
and otherwise move it back and forth between the actual context
register and g.sched.ctxt without write barriers. This means we can
save it in morestack along with all of the other g.sched, eliminate
the save from newstack along with its troublesome write barrier, and
eliminate the shenanigans in gogo to invoke the write barrier when
restoring it.
Once we've done all of this, we can mark newstack
go:nowritebarrierrec.
Tobias Klauser [Sat, 28 Oct 2017 11:45:36 +0000 (13:45 +0200)]
.gitignore: ignore src/cmd/dist/dist
In case of a failed/cancelled build, src/cmd/dist/dist might be left in
place.
Change-Id: Id81b5d663476a880101a2eed54fa051c40b0b0bc
Reviewed-on: https://go-review.googlesource.com/74150 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alberto Donizetti [Mon, 9 Oct 2017 07:54:35 +0000 (09:54 +0200)]
cmd/go: add a test to ensure upx works on go binaries
On linux/amd64, for now.
Updates #16706
Change-Id: Ib8c89b6edc73fb88042c06873ff815d387491504
Reviewed-on: https://go-review.googlesource.com/69117
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 27 Oct 2017 17:52:59 +0000 (13:52 -0400)]
api: update next.txt
Change-Id: Ib342468f3727be0cd6268ab824ad06d783ee7c94
Reviewed-on: https://go-review.googlesource.com/73993 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 27 Oct 2017 20:48:02 +0000 (16:48 -0400)]
cmd/dist: translate /private/var to /var on darwin builders
This is ugly but needed on the builders, because they do not set
PWD/GOROOT consistently, and the new content-based staleness
understands that the setting of GOROOT influences the content in
the linker outputs.
Joshua Rubin [Fri, 27 Oct 2017 07:47:08 +0000 (01:47 -0600)]
net/http: fix minor leak in Header.WriteSubset
Header.WriteSubset uses a sync.Pool but wouldn't Put the sorter back in
the pool if there was an error writing to the io.Writer
I'm not really sure why the sorter is returned to begin with. The
comment says "for possible return to headerSorterCache".
This also doesn't address potential panics that might occur, but the
overhead of doing the Put in a defer would likely be too great.
Change-Id: If3c45a4c3e11f6ec65d187e25b63455b0142d4e3
Reviewed-on: https://go-review.googlesource.com/73910
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
Than McIntosh [Tue, 24 Oct 2017 20:08:46 +0000 (16:08 -0400)]
cmd/compile, cmd/link: support for DWARF file reference relocations
New relocation flavor R_DWARFFILEREF, to be applied to DWARF attribute
values that correspond to file references (ex: DW_AT_decl_file,
DW_AT_call_file). The LSym for this relocation is the file itself; the
linker replaces the relocation target with the index of the specified
file in the line table's file section.
Note: for testing purposes this patch changes the DWARF function
subprogram DIE abbrev to include DW_AT_decl_file (allowed by DWARF
but not especially useful) so as to have a way to test this
functionality. This attribute will be removed once there are other
file reference attributes (coming as part of inlining support).
Russ Cox [Fri, 27 Oct 2017 14:34:25 +0000 (10:34 -0400)]
cmd/dist: use latest heap, sort packages for compiler bootstrap
The compiler depends on the way heap and sort break ties
in some cases. Instead of trying to find them all, bundle
those packages into the bootstrap compiler builds.
The overall goal is that Go1.4 building cmd/compile during the
bootstrap process produces a semantically equivalent compiler
to cmd/compile compiling itself. After this CL, that property is true,
at least for the compiler compiling itself and the other tools.
Russ Cox [Fri, 27 Oct 2017 14:21:46 +0000 (10:21 -0400)]
sort: split post-Go1.4 code into its own file
This will let us build the latest sort when bootstrapping the compiler.
The compiler depends on the precise tie-breaks used by sort in
some cases, and it's easier to bring sort along than require checking
every sort call ever added to the compiler.
Than McIntosh [Thu, 26 Oct 2017 14:15:29 +0000 (10:15 -0400)]
cmd/link: don't pass -gdwarf-2 to external linker
Don't pass -gdwarf-2 to the external linker when external linkage is
requested. The Go compiler is now emitting DWARF version 4, so this
doesn't seem needed any more.
Fixes #22455
Change-Id: Ic4122c55e946619a266430f2d26f06d6803dd232
Reviewed-on: https://go-review.googlesource.com/73672 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Russ Cox [Fri, 27 Oct 2017 02:08:23 +0000 (22:08 -0400)]
cmd/go: delay gcc/clang flag support checks
env.MkEnv was computing the full gcc command line to report as
$GOGCCFLAGS in "go env" output, which meant running gcc (or clang)
multiple times to discern which flags are available.
We also set $GOGCCFLAGS in the environment, but nothing actually uses
that as far as I can tell - it was always intended only for debugging.
Move GOGCCFLAGS to env.ExtraEnvVars, which displayed in "go env"
output but not set in child processes and not computed nearly as
often.
The effect is that trivial commands like "go help" or "go env GOARCH"
or "go tool -n compile" now run in about 0.01s instead of 0.1s,
because they no longer run gcc 4 times each.
go test -short cmd/go drops from 81s to 44s (and needs more trimming).
The $GOROOT/test suite drops from 92s to 33s, because the number of
gcc invocation drops from 13,336 to 0.
Overall, all.bash drops from 5m53s to 4m07s wall time.
Change-Id: Ia85abc89e1e2bb126b933aff3bf7c5f6c0984cd5
Reviewed-on: https://go-review.googlesource.com/73850
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Artyom Pervukhin [Sun, 22 Oct 2017 10:33:06 +0000 (13:33 +0300)]
image/draw, image/color: optimize hot path sqDiff function
Function sqDiff is called multiple times in the hot path (x, y loops) of
drawPaletted from the image/draw package; number of sqDiff calls is
between 4×width×height and 4×width×height×len(palette) for each
drawPaletted call.
Simplify this function by removing arguments comparison and relying
instead on signed to unsigned integer conversion rules and properties of
unsigned integer values operations guaranteed by the spec:
> For unsigned integer values, the operations +, -, *, and << are
> computed modulo 2n, where n is the bit width of the unsigned integer's
> type. Loosely speaking, these unsigned integer operations discard high
> bits upon overflow, and programs may rely on ``wrap around''.
image/gif package benchmark that depends on the code updated shows
throughput improvements:
name old time/op new time/op delta
QuantizedEncode-4 788ms ± 2% 468ms ± 9% -40.58% (p=0.000 n=9+10)
name old speed new speed delta
QuantizedEncode-4 1.56MB/s ± 2% 2.63MB/s ± 8% +68.47% (p=0.000 n=9+10)
Closes #22375.
Change-Id: Ic9a540e39ceb21e7741d308af1cfbe61b4ac347b
Reviewed-on: https://go-review.googlesource.com/72373 Reviewed-by: Nigel Tao <nigeltao@golang.org>
isharipo [Fri, 13 Oct 2017 12:37:16 +0000 (15:37 +0300)]
obj/x86: re-organize code to make codegen easier
1. Move AXXX constants (A-enumeration) from "a.out.go" to "aenum.go"
2. Move VEX-encoded optabs from "asm6.go" to "vex_optabs.go"
Also run "go generate" over aenum.go. This explains diff in "anames.go".
Initialization of opindex is split into 2 loops:
one for `vexOptab`, second for `optab`.
Rationale:
when VEX instructions are generated with current structure,
asm6.go is modified, which can lead to merge conflicts and
larger diffs than desired. Same for a.out.go.
This change makes x86avxgen usage possible:
https://go-review.googlesource.com/c/arch/+/66972
David Crawshaw [Sat, 21 Oct 2017 11:29:46 +0000 (07:29 -0400)]
cmd/link, plugin: always encode path
Both the linker and the plugin package were inconsistent
about when they applied the path encoding defined in
objabi.PathToPrefix. As a result, only some symbols from
a package path that required encoding were being found.
So always encoding the path.
Fixes #22295
Change-Id: Ife86c79ca20b2e9307008ed83885e193d32b7dc4
Reviewed-on: https://go-review.googlesource.com/72390
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ben Shi [Fri, 20 Oct 2017 03:50:15 +0000 (03:50 +0000)]
cmd/compile: optimize MOVBS/MOVBU/MOVHS/MOVHU on ARMv6 and ARMv7
MOVBS/MOVBU/MOVHS/MOVHU can be optimized with a single instruction
on ARMv6 and ARMv7, instead of a pair of left/right shifts.
The benchmark tests show big improvement in special cases and a little
improvement in total.
1. A special case gets about 29% improvement.
name old time/op new time/op delta
TypePro-4 3.81ms ± 1% 2.71ms ± 1% -28.97% (p=0.000 n=26+25)
The source code of this case can be found at
https://github.com/benshi001/ugo1/blob/master/typepromotion_test.go
griesemer [Wed, 25 Oct 2017 23:11:51 +0000 (16:11 -0700)]
test: add test cases for method expressions with literal receiver types
For #9060.
Change-Id: Ibd0f047083f3c98cec96c655a3e2024df8e1d2a0
Reviewed-on: https://go-review.googlesource.com/73554 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Artyom Pervukhin [Sat, 21 Oct 2017 06:36:54 +0000 (09:36 +0300)]
image/draw: reduce drawPaletted allocations for special source cases
drawPaletted has to discover R,G,B,A color values of each source image
pixel in a given rectangle. Doing that by calling image.Image.At()
method returning color.Color interface is quite taxing allocation-wise
since interface values go through heap. Introduce special cases for some
concrete source types by fetching color values using type-specific
methods.
name old time/op new time/op delta
Paletted-4 7.62ms ± 4% 3.72ms ± 3% -51.20% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Paletted-4 480kB ± 0% 0kB ± 0% -99.99% (p=0.000 n=4+5)
name old allocs/op new allocs/op delta
Paletted-4 120k ± 0% 0k ± 0% -100.00% (p=0.008 n=5+5)
Updates #15759.
Change-Id: I0ce1770ff600ac80599541aaad4c2c826855c8fb
Reviewed-on: https://go-review.googlesource.com/72370 Reviewed-by: Nigel Tao <nigeltao@golang.org>
griesemer [Wed, 25 Oct 2017 18:26:02 +0000 (11:26 -0700)]
spec: match syntax for method expressions with implementations
A method expression is of the form T.m where T is a type and m
is a method of that type. The spec restricted T essentially to
a type name. Both cmd/compile and go/types accepted any type
syntactically, and a method expression was really just a form
of a selector expression x.f where x denotes a type.
This CL removes the spec syntax restriction from MethodExpr
to match the actual implementation. It also moves MethodExpr
from Operand to PrimaryExpr, because that's what it is.
It still keeps the separate notion of MethodExpr even though
it looks just like a selector expresion, since a MethodExpr
must start with a type rather than a value, and the spec's
syntax expresses this bit of semantics via distinct productions
(e.g., conversions look like calls but also must start with
a type).
Fixes #9060.
Change-Id: Idd84655b5b4f85d7ee53ebf749f73f0414a05f4a
Reviewed-on: https://go-review.googlesource.com/73233 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Joe Tsai [Mon, 23 Oct 2017 20:47:15 +0000 (13:47 -0700)]
archive/zip: restrict UTF-8 detection for comment and name fields
CL 39570 added support for automatically setting flag bit 11 to
indicate that the filename and comment fields are encoded in UTF-8,
which is (conventionally) the encoding using for most Go strings.
However, the detection added is too lose for two reasons:
* We need to ensure both fields are at least possibly UTF-8.
That is, if any field is definitely not UTF-8, then we can't set the bit.
* The utf8.ValidRune returns true for utf8.RuneError, which iterating
over a Go string automatically returns for invalid UTF-8.
Thus, we manually check for that value.
Updates #22367
Updates #10741
Change-Id: Ie8aae388432e546e44c6bebd06a00434373ca99e
Reviewed-on: https://go-review.googlesource.com/72791 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adam Thomason [Wed, 25 Oct 2017 21:36:23 +0000 (14:36 -0700)]
cmd/go: add -shift to go vet's flag whitelist
CL 40112 intended to allow full flag processing in go vet, but missed
vet's -shift flag; this corrects the omission.
Fixes #22442
Change-Id: I47525018306bd8b9aa452fb378d0d45319f8cf11
Reviewed-on: https://go-review.googlesource.com/73553
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Fraenkel [Wed, 25 Oct 2017 16:42:14 +0000 (12:42 -0400)]
io: flatten MultiWriter writers
Replace any nested Writer that is a MultiWriter with its associated
Writers.
Fixes #22431
Change-Id: Ida7c4c83926363c1780689e216cf0c5241a5b8eb
Reviewed-on: https://go-review.googlesource.com/73470
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lynn Boger [Wed, 25 Oct 2017 19:34:08 +0000 (15:34 -0400)]
cmd/go: enable TestNoteReading on ppc64le
This test was previously disabled when external linking was
unsupported on ppc64le. It should still be disabled on ppc64
since there is no cgo or external linking there, but I removed
the if test for GOARCH=ppc64 since the initial test for cgo
enabled will cause it to be skipped on ppc64.
Fixes #22360
Change-Id: I5a0e3e4a1bd71ac7bf0ed0c792f7b78fb4a5e100
Reviewed-on: https://go-review.googlesource.com/73510
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Henry [Wed, 18 Oct 2017 13:30:08 +0000 (15:30 +0200)]
net/smtp: added Noop to Client
This adds a Noop() function to the net/stmp client.
It allows for testing if a connaction is still healthy.
Fixes #22321
Change-Id: I023b613b1536ea21274cc36d41f5720c9bbdecbc
Reviewed-on: https://go-review.googlesource.com/71650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lorenz Bauer [Wed, 18 Oct 2017 10:15:04 +0000 (11:15 +0100)]
net: make {TCP,Unix}Listener implement syscall.Conn
This change adds the syscall.Conn interface to Listener types, with the caveat that only RawConn.Control is supported. Custom socket options can now be set safely.
Updates #19435
Fixes #22065
Change-Id: I7e74780d00318dc54a923d1c628a18a36009acab
Reviewed-on: https://go-review.googlesource.com/71651
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Theophanes [Wed, 25 Oct 2017 18:01:46 +0000 (11:01 -0700)]
database/sql: scan into *time.Time without reflection
Previously scanning time.Time into a *time.Time required reflection.
Now it does not. Scanning already checked if the source value was of
type time.Time. The only addition was checking the destination was
of type *time.Time.
Existing tests already scan time.Time into *time.Time, so no new
tests were added. Linked issue has performance justification.
Fixes #22300
Change-Id: I4eea461c78fad71ce76e7677c8503a1919666931
Reviewed-on: https://go-review.googlesource.com/73232
Run-TryBot: Daniel Theophanes <kardianos@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Theophanes [Tue, 17 Oct 2017 22:59:56 +0000 (15:59 -0700)]
database/sql: ensure all driver interfaces are called under single lock
Russ pointed out in a previous CL golang.org/cl/65731 that not only
was the locking incomplete, previous changes did not correctly
lock driver calls in other sections. After inspecting
driverConn, driverStmt, driverResult, Tx, and Rows structs
where driver interfaces are stored, I discovered a few more places
that failed to lock driver calls. The largest of these
was the parameter type converter "driverArgs".
driverArgs was typically called right before another call to the
driver in a locked region, so I made the entire driverArgs expect
a locked driver mutex and combined the region. This should not
be a problem because the connection is pulled out of the connection
pool either way so there shouldn't be contention.
Russ Cox [Wed, 25 Oct 2017 15:13:23 +0000 (11:13 -0400)]
runtime: avoid monotonic time zero on systems with low-res timers
Otherwise low-res timers cause problems at call sites that expect to
be able to use 0 as meaning "no time set" and therefore expect that
nanotime never returns 0 itself. For example, sched.lastpoll == 0
means no last poll.
Jason Wangsadinata [Tue, 24 Oct 2017 19:26:46 +0000 (02:26 +0700)]
container/ring: add examples for various Ring functions
Change-Id: I6b16cfcbe824f999c9b64df539deea567e327db7
Reviewed-on: https://go-review.googlesource.com/73090 Reviewed-by: Gabriel Aszalos <gabriel.aszalos@gmail.com> Reviewed-by: Jason Wangsadinata <jwangsadinata@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Gabriel Aszalos <gabriel.aszalos@gmail.com>
Russ Cox [Wed, 25 Oct 2017 01:14:39 +0000 (21:14 -0400)]
cmd/link: only adjust pagezero for iOS on darwin/arm64
The new pagezero_size introduced by CL 72730 breaks
on 32-bit systems, since it is 2³². Restrict the change to
darwin/arm64, since it is intended for iOS only.
We could plausibly allow GOARCH=amd64 as well, but
without a compelling reason, changing the zero page size
doesn't seem worth the risk.
Change-Id: I5d6adcbaff8d0e5b169ff13512f188332cc7ed9a
Reviewed-on: https://go-review.googlesource.com/73250
Run-TryBot: Russ Cox <rsc@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
Joe Tsai [Mon, 23 Oct 2017 21:43:51 +0000 (14:43 -0700)]
encoding/csv: forbid certain Comma and Comment runes
The record delimiter (not configurable by user) is "\r\n" or "\n".
It is insensible for the user to set Comma or Comment delimiters
to be some character that conflicts with the record delimiter.
Furthermore, it is insensible for Comma or Comment to be the same rune.
Allowing this leaks implementation details to the user in regards to
the evaluation order of which rune is checked for first.
Fixes #22404
Change-Id: I31e86abc9b3a8fb4584e090477795587740970ae
Reviewed-on: https://go-review.googlesource.com/72793 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Russ Cox [Tue, 24 Oct 2017 01:57:54 +0000 (21:57 -0400)]
build: move final steps of make.bash, make.bat, make.rc into cmd/dist
This CL expands the job of "dist bootstrap" to be "finish make.bash".
I need to change that logic in upcoming CLs related to cmd/go
changes, and I'd rather not change it in three places in three different
shell script languages.
Change-Id: I545dc215e408289e4d0b28f7c2ffcd849d89ad3b
Reviewed-on: https://go-review.googlesource.com/72870 Reviewed-by: David Crawshaw <crawshaw@golang.org>