Allows safely eliminating more direct uses of Type's Type and Down
fields.
Passes toolstash -cmp.
Change-Id: I5c17fe541a0473c3cd2978d8314c4ab759079a61
Reviewed-on: https://go-review.googlesource.com/20541
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL was mostly produced by a one-off automated rewrite tool
looking for statements like "for X := T.Type; X != nil; X = X.Down"
and a few minor variations.
Passes toolstash -cmp.
Change-Id: Ib22705e37d078ef97841ee2e08f60bdbcabb94ad
Reviewed-on: https://go-review.googlesource.com/20520
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
That was easy.
Fixes#14473.
Change-Id: I9d1d20a5c5a9b1423e6c72c0460ee4a78130864f
Reviewed-on: https://go-review.googlesource.com/20521
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: Ice3aa807169f4fec85745a3991b1084a9f85c1b5
Reviewed-on: https://go-review.googlesource.com/20499
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Accessing the n'th field of a struct is fairly common, and in
particular accessing the 0'th field of the receiver parameter list is
very common. Add helper methods for both of these tasks and update
code to make use of them.
Change-Id: I81f551fecdca306b3800636caebcd0dc106f2ed7
Reviewed-on: https://go-review.googlesource.com/20498
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
Today the linker keeps all methods of reachable types. This is
necessary if a program uses reflect.Value.Call. But while use of
reflection is widespread in Go for encoders and decoders, using
it to call a method is rare.
This CL looks for the use of reflect.Value.Call in a program, and
if it is absent, adopts a (reasonably conservative) method pruning
strategy as part of dead code elimination. Any method that is
directly called is kept, and any method that matches a used
interface's method signature is kept.
Whether or not a method body is kept is determined by the relocation
from its receiver's *rtype to its *rtype. A small change in the
compiler marks these relocations as R_METHOD so they can be easily
collected and manipulated by the linker.
As a bonus, this technique removes the text segment of methods that
have been inlined. Looking at the output of building cmd/objdump with
-ldflags=-v=2 shows that inlined methods like
runtime.(*traceAllocBlockPtr).ptr are removed from the program.
Relatively little work is necessary to do this. Linking two
examples, jujud and cmd/objdump show no more than +2% link time.
Binaries that do not use reflect.Call.Value drop 4 - 20% in size:
addr2line: -793KB (18%)
asm: -346KB (8%)
cgo: -490KB (10%)
compile: -564KB (4%)
dist: -736KB (17%)
fix: -404KB (12%)
link: -328KB (7%)
nm: -827KB (19%)
objdump: -712KB (16%)
pack: -327KB (14%)
yacc: -350KB (10%)
Binaries that do use reflect.Call.Value see a modest size decrease
of 2 - 6% thanks to pruning of unexported methods:
api: -151KB (3%)
cover: -222KB (4%)
doc: -106KB (2.5%)
pprof: -314KB (3%)
trace: -357KB (4%)
vet: -187KB (2.7%)
jujud: -4.4MB (5.8%)
cmd/go: -384KB (3.4%)
The trivial Hello example program goes from 2MB to 1.68MB:
package main
import "fmt"
func main() {
fmt.Println("Hello, 世界")
}
Method pruning also helps when building small binaries with
"-ldflags=-s -w". The above program goes from 1.43MB to 1.2MB.
Unfortunately the linker can only tell if reflect.Value.Call has been
statically linked, not if it is dynamically used. And while use is
rare, it is linked into a very common standard library package,
text/template. The result is programs like cmd/go, which don't use
reflect.Value.Call, see limited benefit from this CL. If binary size
is important enough it may be possible to address this in future work.
For #6853.
Change-Id: Iabe90e210e813b08c3f8fd605f841f0458973396
Reviewed-on: https://go-review.googlesource.com/20483
Reviewed-by: Russ Cox <rsc@golang.org>
More idiomatic naming (in particular, matches the naming used for
go/types.Signature).
Also, convert more code to use these methods and/or IterFields.
(Still more to go; only made a quick pass for low hanging fruit.)
Passes toolstash -cmp.
Change-Id: I61831bfb1ec2cd50d4c7efc6062bca4e0dcf267b
Reviewed-on: https://go-review.googlesource.com/20451
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alternative to golang.org/cl/19852. This memory layout doesn't have
an easy type representation, but it is noticeably smaller than the
current funcType, and saves significant extra space.
Some notes on the layout are in reflect/type.go:
// A *rtype for each in and out parameter is stored in an array that
// directly follows the funcType (and possibly its uncommonType). So
// a function type with one method, one input, and one output is:
//
// struct {
// funcType
// uncommonType
// [2]*rtype // [0] is in, [1] is out
// uncommonTypeSliceContents
// }
There are three arbitrary limits introduced by this CL:
1. No more than 65535 function input parameters.
2. No more than 32767 function output parameters.
3. reflect.FuncOf is limited to 128 parameters.
I don't think these are limits in practice, but are worth noting.
Reduces godoc binary size by 2.4%, 330KB.
For #6853.
Change-Id: I225c0a0516ebdbe92d41dfdf43f716da42dfe347
Reviewed-on: https://go-review.googlesource.com/19916
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of a pointer on every rtype, use a bit flag to indicate that
the contents of uncommonType directly follows the rtype value when it
is needed.
This requires a bit of juggling in the compiler's rtype encoder. The
backing arrays for fields in the rtype are presently encoded directly
after the slice header. This packing requires separating the encoding
of the uncommonType slice headers from their backing arrays.
Reduces binary size of godoc by ~180KB (1.5%).
No measurable change in all.bash time.
For #6853.
Change-Id: I60205948ceb5c0abba76fdf619652da9c465a597
Reviewed-on: https://go-review.googlesource.com/19790
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Deleting the string merging pass makes the linker 30-35% faster
but makes jujud (using the github.com/davecheney/benchjuju snapshot) 2.5% larger.
Two optimizations bring the space overhead down to 0.6%.
First, change the default alignment for string data to 1 byte.
(It was previously defaulting to larger amounts, usually pointer width.)
Second, write out the type string for T (usually a bigger expression) as "*T"[1:],
so that the type strings for T and *T share storage.
Combined, these obtain the bulk of the benefit of string merging
at essentially no cost. The remaining benefit from string merging
is not worth the excessive cost, so delete it.
As penance for making the jujud binary 0.6% larger,
the next CL in this sequence trims the reflect functype
information enough to make the jujud binary overall 0.75% smaller
(that is, that CL has a net -1.35% effect).
For #6853.
Fixes#14648.
Change-Id: I3fdd74c85410930c36bb66160ca4174ed540fc6e
Reviewed-on: https://go-review.googlesource.com/20334
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
They were only used for rtype.ptrToThis which David Crawshaw removed a couple
of weeks ago. Removes two traversals of Ctxt.Allsym from the linker but it
doesn't seem to make much difference to performance.
Change-Id: I5c305e0180186f643221d57822d301de4aa18827
Reviewed-on: https://go-review.googlesource.com/20287
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Simplifies some code as ptrToThis was unreliable under dynamic
linking. Now the same type lookup is used regardless of execution
mode.
A synthetic relocation, R_USETYPE, is introduced to make sure the
linker includes *T on use of T, if *T is carrying methods.
Changes the heap dump format. Anything reading the format needs to
look at the last bool of a type of an interface value to determine
if the type should be the pointer-to type.
Reduces binary size of cmd/go by 0.2%.
For #6853.
Change-Id: I79fcb19a97402bdb0193f3c7f6d94ddf061ee7b2
Reviewed-on: https://go-review.googlesource.com/19695
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replaced comparison based on (*Type).String() with an
allocation-free structural comparison. Roughly doubles
speed of CSE, also reduces allocations.
Checked that roughly the same number of CSEs were detected
during make.bash (about a million) and that "new" CSEs
were caused by the effect described above.
Change-Id: Id205a9f6986efd518043e12d651f0b01206aeb1b
Reviewed-on: https://go-review.googlesource.com/19471
Reviewed-by: Keith Randall <khr@golang.org>
Also eliminates per-maptype hiter and hmap types, since they're not
really needed anyway. Update packages reflect and runtime
accordingly.
Reduces golang.org/x/tools/cmd/godoc's text segment by ~170kB:
text data bss dec hex filename
13085702 140640 151520 13377862 cc2146 godoc.before
12915382 140640 151520 13207542 c987f6 godoc.after
Updates #6853.
Change-Id: I948b2bc1f22d477c1756204996b4e3e1fb568d81
Reviewed-on: https://go-review.googlesource.com/16610
Reviewed-by: Keith Randall <khr@golang.org>
Because there are now multiple packages that compose the runtime
we need to distinguish between the case where a runtime package
is being compiled versus the case the "runtime" package is being
compiled. In golang.org/cl/14204 I mistakenly used
localpkg.Name == "runtime"
to check against the "runtime" package, but doing this would treat
a package with the path "foo.org/bar/runtime" as the runtime package.
The correct check is
myimportpath == "runtime"
.
Change-Id: If90e95cef768d91206f2df1c06e27be876722e4e
Reviewed-on: https://go-review.googlesource.com/17059
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This change breaks out most of the atomics functions in the runtime
into package runtime/internal/atomic. It adds some basic support
in the toolchain for runtime packages, and also modifies linux/arm
atomics to remove the dependency on the runtime's mutex. The mutexes
have been replaced with spinlocks.
all trybots are happy!
In addition to the trybots, I've tested on the darwin/arm64 builder,
on the darwin/arm builder, and on a ppc64le machine.
Change-Id: I6698c8e3cf3834f55ce5824059f44d00dc8e3c2f
Reviewed-on: https://go-review.googlesource.com/14204
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Type Op is enfored now.
Type EType will need further CLs.
Added TODOs where Node.EType is used as a union type.
The TODOs have the format `TODO(marvin): Fix Node.EType union type.`.
Furthermore:
-The flag of Econv function in fmt.go is removed, since unused.
-Some cleaning along the way, e.g. declare vars first when getting initialized.
Passes go build -toolexec 'toolstash -cmp' -a std.
Fixes#11846
Change-Id: I908b955d5a78a195604970983fb9194bd9e9260b
Reviewed-on: https://go-review.googlesource.com/14956
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
gc will need to be rebuild.
Package that assume f.PkgPath != nil means a field is unexported and
must be ignored must be revised to check for
f.PkgPath != nil && !f.Anonymous,
so that they do try to walk into the embedded fields to look for
exported fields contained within.
Closes#12367, fixes#7363, fixes#11007, and fixes#7247.
Change-Id: I16402ee21ccfede80f277f84b3995cf26e97433d
Reviewed-on: https://go-review.googlesource.com/14085
Reviewed-by: Russ Cox <rsc@golang.org>
Update old c-style comments to look like Go comments. Also replace some
lingering references to old .c files that don't exist anymore.
Change-Id: I72b2407a40fc76c23e9048643e0622fd70b4cf90
Reviewed-on: https://go-review.googlesource.com/16190
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The -msan option causes the compiler to add instrumentation for the
C/C++ memory sanitizer. Every memory read/write will be preceded by
a call to msanread/msanwrite.
This CL passes tests but is not usable by itself. The actual
implementation of msanread/msanwrite in the runtime package, and support
for -msan in the go tool and the linker, and tests, will follow in
subsequent CLs.
Change-Id: I3d517fb3e6e65d9bf9433db070a420fd11f57816
Reviewed-on: https://go-review.googlesource.com/16160
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Generate slices of method *Sig(nature)s instead of linked lists.
Remove custom lsort function in favor of sort.Interface.
Eliminates another use of stringsCompare.
Passes go build -a -toolexec 'toolstash -cmp' std cmd.
Change-Id: I9ed1664b7f55be9e967dd7196e396a76f6ea3422
Reviewed-on: https://go-review.googlesource.com/14559
Reviewed-by: Dave Cheney <dave@cheney.net>
This one of a set of changes to make the transition away from NodeList
easier by removing cases in which NodeList doesn't act semi-trivially like a
[]*Node.
This CL was originally prepared by Josh Bleecher Snyder <josharian@gmail.com>.
This change passes go build -toolexec 'toolstash -cmp' -a std.
Change-Id: Ifd73501e06e8ea5efd028b6d473b3e5d1b07a5ac
Reviewed-on: https://go-review.googlesource.com/14570
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Keep track of which types of keys need an update and which don't.
Strings need an update because the new key might pin a smaller backing store.
Floats need an update because it might be +0/-0.
Interfaces need an update because they may contain strings or floats.
Fixes#11088
Change-Id: I9ade53c1dfb3c1a2870d68d07201bc8128e9f217
Reviewed-on: https://go-review.googlesource.com/10843
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Convert some fields of struct Type in go.go from uint8 to bool.
This change passes go build -toolexec 'toolstash -cmp' -a std.
Change-Id: I0a6c53f8ee686839b5234010ee2de7ae3940d499
Reviewed-on: https://go-review.googlesource.com/14370
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This helps vet see a real issue:
cmd/internal/gc$ go vet
gen.go:1223: unreachable code
Fixes#12106.
Change-Id: I720868b07ae6b6d5a4dc6b238baa8c9c889da6d8
Reviewed-on: https://go-review.googlesource.com/14083
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
No longer used after previous hashmap change.
Change-Id: I558470f872281e84a78406132df4e391d077b833
Reviewed-on: https://go-review.googlesource.com/13785
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously t.zero always pointed to runtime.zerovalue. Change the hashmap code
to always return a runtime pointer directly, and change that pointer to point
to a larger buffer if one is needed.
(It might be better to only copy from the pointer returned by the mapaccess
functions when the value type is small enough and have the compiler insert
explicit zeroing for larger value types, but I tried and failed to do this).
This removes all uses of the zero field of the type data; the field itself can
be removed in a separate change.
Fixes#11491
Change-Id: I5b81752ff4067d74a5a281c41e88f151bae0171e
Reviewed-on: https://go-review.googlesource.com/13784
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On most systems, a pointer is the worst case alignment, so adding
a pointer field at the end of a struct guarantees there will be no
padding added after that field (to satisfy overall struct alignment
due to some more-aligned field also present).
In the runtime, the map implementation needs a quick way to
get to the overflow pointer, which is last in the bucket struct,
so it uses size - sizeof(pointer) as the offset.
NaCl/amd64p32 is the exception, as always.
The worst case alignment is 64 bits but pointers are 32 bits.
There's a long history that is not worth going into, but when
we moved the overflow pointer to the end of the struct,
we didn't get the padding computation right.
The compiler computed the regular struct size and then
on amd64p32 added another 32-bit field.
And the runtime assumed it could step back two 32-bit fields
(one 64-bit register size) to get to the overflow pointer.
But in fact if the struct needed 64-bit alignment, the computation
of the regular struct size would have added a 32-bit pad already,
and then the code unconditionally added a second 32-bit pad.
This placed the overflow pointer three words from the end, not two.
The last two were padding, and since the runtime was consistent
about using the second-to-last word as the overflow pointer,
no harm done in the sense of overwriting useful memory.
But writing the overflow pointer to a non-pointer word of memory
means that the GC can't see the overflow blocks, so it will
collect them prematurely. Then bad things happen.
Correct all this in a few steps:
1. Add an explicit check at the end of the bucket layout in the
compiler that the overflow field is last in the struct, never
followed by padding.
2. When padding is needed on nacl (not always, just when needed),
insert it before the overflow pointer, to preserve the "last in the struct"
property.
3. Let the compiler have the final word on the width of the struct,
by inserting an explicit padding field instead of overwriting the
results of the width computation it does.
4. For the same reason (tell the truth to the compiler), set the type
of the overflow field when we're trying to pretend its not a pointer
(in this case the runtime maintains a list of the overflow blocks
elsewhere).
5. Make the runtime use "last in the struct" as its location algorithm.
This fixes TestTraceStress on nacl/amd64p32.
The 'bad map state' and 'invalid free list' failures no longer occur.
Fixes#11838.
Change-Id: If918887f8f252d988db0a35159944d2b36512f92
Reviewed-on: https://go-review.googlesource.com/12971
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
A send on an unbuffered channel to a blocked receiver is the only
case in the runtime where one goroutine writes directly to the stack
of another. The garbage collector assumes that if a goroutine is
blocked, its stack contains no new pointers since the last time it ran.
The send on an unbuffered channel violates this, so it needs an
explicit write barrier. It has an explicit write barrier, but not one that
can handle a write to another stack. Use one that can (based on type bitmap
instead of heap bitmap).
To make this work, raise the limit for type bitmaps so that they are
used for all types up to 64 kB in size (256 bytes of bitmap).
(The runtime already imposes a limit of 64 kB for a channel element size.)
I have been unable to reproduce this problem in a simple test program.
Could help #11035.
Change-Id: I06ad994032d8cff3438c9b3eaa8d853915128af5
Reviewed-on: https://go-review.googlesource.com/10815
Reviewed-by: Austin Clements <austin@google.com>
These were found by grepping the comments from the go code and feeding
the output to aspell.
Change-Id: Id734d6c8d1938ec3c36bd94a4dbbad577e3ad395
Reviewed-on: https://go-review.googlesource.com/10941
Reviewed-by: Aamir Khan <syst3m.w0rm@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trivial merging of 5g, 6g, ... into go tool compile,
and similarlly 5l, 6l, ... into go tool link.
The files compile/main.go and link/main.go are new.
Everything else in those directories is a move followed by
change of imports and package name.
This CL breaks the build. Manual fixups are in the next CL.
See golang-dev thread titled "go tool compile, etc" for background.
Change-Id: Id35ff5a5859ad9037c61275d637b1bd51df6828b
Reviewed-on: https://go-review.googlesource.com/10287
Reviewed-by: Dave Cheney <dave@cheney.net>
Reviewed-by: Rob Pike <r@golang.org>
2015-05-21 17:31:51 +00:00
Renamed from src/cmd/internal/gc/reflect.go (Browse further)
