While tinkering with different block orders for the preemptible
loop experiment, crashed the register allocator with a "bad"
one (these exist). Realized that one knob was controlling
two things (register allocation and branch patterns) and
decided that life would be simpler if the two orders were
independent.
Ran some experiments and determined that we have probably,
mostly, been optimizing for register allocation effects, not
branch effects. Bad block orders for register allocation are
somewhat costly.
This will also allow separate experimentation with perhaps-
better block orders for register allocation.
Change-Id: I6ecf2f24cca178b6f8acc0d3c4caaef043c11ed9
Reviewed-on: https://go-review.googlesource.com/47314
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Enabled when the tool runs with DEBUG_MEMORY_USAGE=1 env var.
After reporting the usage, it waits until user enters input
(helpful when checking top or other memory monitor)
Also adds net/http/pprof to export debug endpoints.
From the trace included in #21870
$ DEBUG_MEMORY_USAGE=1 go tool trace trace.out
2018/02/21 16:04:49 Parsing trace...
after parsing trace
Alloc: 3385747848 Bytes
Sys: 3661654648 Bytes
HeapReleased: 0 Bytes
HeapSys: 3488907264 Bytes
HeapInUse: 3426377728 Bytes
HeapAlloc: 3385747848 Bytes
Enter to continue...
2018/02/21 16:05:09 Serializing trace...
after generating trace
Alloc: 4908929616 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4982865920 Bytes
HeapAlloc: 4908929616 Bytes
Enter to continue...
2018/02/21 16:05:18 Splitting trace...
after spliting trace
Alloc: 4909026200 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4983046144 Bytes
HeapAlloc: 4909026200 Bytes
Enter to continue...
2018/02/21 16:05:39 Opening browser. Trace viewer is listening on http://127.0.0.1:33661
after httpJsonTrace
Alloc: 5288336048 Bytes
Sys: 7790245896 Bytes
HeapReleased: 0 Bytes
HeapSys: 7381123072 Bytes
HeapInUse: 5324120064 Bytes
HeapAlloc: 5288336048 Bytes
Enter to continue...
Change-Id: I88bb3cb1af3cb62e4643a8cbafd5823672b2e464
Reviewed-on: https://go-review.googlesource.com/92355
Reviewed-by: Peter Weinberger <pjw@google.com>
The existing code was somewhat convoluted and made several assumptions
about the encoding of position bases:
1) The position's base for a file contained a position whose base
pointed to itself (which is true but an implementation detail
of src.Pos).
2) Updating the position base for a line directive required finding
the base of the most recent's base position.
This change simply stores the file's position base and keeps using it
directly for each line directive (instead of getting it from the most
recently updated base).
Change-Id: I4d80da513bededb636eab0ce53257fda73f0dbc0
Reviewed-on: https://go-review.googlesource.com/95736
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now that we support the full non-contiguous virtual address space of
amd64 hardware, some of the comments and constants related to this are
out of date.
This renames memLimitBits to heapAddrBits because 1<<memLimitBits is
no longer the limit of the address space and rewrites the comment to
focus first on hardware limits (which span OSes) and then discuss
kernel limits.
Second, this eliminates the memLimit constant because there's no
longer a meaningful "highest possible heap pointer value" on amd64.
Updates #23862.
Change-Id: I44b32033d2deb6b69248fb8dda14fc0e65c47f11
Reviewed-on: https://go-review.googlesource.com/95498
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
On amd64, the virtual address space, when interpreted as signed
values, is [-2^47, 2^47). Currently, we only support heap addresses in
the "positive" half of this, [0, 2^47). This suffices for linux/amd64
and windows/amd64, but solaris/amd64 can map user addresses in the
negative part of this range. Specifically, addresses
0xFFFF8000'00000000 to 0xFFFFFD80'00000000 are part of user space.
This leads to "memory allocated by OS not in usable address space"
panic, since we don't map heap arena index space for these addresses.
Fix this by offsetting addresses when computing arena indexes so that
arena entry 0 corresponds to address -2^47 on amd64. We already map
enough arena space for 2^48 heap addresses on 64-bit (because arm64's
virtual address space is [0, 2^48)), so we don't need to grow any
structures to support this.
A different approach would be to simply mask out the top 16 bits.
However, there are two advantages to the offset approach: 1) invalid
heap addresses continue to naturally map to invalid arena indexes so
we don't need extra checks and 2) it perturbs the mapping of addresses
to arena indexes more, which helps check that we don't accidentally
compute incorrect arena indexes somewhere that happen to be right most
of the time.
Several comments and constant names are now somewhat misleading. We'll
fix that in the next CL. This CL is the core change the arena
indexing.
Fixes#23862.
Change-Id: Idb8e299fded04593a286b01a9582da6ddbac2f9a
Reviewed-on: https://go-review.googlesource.com/95497
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Accessing the arena index is about to get slightly more complicated.
Abstract this away into a set of functions for going back and forth
between addresses and arena slice indexes.
For #23862.
Change-Id: I0b20e74ef47a07b78ed0cf0a6128afe6f6e40f4b
Reviewed-on: https://go-review.googlesource.com/95496
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, bulkBarrierPreWrite uses inheap to decide whether the
destination is in the heap or whether to check for stack or global
data. However, this isn't the best question to ask.
Instead, get the span directly and query its state. This lets us
directly determine whether this might be a global, or is stack memory,
or is heap memory.
At this point, inheap is no longer used in the hot path, so drop it
from the must-be-inlined list and substitute spanOf.
This will help in a circuitous way with #23862, since fixing that is
going to push inheap very slightly over the inline-able threshold on a
few platforms.
Change-Id: I5360fc1181183598502409f12979899e1e4d45f7
Reviewed-on: https://go-review.googlesource.com/95495
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The task-oriented trace view presents the execution trace organized
based on goroutines. Often, which P a goroutine was running on is
useful, so this CL includes the P ids in the goroutine execution slices.
R=go1.11
Change-Id: I96539bf8215e5c1cd8cc997a90204f57347c48c8
Reviewed-on: https://go-review.googlesource.com/90221
Reviewed-by: Heschi Kreinick <heschi@google.com>
The GC time for a task is defined by the sum of GC duration
overlapping with the task's duration.
Also, grey out non-overlapping slices in the task-oriented
trace view.
R=go1.11
Change-Id: I42def0eb520f5d9bd07edd265e558706f6fab552
Reviewed-on: https://go-review.googlesource.com/90219
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reuse even more memory, and keep track of it in a long-lived debugState
object rather than piecemeal in the Cache.
Change-Id: Ib6936b4e8594dc6dda1f59ece753c00fd1c136ba
Reviewed-on: https://go-review.googlesource.com/92404
Reviewed-by: David Chase <drchase@google.com>
Change the closures to methods on debugState, mostly just for aesthetic
reasons.
Change-Id: I5242807f7300efafc7efb4eb3bd305ac3ec8e826
Reviewed-on: https://go-review.googlesource.com/92403
Reviewed-by: David Chase <drchase@google.com>
changedVars was functionally a set, but couldn't be iterated over
efficiently. In functions with many variables, the wasted iteration was
costly. Use a sparseSet instead.
(*gc.Node).String() is very expensive: it calls Sprintf, which does
reflection, etc, etc. Instead, just look at .Sym.Name, which is all we
care about.
Change-Id: Ib61cd7b5c796e1813b8859135e85da5bfe2ac686
Reviewed-on: https://go-review.googlesource.com/92402
Reviewed-by: David Chase <drchase@google.com>
Replace the OnStack boolean in VarLoc with a flag bit in StackOffset.
This doesn't get much memory savings since it's still 64-bit aligned,
but does seem to help a bit anyway.
Change liveSlot to fit into 16 bytes. Because nested structs still get
padding, this required inlining it. Fortunately there's not much logic
to copy.
Change-Id: Ie19a409daa41aa310275c4517a021eecf8886441
Reviewed-on: https://go-review.googlesource.com/92401
Reviewed-by: David Chase <drchase@google.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no changes in the rewrite files.
Change-Id: I8d8cc67d02faca4756cc02402b763f1645ee31de
Reviewed-on: https://go-review.googlesource.com/95935
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ia74acc389cd8310eb7fe8f927171fa3d292d2a86
Reviewed-on: https://go-review.googlesource.com/95797
Reviewed-by: Giovanni Bajo <rasky@develer.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ib0bfbbc181fcec095fb78ac752addd1eee0c3575
Reviewed-on: https://go-review.googlesource.com/95796
Reviewed-by: Giovanni Bajo <rasky@develer.com>
No changes in the generated file, as expected.
Change-Id: I30e0404612cd150f1455378b8db1c18b1e12d34e
Reviewed-on: https://go-review.googlesource.com/95616
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We did warn on them in some cases, but not others. In particular, if one
used a slice composite literal with struct pointer elements, and omitted
the type of an element's composite literal, it would not get any warning
even if it should get one.
The issue is that typ.Underlying() can be of type *types.Pointer. Skip
those levels of indirection before checking for a *types.Struct
underlying type.
isLocalType also needed a bit of tweaking to ignore dereferences.
Perhaps that can be rewritten now that we have type info, but let's
leave it for another time.
Fixes#23539.
Change-Id: I727a497284df1325b70d47a756519f5db1add25d
Reviewed-on: https://go-review.googlesource.com/89715
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sometimes, multiple CLs being merged that create rules on the same
opcodes can cause the generated file to differ compared to a new
regeneration. This is caused by the fact that rulegen splits
generated functions in chunks of 10 rules per function (to avoid
creating functions that are too big). If two CLs add rules to
the same function, they might cause a generated function to
have more than 10 rules, even though each CL individually didn't
pass this limit.
Change-Id: Ib641396b7e9028f80ec8718746969d390a9fbba9
Reviewed-on: https://go-review.googlesource.com/95795
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Calling UDPConn readers (Read, ReadFrom, ReadMsgUDP) to read part of
datagram returns error (in Windows), mentioning there is more data
available, and 0 as size of read data, even though part of data is
already read.
This fix makes UDPConn readers to return truncated payload size,
even there is error due more data available to read.
Fixes#14074
Updates #18056
Change-Id: Id7eec7f544dd759b2d970fa2561eef2937ec4662
Reviewed-on: https://go-review.googlesource.com/92475
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ib1d2b9fbc787379105ec9baf10d2c1e2ff3c4c5c
Reviewed-on: https://go-review.googlesource.com/95615
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We don't want to account the memory for mheap_.arenas because most of
it is never touched, so currently we pass the address of a uint64 on
the heap. However, at least on mips, it's possible for this uint64 to
be unaligned, which causes the atomic add in mSysStatInc to crash.
Fix this by instead passing a nil stat pointer.
Fixes#23946.
Change-Id: I091587df1b3066c330b6bb4d834e4596c407910f
Reviewed-on: https://go-review.googlesource.com/95695
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
+ Move from Markdown checklist to text. The first PR comment is
presented as text when creating it.
+ Add the note about Signed-Off-By: not being required.
Change-Id: I0650891dcf11ed7dd367007148730ba2917784fe
Reviewed-on: https://go-review.googlesource.com/95696
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently zip benchmarks spend 60% in the rleBuffer code,
which is used only to test zip archive/zip itself:
17.48s 37.02% 37.02% 18.12s 38.37% archive/zip.(*rleBuffer).ReadAt
9.51s 20.14% 57.16% 10.43s 22.09% archive/zip.(*rleBuffer).Write
9.15s 19.38% 76.54% 10.85s 22.98% compress/flate.(*compressor).deflate
This means that benchmarks currently test performance of test helper.
Updating ReadAt/Write methods to be more performant makes benchmarks closer to real world.
name old time/op new time/op delta
CompressedZipGarbage-8 2.34ms ± 0% 2.34ms ± 1% ~ (p=0.684 n=10+10)
Zip64Test-8 58.1ms ± 2% 10.7ms ± 1% -81.54% (p=0.000 n=10+10)
Zip64TestSizes/4096-8 4.05µs ± 2% 3.65µs ± 5% -9.96% (p=0.000 n=9+10)
Zip64TestSizes/1048576-8 238µs ± 0% 43µs ± 0% -82.06% (p=0.000 n=10+10)
Zip64TestSizes/67108864-8 15.3ms ± 1% 2.6ms ± 0% -83.12% (p=0.000 n=10+9)
name old alloc/op new alloc/op delta
CompressedZipGarbage-8 17.9kB ±14% 16.0kB ±24% -10.48% (p=0.026 n=9+10)
name old allocs/op new allocs/op delta
CompressedZipGarbage-8 44.0 ± 0% 44.0 ± 0% ~ (all equal)
Change-Id: Idfd920d0e4bed4aec2f5be84dc7e3919d9f1dd2d
Reviewed-on: https://go-review.googlesource.com/83857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
reflect.unsafe_New is an often called function according
to profiling in a large production environment.
Since newobject is not inlined currently there
is call overhead that can be avoided by calling
mallocgc directly.
name old time/op new time/op delta
New 32.4ns ± 2% 29.8ns ± 1% -8.03% (p=0.000 n=19+20)
Change-Id: I572e4be830ed8e5c0da555dc3a8864c8363112be
Reviewed-on: https://go-review.googlesource.com/95015
Reviewed-by: Austin Clements <austin@google.com>
For functions with many local variables, keeping track of every
LocalSlot for every variable is very expensive. Only track the slots
that are actually used by a given variable.
Change-Id: Iaafbce030a782b8b8c4a0eb7cf025e59af899ea4
Reviewed-on: https://go-review.googlesource.com/92400
Reviewed-by: David Chase <drchase@google.com>
Keeping the start state of each block around costs more than just
recomputing them as necessary, especially because many blocks only have
one predecessor and don't need any merging at all. Stop storing the
start state, and reuse predecessors' end states as much as conveniently
possible.
Change-Id: I549bad9e1a35af76a974e46fe69f74cd4dce873b
Reviewed-on: https://go-review.googlesource.com/92399
Reviewed-by: David Chase <drchase@google.com>
This saves an instruction and a register. The new rules
match ~4900 times during all.bash.
Change-Id: I2f867c5e70262004e31f545f3bb89e939c45b718
Reviewed-on: https://go-review.googlesource.com/94767
Reviewed-by: Keith Randall <khr@golang.org>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ibf157382fb4544c063fbc80406fb9302430728fe
Reviewed-on: https://go-review.googlesource.com/95595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For now, limited to a few repetitive boolean rules where the win is
substantial (4+ variants).
Change-Id: I67bce0d356ca7d71a0f15ff98551fe2caff8abf9
Reviewed-on: https://go-review.googlesource.com/95535
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds the ability to add a #x:y anchor to the trace view URL that
causes the viewer to initially select from x ms to y ms.
Change-Id: I4a980d8128ecc85dbe41f224e8ae336707a4eaab
Reviewed-on: https://go-review.googlesource.com/60794
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
In addition to look nicer to the eye, this allows to reformat
and indent rules without causing spurious changes to the generated
file, making it easier to spot functional changes.
After this CL, all CLs that will aggregate rules through
the new "|" functionality should cause no changes to the
generated files.
Change-Id: Icec283585ba8d7b91c79d76513c1d83dca4b30aa
Reviewed-on: https://go-review.googlesource.com/95216
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Replace some ints with bool and use arrays istead of slices where
possible.
Change-Id: I510bdaec48f9c437685e72c4a3291cffeb7ef5fc
Reviewed-on: https://go-review.googlesource.com/83859
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code was maintaining manual versions of it in multiple places -
replace all of them.
Change-Id: I04c3063877b05ba914de9f5dddb33ffe09f308fe
Reviewed-on: https://go-review.googlesource.com/95356
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
