Instead of eagerly creating strings like "literal 2.01" for every
lexed number in case we need to mention it in an error message, defer
this work to (*parser).syntax_error.
name old allocs/op new allocs/op delta
Template 482k ± 0% 482k ± 0% -0.12% (p=0.000 n=9+10)
GoTypes 1.35M ± 0% 1.35M ± 0% -0.04% (p=0.015 n=10+10)
Compiler 5.45M ± 0% 5.44M ± 0% -0.12% (p=0.000 n=9+8)
Change-Id: I333b3c80e583864914412fb38f8c0b7f1d8c8821
Reviewed-on: https://go-review.googlesource.com/22480
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This simplifies comparison of object files across different builds
by ensuring that the strings in the zcgo.go always appear in the
same order.
Change-Id: I3639ea4fd10e0d645b838d1bbb03cd33deca340e
Reviewed-on: https://go-review.googlesource.com/22478
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This version of the file name honors the -trimprefix flag,
which strips off variable parts like $WORK or $PWD.
The TestCgoConsistentResults test now passes.
Change-Id: If93980b054f9b13582dd314f9d082c26eaac4f41
Reviewed-on: https://go-review.googlesource.com/22444
Reviewed-by: Robert Griesemer <gri@golang.org>
Also adds TestGdbBacktrace to the runtime package.
Dwarf modifications written by Bryan Chan (@bryanpkc) who is also
at IBM and covered by the same CLA.
Fixes#14628
Change-Id: I106a1f704c3745a31f29cdadb0032e3905829850
Reviewed-on: https://go-review.googlesource.com/20193
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The comment says 'DΟ NΟT SUBMIT', and that text being in a file can cause
automated errors or warnings when trying to check the Go sources into other
source control systems.
(We reject that string in CL commit messages, which I've avoided here
by changing the O's to Ο's above.)
Change-Id: I6cdd57a8612ded5208f05a8bd6b137f44424a030
Reviewed-on: https://go-review.googlesource.com/22434
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Make sure ops have the right number of args, set
aux and auxint only if allowed, etc.
Normalize error reporting format.
Change-Id: Ie545fcc5990c8c7d62d40d9a0a55885f941eb645
Reviewed-on: https://go-review.googlesource.com/22320
Reviewed-by: David Chase <drchase@google.com>
Renames block to blockGeneric so that it can be called when the
assembly feature check fails. This means making block a var on
platforms without an assembly implementation (similar to the sha1
package).
Also adds a test to check that the fallback path works correctly
when the feature check fails.
name old speed new speed delta
Hash8Bytes 7.13MB/s ± 2% 19.89MB/s ± 1% +178.82% (p=0.000 n=9+10)
Hash1K 121MB/s ± 1% 661MB/s ± 1% +444.54% (p=0.000 n=10+9)
Hash8K 137MB/s ± 0% 918MB/s ± 1% +569.29% (p=0.000 n=10+10)
Change-Id: Id65dd6e943f14eeffe39a904dc88065fc6a60179
Reviewed-on: https://go-review.googlesource.com/22402
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The linker was incorrectly decoding type name lengths, causing
typelinks to be sorted out of order and in cases where the name was
the exact right length, linker panics.
Added a test to the reflect package that causes TestTypelinksSorted
to fail before this CL. It's not the exact failure seen in #15448
but it has the same cause: decodetype_name calculating the wrong
length.
The equivalent decoders in reflect/type.go and runtime/type.go
have the parenthesis in the right place.
Fixes#15448
Change-Id: I33257633d812b7d2091393cb9d6cc8a73e0138c8
Reviewed-on: https://go-review.googlesource.com/22403
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Here, "fix" means "replace". The new dominator computation
is the "simple" algorithm from Lengauer and Tarjan's TOPLAS
paper, with minimal changes.
Also included is a test that tweaks the fixed error.
Change-Id: I0abdf53d5d64df1e67e4e62f55e88957045cd63b
Reviewed-on: https://go-review.googlesource.com/22401
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This is not necessary for reproduceability but it removes
differences due to imported package order between compiles
using textual vs binary export format. The packages list
tends to be very short, so it's ok doing it always for now.
Guarded with a documented (const) flag so it's trivial to
disable and remove eventually.
Also, use the same flag now to enforce parameter numbering.
Change-Id: Ie05d2490df770239696ecbecc07532ed62ccd5c0
Reviewed-on: https://go-review.googlesource.com/22445
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The code sequence for large-offset floating-point stores
includes adding the base pointer to r11. Make sure we
can interpret that instruction correctly.
Fixes build.
Fixes#15440
Change-Id: I7fe5a4a57e08682967052bf77c54e0ec47fcb53e
Reviewed-on: https://go-review.googlesource.com/22440
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
The numbering is only required for parameters of functions/methods
with exported inlineable bodies. For now, always export parameter names
with internal numbering to minimize the diffs between assembly code
dumps of code compiled with the textual vs the binary format.
To be disabled again once the new export format is default.
Change-Id: I6d14c564e734cc5596c7e995d8851e06d5a35013
Reviewed-on: https://go-review.googlesource.com/22441
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Zero the entire buffer so we don't need to
lower its capacity upon return. This lets callers
do some appending without allocation.
Zeroing is cheap, the byte buffer requires only
4 extra instructions.
Fixes#14235
Change-Id: I970d7badcef047dafac75ac17130030181f18fe2
Reviewed-on: https://go-review.googlesource.com/22424
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As a nice side-effect, this allows us to
unify several code paths.
The terminology (low, high, max, simple slice expr,
full slice expr) is taken from the spec and
the examples in the spec.
This is a trial run. The plan, probably for Go 1.8,
is to change slice expressions to use Node.List
instead of OKEY, and to do some similar
tree structure changes for other ops.
Passes toolstash -cmp. No performance change.
all.bash passes with GO_GCFLAGS=-newexport.
Updates #15350
Change-Id: Ic1efdc36e79cdb95ae1636e9817a3ac8f83ab1ac
Reviewed-on: https://go-review.googlesource.com/22425
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
* Make budget an int32 to avoid needless conversions.
* Introduce some temporary variables to reduce repetition.
* If ... args are present, they will be the last argument
to the function. No need to scan all arguments.
Passes toolstash -cmp.
Change-Id: I55203609f5d2f25a4e238cd48c63214651120cfc
Reviewed-on: https://go-review.googlesource.com/22421
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Note that this is only safe because
the compiler generates multiple distinct
gc.Types. If we switch to having canonical
gc.Types, then this will need to be updated
to handle the case in which the user uses both
map[T]S and also map[[8]T]S. In that case,
the runtime needs algs for [8]T, but this could
mark the sole [8]T type as Noalg. This is a general
problem with having a single bool to represent
whether alg generation is needed for a type.
Cuts 5k off cmd/go and 22k off golang.org/x/tools/cmd/godoc,
approx 0.04% and 0.12% respectively.
For #6853 and #9930
Change-Id: I30a15ec72ecb62e2aa053260a7f0f75015fc0ade
Reviewed-on: https://go-review.googlesource.com/19769
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Note that this is only safe because
the compiler generates multiple distinct
gc.Types. If we switch to having canonical
gc.Types, then this will need to be updated
to handle the case in which the user uses both
map[[n]T]S and also calls a function f(...T) with n arguments.
In that case, the runtime needs algs for [n]T, but this could
mark the sole [n]T type as Noalg. This is a general
problem with having a single bool to represent
whether alg generation is needed for a type.
Cuts 17k off cmd/go and 13k off golang.org/x/tools/cmd/godoc,
approx 0.14% and 0.07% respectively.
For #6853 and #9930
Change-Id: Iccb6b9fd88ade5497d7090528a903816d340bf0a
Reviewed-on: https://go-review.googlesource.com/19770
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
func f(x, y, z *int) {
a := []*int{x,y,z}
...
}
We used to use:
var tmp [3]*int
a := tmp[:]
a[0] = x
a[1] = y
a[2] = z
Now we do:
var tmp [3]*int
tmp[0] = x
tmp[1] = y
tmp[2] = z
a := tmp[:]
Doesn't sound like a big deal, but the compiler has trouble
eliminating write barriers when using the former method because it
doesn't know that the slice points to the stack. In the latter
method, the compiler knows the array is on the stack and as a result
doesn't emit any write barriers.
This turns out to be extremely common when building ... args, like
for calls fmt.Printf.
Makes go binaries ~1% smaller.
Doesn't have a measurable effect on the go1 fmt benchmarks,
unfortunately.
Fixes#14263
Update #6853
Change-Id: I9074a2788ec9e561a75f3b71c119b69f304d6ba2
Reviewed-on: https://go-review.googlesource.com/22395
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
On some processors cputicks (used to generate trace timestamps)
produce non-monotonic timestamps. It is important that the parser
distinguishes logically inconsistent traces (e.g. missing, excessive
or misordered events) from broken timestamps. The former is a bug
in tracer, the latter is a machine issue.
Test that (1) parser does not return a logical error in case of
broken timestamps and (2) broken timestamps are eventually detected
and reported.
Change-Id: Ib4b1eb43ce128b268e754400ed8b5e8def04bd78
Reviewed-on: https://go-review.googlesource.com/21608
Reviewed-by: Austin Clements <austin@google.com>
Reloc.SymbolTableIndex is an index into symbol table. But
Reloc.SymbolTableIndex cannot be used as index into File.Symbols,
because File.Symbols slice has Aux lines removed as it is built.
We cannot change the way File.Symbols works, so I propose we
introduce new File.COFFSymbols that does not have that limitation.
Also unlike File.Symbols, File.COFFSymbols will consist of
COFFSymbol. COFFSymbol matches PE COFF specification exactly,
and it is simpler to use.
Updates #15345
Change-Id: Icbc265853a472529cd6d64a76427b27e5459e373
Reviewed-on: https://go-review.googlesource.com/22336
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we're using 32-bit ops for 8/16-bit logical operations
(to avoid partial register stalls), there's really no need to
keep track of the 8/16-bit ops at all. Convert everything we
can to 32-bit ops.
This CL is the obvious stuff. I might think a bit more about
whether we can get rid of weirder stuff like HMULWU.
The only downside to this CL is that we lose some information
about constants. If we had source like:
var a byte = ...
a += 128
a += 128
We will convert that to a += 256, when we could get rid of the
add altogether. This seems like a fairly unusual scenario and
I'm happy with forgoing that optimization.
Change-Id: Ia7c1e5203d0d110807da69ed646535194a3efba1
Reviewed-on: https://go-review.googlesource.com/22382
Reviewed-by: Todd Neal <todd@tneal.org>
Combine stores into larger widths when it is safe to do so.
Add clobber() function so stray dead uses do not impede the
above rewrites.
Fix bug in loads where all intermediate values depending on
a small load (not just the load itself) must have no other uses.
We really need the small load to be dead after the rewrite..
Fixes#14267
Change-Id: Ib25666cb19777f65082c76238fba51a76beb5d74
Reviewed-on: https://go-review.googlesource.com/22326
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Currently tracer uses global sequencer and it introduces
significant slowdown on parallel machines (up to 10x).
Replace the global sequencer with per-goroutine sequencer.
If we assign per-goroutine sequence numbers to only 3 types
of events (start, unblock and syscall exit), it is enough to
restore consistent partial ordering of all events. Even these
events don't need sequence numbers all the time (if goroutine
starts on the same P where it was unblocked, then start does
not need sequence number).
The burden of restoring the order is put on trace parser.
Details of the algorithm are described in the comments.
On http benchmark with GOMAXPROCS=48:
no tracing: 5026 ns/op
tracing: 27803 ns/op (+453%)
with this change: 6369 ns/op (+26%, mostly for traceback)
Also trace size is reduced by ~22%. Average event size before: 4.63
bytes/event, after: 3.62 bytes/event.
Besides running trace tests, I've also tested with manually broken
cputicks (random skew for each event, per-P skew and episodic random skew).
In all cases broken timestamps were detected and no test failures.
Change-Id: I078bde421ccc386a66f6c2051ab207bcd5613efa
Reviewed-on: https://go-review.googlesource.com/21512
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Exporting filenames as part of the position information can lead
to different object files which breaks tests.
Change-Id: Ia678ab64293ebf04bf83601e6ba72919d05762a4
Reviewed-on: https://go-review.googlesource.com/22385
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This provides a way to disable the escaping of <, >, and & in JSON
strings.
Fixes#14749.
Change-Id: I1afeb0244455fc8b06c6cce920444532f229555b
Reviewed-on: https://go-review.googlesource.com/21796
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of switching on Ctype (which internally uses a type switch)
and then scattering lots of type assertions throughout the CTFOO case
clauses, just use type switches directly on the underlying constant
value.
Passes toolstash/buildall.
Change-Id: I9bc172cc67e5f391cddc15539907883b4010689e
Reviewed-on: https://go-review.googlesource.com/22384
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 22372 changed ppc64le to use normal cgo initialization on ppc64le.
Doing this uncovered a cmd/link error using internal linking.
Opened issue 15409 for the problem. This CL disables the test.
Update #15409.
Change-Id: Ia1bb6b874c1b5a4df1a0436c8841c145142c30f7
Reviewed-on: https://go-review.googlesource.com/22379
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Generate new protobuf pprof profiles with embed symbol info.
This makes program binary unnecessary.
Change-Id: Ie628439c13c5e34199782031138102c83ea50621
Reviewed-on: https://go-review.googlesource.com/21873
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Replaced incorrect recursion-free rendering of DFS with
something that was correct. Enhanced test with all
permutations of IF successors to ensure that all possible
DFS traversals are exercised.
Test is improved version of
https://go-review.googlesource.com/#/c/22334
Update 15084.
Change-Id: I6e944c41244e47fe5f568dfc2b360ff93b94079e
Reviewed-on: https://go-review.googlesource.com/22347
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
This reverses the change to this benchmark made in 9b6bf20.
Change-Id: I79ab88286c3028d3be561957140375bbc413e7ab
Reviewed-on: https://go-review.googlesource.com/22340
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
The input buffer is aligned to a doubleword boundary to
improve performance of the vector instructions. The pure
Go implementation is used to align the input data, and is
also used when the vector instructions are not available
or the data length is less than 64 bytes.
Change-Id: Ie259a5f2f1562bcc17961c99e5776c99091d6bed
Reviewed-on: https://go-review.googlesource.com/22201
Reviewed-by: Michael Munday <munday@ca.ibm.com>
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bill O'Farrell <billotosyr@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Remove the "optimization" that was causing the issue.
For the following code the "optimization" was
converting v to (OpCopy x) which is wrong because
x doesn't dominate v.
b1:
y = ...
First .. b3
b2:
x = ...
Goto b3
b3:
v = phi x y
... use v ...
That "optimization" is likely no longer needed because
we now have a second opt pass with a dce in between
which removes blocks of type First.
For pkg/tools/linux_amd64/* the binary size drops
from 82142886 to 82060034.
Change-Id: I10428abbd8b32c5ca66fec3da2e6f3686dddbe31
Reviewed-on: https://go-review.googlesource.com/22312
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
See discussion at [1]. True value must have a fixed non-zero
representation meaning that a && b can be implemented as a & b.
[1] https://groups.google.com/forum/#!topic/golang-dev/xV0vPuFP9Vg
This change helps with m := a && b, but it's more common to see
if a && b { do something } which is not handled.
Change-Id: Ib6f9ff898a0a8c05d12466e2464e4fe781035394
Reviewed-on: https://go-review.googlesource.com/22313
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
They are guaranteed to be non-nil, no point in inserting
nil checks for them.
Fixes#15390
Change-Id: I3b9a0f2319affc2139dcc446d0a56c6785ae5a86
Reviewed-on: https://go-review.googlesource.com/22291
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The special case was because PPC did not support external linking, but
now it does.
Fixes#10410.
Change-Id: I9b024686e0f03da7a44c1c59b41c529802f16ab0
Reviewed-on: https://go-review.googlesource.com/22372
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that reflect.name objects contain an offset to pkgPath instead of a
pointer, there is no need to align the symbol data.
Removes approx. 10KB from the cmd/go binary. The effect becomes more
important later as more type data is moved into name objects.
For #6853
Change-Id: Idb507fdbdad04f16fc224378f82272cb5c236ab7
Reviewed-on: https://go-review.googlesource.com/21776
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the compute intermediate message digest (KIMD) instruction
when possible. Adds test to check fallback code path in case
KIMD is not available.
Benchmark changes:
Hash8Bytes 3.4x
Hash1K 9.3x
Hash8K 10.9x
Change-Id: Ibcd71a886dfd7b3822042235b4f4eaa7a148036b
Reviewed-on: https://go-review.googlesource.com/22350
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Document the subtle property that files with equivalent base names
will overwrite extant templates with those same names.
Fixesgolang/go#14320
Change-Id: Ie9ace1b08e6896ea599836e31582123169aa7a25
Reviewed-on: https://go-review.googlesource.com/21824
Reviewed-by: Rob Pike <r@golang.org>
Before this change, a go-vendor-issue-14613 file would be left in the
working directory after tests run.
Change-Id: If1858421bb287215ab4a19163f489131b2e8912c
Reviewed-on: https://go-review.googlesource.com/22169
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Does anyone actually pass -a to the linker?
Change-Id: I1d31ea66aa5604b7fd42adf15bdab71e9f52d0ed
Reviewed-on: https://go-review.googlesource.com/22356
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
There should be a unit, and s is the SI unit name, so use that.
The other obvious possibility is ns (nanosecond), but the fact
that durations are measured in nanoseconds is an internal detail.
Fixes#14058.
Change-Id: Id1f8f3c77088224d9f7cd643778713d5cc3be5d9
Reviewed-on: https://go-review.googlesource.com/22357
Reviewed-by: Robert Griesemer <gri@golang.org>
The cached copy's ID is sometimes outside the bounds of the orig array.
There's no reason to start at the cached copy and work backwards
to the original value. We already have the original value ID at
all the callsites.
Fixes noopt build
Change-Id: I313508a1917e838a87e8cc83b2ef3c2e4a8db304
Reviewed-on: https://go-review.googlesource.com/22355
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of using TARRAY for both arrays and slices, create a new
TSLICE kind to handle slices.
Also, get rid of the "DDDArray" distinction. While kinda ugly, it
seems likely we'll need to defer evaluating the constant bounds
expressions for golang.org/issue/13890.
Passes toolstash/buildall.
Change-Id: I8e45d4900e7df3a04cce59428ec8b38035d3cc3a
Reviewed-on: https://go-review.googlesource.com/22329
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently when we compute the trigger for the next GC, we do it based
on an estimate of the reachable heap size at the start of the GC
cycle, which is itself based on an estimate of the floating garbage.
This was introduced by 4655aad to fix a bad feedback loop that allowed
the heap to grow to many times the true reachable size.
However, this estimate gets easily confused by rapidly allocating
applications, and, worse it's different than the heap size the trigger
controller uses to compute the trigger itself. This results in the
trigger controller often thinking that GC finished before it started.
Since this would be a pretty great outcome from it's perspective, it
sets the trigger for the next cycle as close to the next goal as
possible (which is limited to 95% of the goal).
Furthermore, the bad feedback loop this estimate originally fixed
seems not to happen any more, suggesting it was fixed more correctly
by some other change in the mean time. Finally, with the change to
allocate black, it shouldn't even be theoretically possible for this
bad feedback loop to occur.
Hence, eliminate the floating garbage estimate and simply consider the
reachable heap to be the marked heap. This harms overall throughput
slightly for allocation-heavy benchmarks, but significantly improves
mutator availability.
Fixes#12204. This brings the average trigger in this benchmark from
0.95 (the cap) to 0.7 and the active GC utilization from ~90% to ~45%.
Updates #14951. This makes the trigger controller much better behaved,
so it pulls the trigger lower if assists are consuming a lot of CPU
like it's supposed to, increasing mutator availability.
name old time/op new time/op delta
XBenchGarbage-12 2.21ms ± 1% 2.28ms ± 3% +3.29% (p=0.000 n=17+17)
Some of this slow down we paid for in earlier commits. Relative to the
start of the series to switch to allocate-black (the parent of "count
black allocations toward scan work"), the garbage benchmark is 2.62%
slower.
name old time/op new time/op delta
BinaryTree17-12 2.53s ± 3% 2.53s ± 3% ~ (p=0.708 n=20+19)
Fannkuch11-12 2.08s ± 0% 2.08s ± 0% -0.22% (p=0.002 n=19+18)
FmtFprintfEmpty-12 45.3ns ± 2% 45.2ns ± 3% ~ (p=0.505 n=20+20)
FmtFprintfString-12 129ns ± 0% 131ns ± 2% +1.80% (p=0.000 n=16+19)
FmtFprintfInt-12 121ns ± 2% 121ns ± 2% ~ (p=0.768 n=19+19)
FmtFprintfIntInt-12 186ns ± 1% 188ns ± 3% +0.99% (p=0.000 n=19+19)
FmtFprintfPrefixedInt-12 188ns ± 1% 188ns ± 1% ~ (p=0.947 n=18+16)
FmtFprintfFloat-12 254ns ± 1% 255ns ± 1% +0.30% (p=0.002 n=19+17)
FmtManyArgs-12 763ns ± 0% 770ns ± 0% +0.92% (p=0.000 n=18+18)
GobDecode-12 7.00ms ± 1% 7.04ms ± 1% +0.61% (p=0.049 n=20+20)
GobEncode-12 5.88ms ± 1% 5.88ms ± 0% ~ (p=0.641 n=18+19)
Gzip-12 214ms ± 1% 215ms ± 1% +0.43% (p=0.002 n=18+19)
Gunzip-12 37.6ms ± 0% 37.6ms ± 0% +0.11% (p=0.015 n=17+18)
HTTPClientServer-12 76.9µs ± 2% 78.1µs ± 2% +1.44% (p=0.000 n=20+18)
JSONEncode-12 15.2ms ± 2% 15.1ms ± 1% ~ (p=0.271 n=19+18)
JSONDecode-12 53.1ms ± 1% 53.3ms ± 0% +0.49% (p=0.000 n=18+19)
Mandelbrot200-12 4.04ms ± 1% 4.03ms ± 0% -0.33% (p=0.005 n=18+18)
GoParse-12 3.29ms ± 1% 3.28ms ± 1% ~ (p=0.146 n=16+17)
RegexpMatchEasy0_32-12 69.9ns ± 3% 69.5ns ± 1% ~ (p=0.785 n=20+19)
RegexpMatchEasy0_1K-12 237ns ± 0% 237ns ± 0% ~ (p=1.000 n=18+18)
RegexpMatchEasy1_32-12 69.5ns ± 1% 69.2ns ± 1% -0.44% (p=0.020 n=16+19)
RegexpMatchEasy1_1K-12 372ns ± 1% 371ns ± 2% ~ (p=0.086 n=20+19)
RegexpMatchMedium_32-12 108ns ± 3% 107ns ± 1% -1.00% (p=0.004 n=19+14)
RegexpMatchMedium_1K-12 34.2µs ± 4% 34.0µs ± 2% ~ (p=0.380 n=19+20)
RegexpMatchHard_32-12 1.77µs ± 4% 1.76µs ± 3% ~ (p=0.558 n=18+20)
RegexpMatchHard_1K-12 53.4µs ± 4% 52.8µs ± 2% -1.10% (p=0.020 n=18+20)
Revcomp-12 359ms ± 4% 377ms ± 0% +5.19% (p=0.000 n=20+18)
Template-12 63.7ms ± 2% 62.9ms ± 2% -1.27% (p=0.005 n=18+20)
TimeParse-12 316ns ± 2% 313ns ± 1% ~ (p=0.059 n=20+16)
TimeFormat-12 329ns ± 0% 331ns ± 0% +0.39% (p=0.000 n=16+18)
[Geo mean] 51.6µs 51.7µs +0.18%
Change-Id: I1dce4640c8205d41717943b021039fffea863c57
Reviewed-on: https://go-review.googlesource.com/21324
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently we allocate white for most of concurrent marking. This is
based on the classical argument that it produces less floating
garbage, since allocations during GC may not get linked into the heap
and allocating white lets us reclaim these. However, it's not clear
how often this actually happens, especially since our write barrier
shades any pointer as soon as it's installed in the heap regardless of
the color of the slot.
On the other hand, allocating black has several advantages that seem
to significantly outweigh this downside.
1) It naturally bounds the total scan work to the live heap size at
the start of a GC cycle. Allocating white does not, and thus depends
entirely on assists to prevent the heap from growing faster than it
can be scanned.
2) It reduces the total amount of scan work per GC cycle by the size
of newly allocated objects that are linked into the heap graph, since
objects allocated black never need to be scanned.
3) It reduces total write barrier work since more objects will already
be black when they are linked into the heap graph.
This gives a slight overall improvement in benchmarks.
name old time/op new time/op delta
XBenchGarbage-12 2.24ms ± 0% 2.21ms ± 1% -1.32% (p=0.000 n=18+17)
name old time/op new time/op delta
BinaryTree17-12 2.60s ± 3% 2.53s ± 3% -2.56% (p=0.000 n=20+20)
Fannkuch11-12 2.08s ± 1% 2.08s ± 0% ~ (p=0.452 n=19+19)
FmtFprintfEmpty-12 45.1ns ± 2% 45.3ns ± 2% ~ (p=0.367 n=19+20)
FmtFprintfString-12 131ns ± 3% 129ns ± 0% -1.60% (p=0.000 n=20+16)
FmtFprintfInt-12 122ns ± 0% 121ns ± 2% -0.86% (p=0.000 n=16+19)
FmtFprintfIntInt-12 187ns ± 1% 186ns ± 1% ~ (p=0.514 n=18+19)
FmtFprintfPrefixedInt-12 189ns ± 0% 188ns ± 1% -0.54% (p=0.000 n=16+18)
FmtFprintfFloat-12 256ns ± 0% 254ns ± 1% -0.43% (p=0.000 n=17+19)
FmtManyArgs-12 769ns ± 0% 763ns ± 0% -0.72% (p=0.000 n=18+18)
GobDecode-12 7.08ms ± 2% 7.00ms ± 1% -1.22% (p=0.000 n=20+20)
GobEncode-12 5.88ms ± 0% 5.88ms ± 1% ~ (p=0.406 n=18+18)
Gzip-12 214ms ± 0% 214ms ± 1% ~ (p=0.103 n=17+18)
Gunzip-12 37.6ms ± 0% 37.6ms ± 0% ~ (p=0.563 n=17+17)
HTTPClientServer-12 77.2µs ± 3% 76.9µs ± 2% ~ (p=0.606 n=20+20)
JSONEncode-12 15.1ms ± 1% 15.2ms ± 2% ~ (p=0.138 n=19+19)
JSONDecode-12 53.3ms ± 1% 53.1ms ± 1% -0.33% (p=0.000 n=19+18)
Mandelbrot200-12 4.04ms ± 1% 4.04ms ± 1% ~ (p=0.075 n=19+18)
GoParse-12 3.30ms ± 1% 3.29ms ± 1% -0.57% (p=0.000 n=18+16)
RegexpMatchEasy0_32-12 69.5ns ± 1% 69.9ns ± 3% ~ (p=0.822 n=18+20)
RegexpMatchEasy0_1K-12 237ns ± 1% 237ns ± 0% ~ (p=0.398 n=19+18)
RegexpMatchEasy1_32-12 69.8ns ± 2% 69.5ns ± 1% ~ (p=0.090 n=20+16)
RegexpMatchEasy1_1K-12 371ns ± 1% 372ns ± 1% ~ (p=0.178 n=19+20)
RegexpMatchMedium_32-12 108ns ± 2% 108ns ± 3% ~ (p=0.124 n=20+19)
RegexpMatchMedium_1K-12 33.9µs ± 2% 34.2µs ± 4% ~ (p=0.309 n=20+19)
RegexpMatchHard_32-12 1.75µs ± 2% 1.77µs ± 4% +1.28% (p=0.018 n=19+18)
RegexpMatchHard_1K-12 52.7µs ± 1% 53.4µs ± 4% +1.23% (p=0.013 n=15+18)
Revcomp-12 354ms ± 1% 359ms ± 4% +1.27% (p=0.043 n=20+20)
Template-12 63.6ms ± 2% 63.7ms ± 2% ~ (p=0.654 n=20+18)
TimeParse-12 313ns ± 1% 316ns ± 2% +0.80% (p=0.014 n=17+20)
TimeFormat-12 332ns ± 0% 329ns ± 0% -0.66% (p=0.000 n=16+16)
[Geo mean] 51.7µs 51.6µs -0.09%
Change-Id: I2214a6a0e4f544699ea166073249a8efdf080dc0
Reviewed-on: https://go-review.googlesource.com/21323
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently allocating black switches to the system stack (which is
probably a historical accident) and atomically updates the global
bytes marked stat. Since we're about to depend on this much more,
optimize it a bit by putting it back on the regular stack and updating
the per-P bytes marked stat, which gets lazily folded into the global
bytes marked stat.
Change-Id: Ibbe16e5382d3fd2256e4381f88af342bf7020b04
Reviewed-on: https://go-review.googlesource.com/22170
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently we count black allocations toward the scannable heap size,
but not toward the scan work we've done so far. This is clearly
inconsistent (we have, in effect, scanned these allocations and since
they're already black, we're not going to scan them again). Worse, it
means we don't count black allocations toward the scannable heap size
as of the *next* GC because this is based on the amount of scan work
we did in this cycle.
Fix this by counting black allocations as scan work. Currently the GC
spends very little time in allocate-black mode, so this probably
hasn't been a problem, but this will become important when we switch
to always allocating black.
Change-Id: If6ff693b070c385b65b6ecbbbbf76283a0f9d990
Reviewed-on: https://go-review.googlesource.com/22119
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allows passing regexps per subtest to --test.run and --test.bench
Note that the documentation explicitly states that the split regular
expressions match the correpsonding parts (path components) of
the bench/test identifier. This is intended and slightly different
from the i'th RE matching the subtest/subbench at the respective
level. Picking this semantics allows guaranteeing that a test or
benchmark identifier as printed by go test can be passed verbatim
(possibly quoted) to, respectively, -run or -bench: subtests and
subbenches might have a '/' in their name, causing a misaligment if
their ID is passed to -run or -bench as is.
This semantics has other benefits, but this is the main motivation.
Fixes golang.go#15126
Change-Id: If72e6d3f54db1df6bc2729ac6edc7ab3c740e7c3
Reviewed-on: https://go-review.googlesource.com/19122
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Progress on SSA backend for ARM. Still not complete. It compiles a
Fibonacci function, but the caller picked the return value from an
incorrect offset. This CL adjusts it to match the stack frame layout
for architectures with link register.
Updates #15365.
Change-Id: I01e03c3e95f5503a185e8ac2b6d9caf4faf3d014
Reviewed-on: https://go-review.googlesource.com/22186
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Progress on SSA for ARM. Still not complete. Now Fibonacci function compiles
and runs correctly.
The old backend swaps the operands for CMP instruction. This CL does the same
on SSA backend, and uses conditional branch accordingly.
Updates #15365.
Change-Id: I117e17feb22f03d936608bd232f76970e4bbe21a
Reviewed-on: https://go-review.googlesource.com/22187
Reviewed-by: Keith Randall <khr@golang.org>
Reduces link time for cmd/go by 1%.
Change-Id: Iad4a16db0aedc56f81ddf73ba9b632e418dc1b19
Reviewed-on: https://go-review.googlesource.com/22242
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
cmd/link reads PE object files when building programs with cgo.
cmd/link accesses object relocations. Add new Section.Relocs that
provides similar functionality in debug/pe.
Updates #15345
Change-Id: I34de91b7f18cf1c9e4cdb3aedd685486a625ac92
Reviewed-on: https://go-review.googlesource.com/22332
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Consistently use type int for the size argument of
runtime.newarray, runtime.reflect_unsafe_NewArray
and reflect.unsafe_NewArray.
Change-Id: Ic77bf2dde216c92ca8c49462f8eedc0385b6314e
Reviewed-on: https://go-review.googlesource.com/22311
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For the Solaris and S/390 builders.
Change-Id: Id9a83e0df91e6d0df8488ec5e2a546ba8e2d800e
Reviewed-on: https://go-review.googlesource.com/22327
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Michael Munday <munday@ca.ibm.com>
Test with forceNewExport set to true (but continues to be disabled by
default for now).
Fixes#15322.
Change-Id: I3b893db2206cbb79e66339284f22f4a0b20bf137
Reviewed-on: https://go-review.googlesource.com/22328
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
First (and largest single) step to switching cmd/link from linked
lists of symbols to slices.
Sort sections independently and concurrently.
This reduces jujud link times on linux/amd64 by ~4%.
Updates #15374
Change-Id: I452bc8f33081039468636502fe3c1cc8d6ed9efa
Reviewed-on: https://go-review.googlesource.com/22205
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
This change improves the performance of the block
function used within crypto/md5 on ppc64le. The following
improvement was seen:
BenchmarkHash8Bytes 8.39 26.04 3.10x
BenchmarkHash1K 99.41 407.84 4.10x
BenchmarkHash8K 108.87 460.00 4.23x
BenchmarkHash8BytesUnaligned 8.39 25.80 3.08x
BenchmarkHash1KUnaligned 89.94 407.81 4.53x
BenchmarkHash8KUnaligned 96.57 459.22 4.76x
Fixes#15385
Change-Id: I8af5af089cc3e3740c33c662003d104de5fe1d1b
Reviewed-on: https://go-review.googlesource.com/22294
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Follow-up to https://golang.org/cl/21755.
This turned out to be a bit more than just a few nits
as originally expected in that CL.
1) The actual mantissa may be shorter than required for the
given precision (because of trailing 0's): no need to
allocate space for it (and transmit 0's). This can save
a lot of space when the precision is high: E.g., for
prec == 1000, 16 words or 128 bytes are required at the
most, but if the actual number is short, it may be much
less (for the test cases present, it's significantly less).
2) The actual mantissa may be longer than the number of
words required for the given precision: make sure to
not overflow when encoding in bytes.
3) Add more documentation.
4) Add more tests.
Change-Id: I9f40c408cfdd9183a8e81076d2f7d6c75e7a00e9
Reviewed-on: https://go-review.googlesource.com/22324
Reviewed-by: Alan Donovan <adonovan@google.com>
mapaccess{1,2} returns a pointer to the value. When the key
is not in the map, it returns a pointer to zeroed memory.
Currently, for large map values we have a complicated scheme which
dynamically allocates zeroed memory for this purpose. It is ugly
code and requires an atomic.Load in a bunch of places we'd rather
not have it.
Switch to a scheme where callsites of mapaccess{1,2} which expect
large return values pass in a pointer to zeroed memory that
mapaccess can return if the key is not found. This avoids the
atomic.Load on all map accesses with a few extra instructions only
for the large value acccesses, plus a bit of bss space.
There was a time (1.4 & 1.5?) where we did something like this but
all the tricks to make the right size zero value were done by the
linker. That scheme broke in the presence of dyamic linking.
The scheme in this CL works even when dynamic linking.
Fixes#12337
Change-Id: Ic2d0319944af33bbb59785938d9ab80958d1b4b1
Reviewed-on: https://go-review.googlesource.com/22221
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Added GobEncode/Decode and a test for them.
Fixes#14593
Change-Id: Ic8d3efd24d0313a1a66f01da293c4c1fd39764a8
Reviewed-on: https://go-review.googlesource.com/21755
Reviewed-by: Robert Griesemer <gri@golang.org>
Adds support for single block encryption using the cipher message
(KM) instruction. KM handles key expansion internally and
therefore it is not done up front when using the assembly
implementation on s390x.
Change-Id: I69954b8ae36d549e1dc40d7acd5a10bedfaaef9c
Reviewed-on: https://go-review.googlesource.com/22194
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bill O'Farrell <billotosyr@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Improve forward-looking desired register calculations.
It is now inter-block and handles a bunch more cases.
Fixes#14504Fixes#14828Fixes#15254
Change-Id: Ic240fa0ec6a779d80f577f55c8a6c4ac8c1a940a
Reviewed-on: https://go-review.googlesource.com/22160
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
In BenchmarkDup fuction, heap is created as h := make(myHeap, n)
and then n elements are added, so first time there are 2*n elements
in heap.
Fixes#15380
Change-Id: I0508486a847006b3cd545fd695e8b09af339134f
Reviewed-on: https://go-review.googlesource.com/22310
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
