Dhananjay Nakrani [Sun, 30 Oct 2016 19:47:53 +0000 (12:47 -0700)]
cmd/compile: initialize Decldepth in all cases
Previously, on encountering Func.Nname.Type == nil, typecheckfunc()
returned without initializing Decldepth for that func. This causes
typecheckclosure() to fatal. This change ensures that we initialize
Decldepth in all cases.
Keith Randall [Mon, 31 Oct 2016 04:10:03 +0000 (21:10 -0700)]
cmd/compile: make [0]T and [1]T SSAable types
We used to have to keep on-stack copies of these types.
Now they can be registerized.
[0]T is kind of trivial but might as well handle it.
This change enables another change I'm working on to improve how x.(T)
expressions are handled (#17405). This CL helps because now all
types that are direct interface types are registerizeable (e.g. [1]*byte).
No higher-degree arrays for now because non-constant indexes are hard.
Update #17405
Change-Id: I2399940965d17b3969ae66f6fe447a8cefdd6edd
Reviewed-on: https://go-review.googlesource.com/32416
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
David Chase [Fri, 28 Oct 2016 17:33:57 +0000 (13:33 -0400)]
cmd/compile: mark temps with new AutoTemp flag, and use it.
This is an extension of
https://go-review.googlesource.com/c/31662/
to mark all the temporaries, not just the ssa-generated ones.
Before-and-after ls -l `go tool -n compile` shows a 3%
reduction in size (or rather, a prior 3% inflation for
failing to filter temps out properly.)
Replaced name-dependent "is it a temp?" tests with calls to
*Node.IsAutoTmp(), which depends on AutoTemp. Also replace
calls to istemp(n) with n.IsAutoTmp(), to reduce duplication
and clean up function name space. Generated temporaries
now come with a "." prefix to avoid (apparently harmless)
clashes with legal Go variable names.
Fixes #17644.
Fixes #17240.
Change-Id: If1417f29c79a7275d7303ddf859b51472890fd43
Reviewed-on: https://go-review.googlesource.com/32255
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Mon, 31 Oct 2016 19:01:32 +0000 (12:01 -0700)]
cmd/compile: remove legacy debug flags
-M, -P, and -R were for debugging backend passes that no longer
exists.
-g is used for debugging instructions generated with Gins, but the SSA
backend mostly generates instructions directly. The handful of
instructions still generated with Gins are pretty useless for
debugging.
-x was used to debug the old lexer, but now it only causes us to print
file names as they're parsed, and only if we manually hack the
compiler to enable tracing.
Mike Strosaker [Fri, 28 Oct 2016 23:50:16 +0000 (19:50 -0400)]
crypto/sha256: improve performance for sha256.block on ppc64le
Adds an assembly implementation of sha256.block for ppc64le to improve its
performance. This implementation is largely based on the original amd64
implementation, which unrolls the 64 iterations of the inner loop.
Fixes #17652
benchmark old ns/op new ns/op delta
BenchmarkHash8Bytes 1263 767 -39.27%
BenchmarkHash1K 14048 7766 -44.72%
BenchmarkHash8K 102245 55626 -45.60%
benchmark old MB/s new MB/s speedup
BenchmarkHash8Bytes 6.33 10.43 1.65x
BenchmarkHash1K 72.89 131.85 1.81x
BenchmarkHash8K 80.12 147.27 1.84x
Change-Id: Ib4adf429423b20495580400be10bd7e171bcc70b
Reviewed-on: https://go-review.googlesource.com/32318 Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mike Strosaker [Sat, 29 Oct 2016 06:36:41 +0000 (02:36 -0400)]
crypto/sha512: improve performance for sha512.block on ppc64le
Adds an assembly implementation of sha512.block for ppc64le to improve its
performance. This implementation is largely based on the original amd64
implementation, unrolling the 80 iterations of the inner loop.
Fixes #17660
benchmark old ns/op new ns/op delta
BenchmarkHash8Bytes 1715 1133 -33.94%
BenchmarkHash1K 10098 5513 -45.41%
BenchmarkHash8K 68004 35278 -48.12%
benchmark old MB/s new MB/s speedup
BenchmarkHash8Bytes 4.66 7.06 1.52x
BenchmarkHash1K 101.40 185.72 1.83x
BenchmarkHash8K 120.46 232.21 1.93x
Change-Id: Ifd55a49a24cb159b3a09a8e928c3f37727aca103
Reviewed-on: https://go-review.googlesource.com/32320 Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
David Crawshaw [Mon, 31 Oct 2016 01:19:59 +0000 (21:19 -0400)]
runtime: make module typemaps visible to the GC
The map[typeOff]*_type object is created at run time and stored in
the moduledata. The moduledata object is marked by the linker as
SNOPTRDATA, so the reference is ignored by the GC. Running
misc/cgo/testplugin/test.bash with GOGC=1 will eventually collect
the typemap and crash.
This bug probably comes up in -linkshared binaries in Go 1.7.
I don't know why we haven't seen a report about this yet.
Fixes #17680
Change-Id: I0e9b5c006010e8edd51d9471651620ba665248d3
Reviewed-on: https://go-review.googlesource.com/32430
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
David Crawshaw [Sun, 30 Oct 2016 19:31:21 +0000 (15:31 -0400)]
cmd/link, plugin: use full plugin path for symbols
Plumb the import path of a plugin package through to the linker, and
use it as the prefix on the exported symbol names.
Before this we used the basename of the plugin file as the prefix,
which could conflict and result in multiple loaded plugins sharing
symbols that are distinct.
Fixes #17155
Fixes #17579
Change-Id: I7ce966ca82d04e8507c0bcb8ea4ad946809b1ef5
Reviewed-on: https://go-review.googlesource.com/32355 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Gudger [Sat, 29 Oct 2016 03:09:02 +0000 (20:09 -0700)]
syscall: remove X__cmsg_data from Cmsghdr
This field is a zero length array and has little use. Since Go 1.5, trailing
zero-length arrays take up space. Both syscall.UnixRights() and
syscall.ParseSocketControlMessage() depend on being able to do an unsafe cast
of socket control message data to Cmsghdr this is only safe if the socket
control message data is greater than or equal to the size of Cmsghdr. Since
control message data that is equal in size to Cmsghdr without X__cmsg_data is
a valid socket control message, we must remove X__cmsg_data or not perform the
unsafe cast.
Removing X__cmsg_data will prevent Go code that uses X__cmsg_data from
compiling, but removing the unsafe cast will cause Go code that uses
X__cmsg_data to fail or exhibit undefined behavior at runtime. It was
therefore decided that removing X__cmsg_data was the better option.
Daniel Theophanes [Mon, 17 Oct 2016 06:11:55 +0000 (23:11 -0700)]
database/sql: add context helper methods and transaction types
Prior to this change, it was implied that transaction properties
would be carried in the context value. However, no such properties
were defined, not even common ones. Define two common properties:
isolation level and read-only. Drivers may choose to support
additional transaction properties. It is not expected any
further transaction properties will be added in the future.
Dmitry Vyukov [Fri, 28 Oct 2016 15:12:39 +0000 (17:12 +0200)]
runtime/race: update race runtime
This updates the runtime to HEAD to keep it aligned and fixes some bugs.
http://llvm.org/viewvc/llvm-project?view=revision&revision=285454
fixes the crash on darwin related to unaligned data section (#17065).
http://llvm.org/viewvc/llvm-project?view=revision&revision=285451
enables core dumps by default (#16527).
http://llvm.org/viewvc/llvm-project?view=revision&revision=285455
adds a hook to obtain number of races reported so far (#15972).
Can now be obtained with:
//go:nosplit
func RaceReportCount() int {
var n uint64
racecall(&__tsan_report_count, uintptr(unsafe.Pointer(&n)), 0, 0, 0)
return int(n)
}
Fixes #16527.
Fixes #17065.
Update #15972.
Change-Id: I8f869cb6275c9521a47303f3810a9965e9314357
Reviewed-on: https://go-review.googlesource.com/32160
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Joe Tsai [Sat, 29 Oct 2016 19:25:20 +0000 (12:25 -0700)]
compress/gzip: only encode MTIME if it is valid
The GZIP format records the ModTime as an uint32 counting seconds since
the Unix epoch. The zero value is explicitly defined in section 2.3.1
as meaning no timestamp is available.
Currently, the Writer always encodes the ModTime even if it is the zero
time.Time value, which causes the Writer to try and encode the value
-62135596800 into the uint32 MTIME field. This causes an overflow and
results in our GZIP files having MTIME fields indicating a date in 2042-07-13.
We alter the Writer to only encode ModTime if the value does not underflow
the MTIME field (i.e., it is newer than the Unix epoch). We do not attempt
to fix what happens when the timestamp overflows in the year 2106.
We alter the Reader to only decode ModTime if the value is non-zero.
There is no risk of overflowing time.Time when decoding.
Russ Cox [Tue, 25 Oct 2016 12:59:35 +0000 (08:59 -0400)]
cmd/link: fix -X importpath.name=value when import path needs escaping
After the final slash, dots are %-escaped when constructing a symbol name,
so that in the actual symbol table, the import path githost.com/my.git
becomes githost.com/my%2egit. In this case, -X githost.com/my.git.Value=foo
needs to set githost.com/my%2egit.Value. This is a detail of the object format
and not something users should know or depend on, so apply the escaping
as needed.
People who have run across this already and figured out and started using
the escaped forms with -X will find those forms not working anymore.
That is, -X githost.com/my%2egit.Value=foo is the Go 1.7 workaround but
will stop working in Go 1.8 once this proper fix is in place.
People who need to keep scripts working with older and newer versions of Go
can safely pass both forms, and one will be ignored:
Zev Goldstein [Fri, 28 Oct 2016 15:42:27 +0000 (11:42 -0400)]
path/filepath: fix Abs on Windows
The filepath.Abs function in windows did not call Clean as the
documentation claimed. This change not only fixes that behavior but
also adjusts TestAbs to verify Abs calls Clean as documented.
Fixes #17210
Change-Id: I20c5f5026042fd7bd9d929ff5b17c8b2653f8afe
Reviewed-on: https://go-review.googlesource.com/32292 Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
* Before:
./issue17631.go:20: unknown struct { about string; before map[string]uint;
update map[string]int; updateTime time.Time; expect map[string]int } field
'updates' in struct literal
* After:
./issue17631.go:20: unknown field 'updates' in struct literal of type { about string;
before map[string]uint; update map[string]int; updateTime time.Time;
expect map[string]int }
Dmitry Vyukov [Fri, 28 Oct 2016 22:53:49 +0000 (00:53 +0200)]
runtime/race: ignore user GORACE env var in tests
I did 'export GORACE=atexit_sleep_ms=0' in a console
and then was puzzled as to why race tests fail.
Existing GORACE env var may (or may not) override
the one that we setup.
Filter out GORACE as we do for other important env vars.
Than McIntosh [Fri, 28 Oct 2016 17:28:56 +0000 (13:28 -0400)]
test: add test for gccgo issue #17640
Change-Id: Iec35f9b62982da40de400397bc456149216303dc
Reviewed-on: https://go-review.googlesource.com/32297 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Fri, 28 Oct 2016 22:25:31 +0000 (18:25 -0400)]
time: clarify Equal docs
The docs used to imply that using == would compare Locations, but of
course it just compares Location pointers, which will have unpredictable
results depending on how the pointers are loaded.
Peter Weinberger [Fri, 28 Oct 2016 19:12:18 +0000 (15:12 -0400)]
runtime: ensure elapsed cycles are not negative
On solaris/amd64 sometimes the reported cycle count is negative. Replace
with 0.
Change-Id: I364eea5ca072281245c7ab3afb0bf69adc3a8eae
Reviewed-on: https://go-review.googlesource.com/32258 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Fri, 28 Oct 2016 21:18:36 +0000 (17:18 -0400)]
runtime: fix SP adjustment on amd64p32
On amd64p32, rt0_go attempts to reserve 128 bytes of scratch space on
the stack, but due to a register mixup this ends up being a no-op. Fix
this so we actually reserve the stack space.
Change-Id: I04dbfbeb44f3109528c8ec74e1136bc00d7e1faa
Reviewed-on: https://go-review.googlesource.com/32331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Sun, 23 Oct 2016 15:07:49 +0000 (11:07 -0400)]
runtime: disable stack rescanning by default
With the hybrid barrier in place, we can now disable stack rescanning
by default. This commit adds a "gcrescanstacks" GODEBUG variable that
is off by default but can be set to re-enable STW stack rescanning.
The plan is to leave this off but available in Go 1.8 for debugging
and as a fallback.
With this change, worst-case mark termination time at GOMAXPROCS=12
*not* including time spent stopping the world (which is still
unbounded) is reliably under 100 µs, with a 95%ile around 50 µs in
every benchmark I tried (the go1 benchmarks, the x/benchmarks garbage
benchmark, and the gcbench activegs and rpc benchmarks). Including
time spent stopping the world usually adds about 20 µs to total STW
time at GOMAXPROCS=12, but I've seen it add around 150 µs in these
benchmarks when a goroutine takes time to reach a safe point (see
issue #10958) or when stopping the world races with goroutine
switches. At GOMAXPROCS=1, where this isn't an issue, worst case STW
is typically 30 µs.
The go-gcbench activegs benchmark is designed to stress large numbers
of dirty stacks. This commit reduces 95%ile STW time for 500k dirty
stacks by nearly three orders of magnitude, from 150ms to 195µs.
This has little effect on the throughput of the go1 benchmarks or the
x/benchmarks benchmarks.
It reduces the tail latency of the x/benchmarks HTTP benchmark:
name old p50-time new p50-time delta
XHTTP-12 489µs ± 0% 491µs ± 1% +0.54% (p=0.000 n=20+18)
name old p95-time new p95-time delta
XHTTP-12 957µs ± 1% 960µs ± 1% +0.28% (p=0.002 n=20+17)
name old p99-time new p99-time delta
XHTTP-12 1.76ms ± 1% 1.64ms ± 1% -7.20% (p=0.000 n=20+18)
Comparing to the beginning of the hybrid barrier implementation
("runtime: parallelize STW mcache flushing") shows that the hybrid
barrier trades a small performance impact for much better STW latency,
as expected. The magnitude of the performance impact is generally
small:
Updates #17099, since you can't have a rescan list bug if there's no
rescan list. I'm not marking it as fixed, since gcrescanstacks can
still be set to re-enable the rescan lists.
Change-Id: I6e926b4c2dbd4cd56721869d4f817bdbb330b851
Reviewed-on: https://go-review.googlesource.com/31766 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Sun, 23 Oct 2016 15:03:56 +0000 (11:03 -0400)]
runtime: implement unconditional hybrid barrier
This implements the unconditional version of the hybrid deletion write
barrier, which always shades both the old and new pointer. It's
unconditional for now because barriers on channel operations require
checking both the source and destination stacks and we don't have a
way to funnel this information into the write barrier at the moment.
As part of this change, we modify the typed memclr operations
introduced earlier to invoke the write barrier.
This has basically no overall effect on benchmark performance. This is
good, since it indicates that neither the extra shade nor the new bulk
clear barriers have much effect. It also has little effect on latency.
This is expected, since we haven't yet modified mark termination to
take advantage of the hybrid barrier.
Updates #17503.
Change-Id: Iebedf84af2f0e857bd5d3a2d525f760b5cf7224b
Reviewed-on: https://go-review.googlesource.com/31765 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Wed, 26 Oct 2016 21:05:41 +0000 (17:05 -0400)]
runtime: avoid getfull() barrier most of the time
With the hybrid barrier, unless we're doing a STW GC or hit a very
rare race (~once per all.bash) that can start mark termination before
all of the work is drained, we don't need to drain the work queue at
all. Even draining an empty work queue is rather expensive since we
have to enter the getfull() barrier, so it's worth avoiding this.
Conveniently, it's quite easy to detect whether or not we actually
need the getufull() barrier: since the world is stopped when we enter
mark termination, everything must have flushed its work to the work
queue, so we can just check the queue. If the queue is empty and we
haven't queued up any jobs that may create more work (which should
always be the case with the hybrid barrier), we can simply have all GC
workers perform non-blocking drains.
Also conveniently, this solution is quite safe. If we do somehow screw
something up and there's work on the work queue, some worker will
still process it, it just may not happen in parallel.
This is not the "right" solution, but it's simple, expedient,
low-risk, and maintains compatibility with debug.gcrescanstacks. When
we remove the gcrescanstacks fallback in Go 1.9, we should also fix
the race that starts mark termination early, and then we can eliminate
work draining from mark termination.
Updates #17503.
Change-Id: I7b3cd5de6a248ab29d78c2b42aed8b7443641361
Reviewed-on: https://go-review.googlesource.com/32186 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Wed, 19 Oct 2016 19:49:31 +0000 (15:49 -0400)]
runtime: add deletion barriers on gobuf.ctxt
gobuf.ctxt is set to nil from many places in assembly code and these
assignments require write barriers with the hybrid barrier.
Conveniently, in most of these places ctxt should already be nil, in
which case we don't need the barrier. This commit changes these places
to assert that ctxt is already nil.
gogo is more complicated, since ctxt may not already be nil. For gogo,
we manually perform the write barrier if ctxt is not nil.
Austin Clements [Mon, 22 Aug 2016 20:02:54 +0000 (16:02 -0400)]
runtime: perform write barrier before pointer write
Currently, we perform write barriers after performing pointer writes.
At the moment, it simply doesn't matter what order this happens in, as
long as they appear atomic to GC. But both the hybrid barrier and ROC
are going to require a pre-write write barrier.
For the hybrid barrier, this is important because the barrier needs to
observe both the current value of the slot and the value that will be
written to it. (Alternatively, the caller could do the write and pass
in the old value, but it seems easier and more useful to just swap the
order of the barrier and the write.)
For ROC, this is necessary because, if the pointer write is going to
make the pointer reachable to some goroutine that it currently is not
visible to, the garbage collector must take some special action before
that pointer becomes more broadly visible.
This commits swaps pointer writes around so the write barrier occurs
before the pointer write.
The main subtlety here is bulk memory writes. Currently, these copy to
the destination first and then use the pointer bitmap of the
destination to find the copied pointers and invoke the write barrier.
This is necessary because the source may not have a pointer bitmap. To
handle these, we pass both the source and the destination to the bulk
memory barrier, which uses the pointer bitmap of the destination, but
reads the pointer values from the source.
Updates #17503.
Change-Id: I78ecc0c5c94ee81c29019c305b3d232069294a55
Reviewed-on: https://go-review.googlesource.com/31763 Reviewed-by: Rick Hudson <rlh@golang.org>
Cherry Zhang [Thu, 27 Oct 2016 18:33:58 +0000 (14:33 -0400)]
cmd/link: put text at address 0x1000000 on darwin/amd64
Apparently on macOS Sierra LLDB thinks /usr/lib/dyld is mapped
at address 0, even if Go code starts at 0x1000, and it looks up
addresses from dyld which shadows Go symbols. Move Go binary at
a higher address to avoid clash.
Austin Clements [Tue, 18 Oct 2016 14:26:28 +0000 (10:26 -0400)]
cmd/compile: disable various write barrier optimizations
Several of our current write barrier elision optimizations are invalid
with the hybrid barrier. Eliding the hybrid barrier requires that
*both* the current and new pointer be already shaded and, since we
don't have the flow analysis to figure out anything about the slot's
current value, for now we have to just disable several of these
optimizations.
This has a slight impact on binary size. On linux/amd64, the go tool
binary increases by 0.7% and the compile binary increases by 1.5%.
It also has a slight impact on performance, as one would expect. We'll
win some of this back in subsequent commits.
Austin Clements [Wed, 19 Oct 2016 20:16:40 +0000 (16:16 -0400)]
runtime: eliminate write barriers from save
As for dropg, save is writing a nil pointer that will generate a write
barrier with the hybrid barrier. However, in this case, ctxt always
should already be nil, so replace the write with an assertion that
this is the case.
At this point, we're ready to disable the write barrier elision
optimizations that interfere with the hybrid barrier.
Updates #17503.
Change-Id: I83208e65aa33403d442401f355b2e013ab9a50e9
Reviewed-on: https://go-review.googlesource.com/31571 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Wed, 19 Oct 2016 20:00:07 +0000 (16:00 -0400)]
runtime: eliminate write barriers from dropg
Currently this contains no write barriers because it's writing nil
pointers, but with the hybrid barrier, even these will produce write
barriers. However, since these are *gs and *ms, they don't need write
barriers, so we can simply eliminate them.
Updates #17503.
Change-Id: Ib188a60492c5cfb352814bf9b2bcb2941fb7d6c0
Reviewed-on: https://go-review.googlesource.com/31570 Reviewed-by: Rick Hudson <rlh@golang.org>
The hybrid barrier requires allocate-black, but there's one case where
we don't currently allocate black: the tiny allocator. If we allocate
a *new* tiny alloc block during GC, it will be allocated black, but if
we allocated the current block before GC, it won't be black, and the
further allocations from it won't mark it, which means we may free a
reachable tiny block during sweeping.
Fix this by passing over all mcaches at the beginning of mark, while
the world is still stopped, and greying their tiny blocks.
Updates #17503.
Change-Id: I04d4df7cc2f553f8f7b1e4cb0b52e2946588111a
Reviewed-on: https://go-review.googlesource.com/31456 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Thu, 13 Oct 2016 19:34:56 +0000 (15:34 -0400)]
runtime: shade stack-to-stack copy when starting a goroutine
The hybrid barrier requires barriers on stack-to-stack copies if
either stack is grey. There are only two instances of this in the
runtime: channel sends and starting a goroutine. Channel sends already
use typedmemmove and hence have the necessary barriers. This commits
adds barriers for the stack-to-stack copy when starting a goroutine.
Updates #17503.
Change-Id: Ibb55e08127ca4d021ac54be61cb96732efa5df5b
Reviewed-on: https://go-review.googlesource.com/31455 Reviewed-by: Rick Hudson <rlh@golang.org>
Michael Matloob [Fri, 28 Oct 2016 19:28:18 +0000 (15:28 -0400)]
runtime/pprof: write profiles in protobuf format.
Original Change by Daria Kolistratova <daria.kolistratova@intel.com>
Added functions with suffix proto and stuff from pprof tool to translate
to protobuf. Done as the profile proto is more extensible than the legacy
pprof format and is pprof's preferred profile format. Large part was taken
from https://github.com/google/pprof tool. Tested by hand and compared the
result with translated by pprof tool, profiles are identical.
Fixes #16093
Change-Id: I2751345b09a66ee2b6aa64be76cba4cd1c326aa6
Reviewed-on: https://go-review.googlesource.com/32257
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Austin Clements [Wed, 26 Oct 2016 18:34:20 +0000 (14:34 -0400)]
runtime: zero-initialize LR on new stacks
Currently we initialize LR on a new stack by writing nil to it. But
this is an initializing write since the newly allocated stack is not
zeroed, so this is unsafe with the hybrid barrier. Change this is a
uintptr write to avoid a bad write barrier.
Updates #17503.
Change-Id: I062ac352e35df7da4644c1f2a5aaab87049d1f60
Reviewed-on: https://go-review.googlesource.com/32093 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Fri, 14 Oct 2016 17:39:07 +0000 (13:39 -0400)]
runtime: ensure finalizers are zero-initialized before reuse
We reuse finalizers in finblocks, which are allocated off-heap. This
means they have to be zero-initialized before becoming visible to the
garbage collector. We actually already do this by clearing the
finalizer before returning it to the pool, but we're not careful to
enforce correct memory ordering. Fix this by manipulating the
finalizer count atomically so these writes synchronize properly with
the garbage collector.
Updates #17503.
Change-Id: I7797d31df3c656c9fe654bc6da287f66a9e2037d
Reviewed-on: https://go-review.googlesource.com/31454 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Mon, 3 Oct 2016 18:45:52 +0000 (14:45 -0400)]
runtime: avoid write barriers to uninitialized finalizer frame memory
runfinq allocates a stack frame on the heap for constructing the
finalizer function calls and reuses it for each call. However, because
the type of this frame is constantly shifting, it tells mallocgc there
are no pointers in it and it acts essentially like uninitialized
memory between uses. But runfinq uses pointer writes with write
barriers to "initialize" this memory, which is not going to be safe
with the hybrid barrier, since the hybrid barrier may see a stale
pointer left in the "uninitialized" frame.
Fix this by zero-initializing the argument values in the frame before
writing the argument pointers.
Updates #17503.
Change-Id: I951c0a2be427eb9082a32d65c4410e6fdef041be
Reviewed-on: https://go-review.googlesource.com/31453 Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Tue, 18 Oct 2016 15:06:28 +0000 (11:06 -0400)]
cmd/compile: use typedmemclr for zeroing if there are pointers
Currently, zeroing generates an ssa.OpZero, which never has write
barriers, even if the assignment is an OASWB. The hybrid barrier
requires write barriers on zeroing, so change OASWB to generate an
ssa.OpZeroWB when assigning the zero value, which turns into a
typedmemclr.
Since barrier-less memclr is only safe in very narrow circumstances,
this commit renames memclr to avoid accidentally calling memclr on
typed memory. This can cause subtle, non-deterministic bugs, so it's
worth some effort to prevent. In the near term, this will also prevent
bugs creeping in from any concurrent CLs that add calls to memclr; if
this happens, whichever patch hits master second will fail to compile.
This also adds the other new memclr variants to the compiler's
builtin.go to minimize the churn on that binary blob. We'll use these
in future commits.
Updates #17503.
Change-Id: I00eead049f5bd35ca107ea525966831f3d1ed9ca
Reviewed-on: https://go-review.googlesource.com/31369 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
Currently fixalloc does not zero memory it reuses. This is dangerous
with the hybrid barrier if the type may contain heap pointers, since
it may cause us to observe a dead heap pointer on reuse. It's also
error-prone since it's the only allocator that doesn't zero on
allocation (mallocgc of course zeroes, but so do persistentalloc and
sysAlloc). It's also largely pointless: for mcache, the caller
immediately memclrs the allocation; and the two specials types are
tiny so there's no real cost to zeroing them.
Change fixalloc to zero allocations by default.
The only type we don't zero by default is mspan. This actually
requires that the spsn's sweepgen survive across freeing and
reallocating a span. If we were to zero it, the following race would
be possible:
1. The current sweepgen is 2. Span s is on the unswept list.
2. Direct sweeping sweeps span s, finds it's all free, and releases s
to the fixalloc.
3. Thread 1 allocates s from fixalloc. Suppose this zeros s, including
s.sweepgen.
4. Thread 1 calls s.init, which sets s.state to _MSpanDead.
5. On thread 2, background sweeping comes across span s in allspans
and cas's s.sweepgen from 0 (sg-2) to 1 (sg-1). Now it thinks it
owns it for sweeping. 6. Thread 1 continues initializing s.
Everything breaks.
I would like to fix this because it's obviously confusing, but it's a
subtle enough problem that I'm leaving it alone for now. The solution
may be to skip sweepgen 0, but then we have to think about wrap-around
much more carefully.
Updates #17503.
Change-Id: Ie08691feed3abbb06a31381b94beb0a2e36a0613
Reviewed-on: https://go-review.googlesource.com/31368 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Tue, 18 Oct 2016 21:29:37 +0000 (17:29 -0400)]
runtime: make _MSpanDead be the zero state
Currently the zero value of an mspan is in the "in use" state. This
seems like a bad idea in general. But it's going to wreak havoc when
we make fixalloc zero allocations: even "freed" mspan objects are
still on the allspans list and still get looked at by the garbage
collector. Hence, if we leave the mspan states the way they are,
allocating a span that reuses old memory will temporarily pass that
span (which is visible to GC!) through the "in use" state, which can
cause "unswept span" panics.
Fix all of this by making the zero state "dead".
Updates #17503.
Change-Id: I77c7ac06e297af4b9e6258bc091c37abe102acc3
Reviewed-on: https://go-review.googlesource.com/31367 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Mon, 17 Oct 2016 21:00:05 +0000 (17:00 -0400)]
runtime: use typedmemclr for typed memory
The hybrid barrier requires distinguishing typed and untyped memory
even when zeroing because the *current* contents of the memory matters
even when overwriting.
This commit introduces runtime.typedmemclr and runtime.memclrHasPointers
as a typed memory clearing functions parallel to runtime.typedmemmove.
Currently these simply call memclr, but with the hybrid barrier we'll
need to shade any pointers we're overwriting. These will provide us
with the necessary hooks to do so.
Updates #17503.
Change-Id: I74478619f8907825898092aaa204d6e4690f27e6
Reviewed-on: https://go-review.googlesource.com/31366 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
Austin Clements [Tue, 25 Oct 2016 17:56:37 +0000 (13:56 -0400)]
runtime: parallelize STW mcache flushing
Currently all mcaches are flushed in a single STW root job. This takes
about 5 µs per P, but since it's done sequentially it adds about
5*GOMAXPROCS µs to the STW.
Fix this by parallelizing the job. Since there are exactly GOMAXPROCS
mcaches to flush, this parallelizes quite nicely and brings the STW
latency cost down to a constant 5 µs (assuming GOMAXPROCS actually
reflects the number of CPUs).
Updates #17503.
Change-Id: Ibefeb1c2229975d5137c6e67fac3b6c92103742d
Reviewed-on: https://go-review.googlesource.com/32033 Reviewed-by: Rick Hudson <rlh@golang.org>
cmd/compile: don't alloc Name/Param for unresolved syms
ONONAME nodes generated from unresolved symbols don't need Params.
They only need Names to store Iota; move Iota to Node.Xoffset.
While we're here, change iota to int64 to reduce casting.
unknown [Fri, 7 Oct 2016 15:21:03 +0000 (18:21 +0300)]
runtime/pprof: write profiles in protobuf format.
Added functions with suffix proto and stuff from pprof tool to translate
to protobuf. Done as the profile proto is more extensible than the legacy
pprof format and is pprof's preferred profile format. Large part was taken
from https://github.com/google/pprof tool. Tested by hand and compared the
result with translated by pprof tool, profiles are identical.
Fixes #16093
Change-Id: I5acdb2809cab0d16ed4694fdaa7b8ddfd68df11e
Reviewed-on: https://go-review.googlesource.com/30556
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Austin Clements [Fri, 28 Oct 2016 15:14:07 +0000 (11:14 -0400)]
runtime: fix preemption of root marking jobs
The current logic in gcDrain conflates non-blocking with preemptible
draining for root jobs. As a result, if you do a non-blocking (but
*not* preemptible) drain, like dedicated workers do, the root job
drain will stop if preempted and fall through to heap marking jobs,
which won't stop until it fails to get a heap marking job.
This commit fixes the condition on root marking jobs so they only stop
when preempted if the drain is preemptible.
Coincidentally, this also fixes a nil pointer dereference if we call
gcDrain with gcDrainNoBlock and without a user G, since it tries to
get the preempt flag from the nil user G. This combination never
happens right now, but will in the future.
Austin Clements [Fri, 28 Oct 2016 14:53:41 +0000 (10:53 -0400)]
runtime/trace, internal/trace: script to collect canned traces
This adds support to the runtime/trace test for saving traces
collected by its tests to disk and a script in internal/trace that
uses this to collect canned traces for the trace test suite. This can
be used to add to the test suite when we introduce a new trace format
version.
Joe Tsai [Thu, 20 Oct 2016 10:16:22 +0000 (03:16 -0700)]
bytes, strings: optimize for ASCII sets
In a large codebase within Google, there are thousands of uses of:
ContainsAny|IndexAny|LastIndexAny|Trim|TrimLeft|TrimRight
An analysis of their usage shows that over 97% of them only use character
sets consisting of only ASCII symbols.
Uses of ContainsAny|IndexAny|LastIndexAny:
6% are 1 character (e.g., "\n" or " ")
58% are 2-4 characters (e.g., "<>" or "\r\n\t ")
24% are 5-9 characters (e.g., "()[]*^$")
10% are 10+ characters (e.g., "+-=&|><!(){}[]^\"~*?:\\/ ")
We optimize for ASCII sets, which are commonly used to search for
"control" characters in some string. We don't optimize for the
single character scenario since IndexRune or IndexByte could be used.
Uses of Trim|TrimLeft|TrimRight:
71% are 1 character (e.g., "\n" or " ")
14% are 2 characters (e.g., "\r\n")
10% are 3-4 characters (e.g., " \t\r\n")
5% are 10+ characters (e.g., "0123456789abcdefABCDEF")
We optimize for the single character case with a simple closured function
that only checks for that character's value. We optimize for the medium
and larger sets using a 16-byte bit-map representing a set of ASCII characters.
The benchmarks below have the following suffix name "%d:%d" where the first
number is the length of the input and the second number is the length
of the charset.
The benchmarks added test the worst case. For IndexAny, that is when the
charset matches none of the input. For Trim, it is when the charset matches
all of the input.
Change-Id: I970874d101a96b33528fc99b165379abe58cf6ea
Reviewed-on: https://go-review.googlesource.com/31593
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Martin Möhrmann <martisch@uos.de>
Russ Cox [Wed, 26 Oct 2016 17:12:17 +0000 (13:12 -0400)]
html/template, text/template: drop defined template list from errors
The report in #17414 points out that if you have many many templates,
then this is an overwhelming list and just hurts the signal-to-noise ratio of the error.
Even the test of the old behavior also supports the idea that this is noise:
template: empty: "empty" is an incomplete or empty template; defined templates are: "secondary"
The chance that someone mistyped "secondary" as "empty" is slim at best.
Similarly, the compiler does not augment an error like 'unknown variable x'
by dumping the full list of all the known variables.
Josh Chorlton [Thu, 27 Oct 2016 15:10:26 +0000 (23:10 +0800)]
net/http: fix cookie Expires minimum year to 1601 instead of Epoch year 1970
Following RFC 6265 Section 5.1.1.5, ensure that the minimum
year for which an Expires value is valid and can be included in
the cookie's string, is 1601 instead of the Epoch year 1970.
A detailed specification for parsing the Expiry field is at:
https://tools.ietf.org/html/rfc6265#section-5.2.1
I stumbled across this bug due to this StackOverflow answer
that recommends setting the Expiry to the Epoch:
http://stackoverflow.com/a/5285982
Russ Cox [Wed, 26 Oct 2016 17:34:55 +0000 (13:34 -0400)]
mime/quotedprintable: accept = not followed by 2 hex digits as literal equals
This lets quotedprintable handle some inputs found in the wild,
most notably generated by "Microsoft CDO for Exchange 2000",
and it also matches how Python's quopri package handles these inputs.