Now only cmd/asm and cmd/compile depend on cmd/internal/obj. Changing
the assembler backends no longer requires reinstalling cmd/link or
cmd/addr2line.
There's also now one canonical definition of the object file format in
cmd/internal/objabi/doc.go, with a warning to update all three
implementations.
objabi is still something of a grab bag of unrelated code (e.g., flag
and environment variable handling probably belong in a separate "tool"
package), but this is still progress.
Fixes#15165.
Fixes#20026.
Change-Id: Ic4b92fac7d0d35438e0d20c9579aad4085c5534c
Reviewed-on: https://go-review.googlesource.com/40972
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
sort.Slice was added in Go 1.8.
It's nice to use, and faster than sort.Sort,
so it'd be nice to be able to use it in the toolchain.
This CL adds obj.SortSlice, which is sort.Slice,
but with a slower fallback version for bootstrapping.
This CL also includes a single demo+test use.
Change-Id: I2accc60b61f8e48c8ab4f1a63473e3b87af9b691
Reviewed-on: https://go-review.googlesource.com/40114
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
pkgByPath was added in d78c84c4 to eliminate the differences between the
export formats around the time of Go 1.7.
The last remnants of the textual export format was removed by Josh in
39850 making the pkgByPath sorting type unused.
Change-Id: I168816d6401f45119475a4fe5ada00d9ce571a9e
Reviewed-on: https://go-review.googlesource.com/40050
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is a re-roll of CL 39710,
which broke deterministic builds.
typenamesym is called from three places:
typename, ngotype, and Type.Symbol.
Only in typename do we actually need a Node.
ngotype and Type.Symbol require only a Sym.
And writing the newly created Node to
Sym.Def is unsafe in a concurrent backend.
Rather than use a mutex protect to Sym.Def,
make typenamesym not touch Sym.Def.
The assignment to Sym.Def was serving a second purpose,
namely to prevent duplicate entries on signatlist.
Preserve that functionality by switching signatlist to a map.
This in turn requires that we sort signatlist
when exporting it, to preserve reproducibility.
We sort using exactly the same mechanism
that the export code (dtypesym) uses.
Failure to do that led to non-deterministic builds (#19872).
Since we've already calculated the Type's export name,
we could pass it to dtypesym, sparing it a bit of work.
That can be done as a future optimization.
Updates #15756
name old alloc/op new alloc/op delta
Template 39.2MB ± 0% 39.3MB ± 0% ~ (p=0.075 n=10+10)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.393 n=10+10)
GoTypes 113MB ± 0% 113MB ± 0% +0.06% (p=0.027 n=10+8)
SSA 1.25GB ± 0% 1.25GB ± 0% +0.05% (p=0.000 n=8+10)
Flate 25.3MB ± 0% 25.3MB ± 0% ~ (p=0.105 n=10+10)
GoParser 31.7MB ± 0% 31.8MB ± 0% ~ (p=0.165 n=10+10)
Reflect 78.2MB ± 0% 78.2MB ± 0% ~ (p=0.190 n=10+10)
Tar 26.6MB ± 0% 26.6MB ± 0% ~ (p=0.481 n=10+10)
XML 42.2MB ± 0% 42.2MB ± 0% ~ (p=0.968 n=10+9)
name old allocs/op new allocs/op delta
Template 384k ± 1% 386k ± 1% +0.43% (p=0.019 n=10+10)
Unicode 320k ± 0% 321k ± 0% +0.36% (p=0.015 n=10+10)
GoTypes 1.14M ± 0% 1.14M ± 0% +0.33% (p=0.000 n=10+8)
SSA 9.69M ± 0% 9.71M ± 0% +0.18% (p=0.000 n=10+9)
Flate 233k ± 1% 233k ± 1% ~ (p=0.481 n=10+10)
GoParser 315k ± 1% 316k ± 1% ~ (p=0.113 n=9+10)
Reflect 979k ± 0% 979k ± 0% ~ (p=0.971 n=10+10)
Tar 250k ± 1% 250k ± 1% ~ (p=0.481 n=10+10)
XML 391k ± 1% 392k ± 0% ~ (p=1.000 n=10+9)
Change-Id: Ia9f21cc29c047021fa8a18c2a3d861a5146aefac
Reviewed-on: https://go-review.googlesource.com/39915
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- created new package cmd/compile/internal/types
- moved Pkg, Sym, Type to new package
- to break cycles, for now we need the (ugly) types/utils.go
file which contains a handful of functions that must be installed
early by the gc frontend
- to break cycles, for now we need two functions to convert between
*gc.Node and *types.Node (the latter is a dummy type)
- adjusted the gc's code to use the new package and the conversion
functions as needed
- made several Pkg, Sym, and Type methods functions as needed
- renamed constructors typ, typPtr, typArray, etc. to types.New,
types.NewPtr, types.NewArray, etc.
Passes toolstash-check -all.
Change-Id: I8adfa5e85c731645d0a7fd2030375ed6ebf54b72
Reviewed-on: https://go-review.googlesource.com/39855
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The textual import/export format is ancient history.
Change-Id: Iebe90bfd9bd3074eb191186d86e5f4286ce3b1f3
Reviewed-on: https://go-review.googlesource.com/39850
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
typenamesym is called from three places:
typename, ngotype, and Type.Symbol.
Only in typename do we actually need a Node.
ngotype and Type.Symbol require only a Sym.
And writing the newly created Node to
Sym.Def is unsafe in a concurrent backend.
Rather than use a mutex protect to Sym.Def,
make typenamesym not touch Sym.Def.
The assignment to Sym.Def was serving a second purpose,
namely to prevent duplicate entries on signatlist.
Preserve that functionality by switching signatlist to a map.
This in turn requires that we sort signatlist
when exporting it, to preserve reproducibility.
We'd like to use Type.cmp for sorting,
but that causes infinite recursion at the moment;
see #19869.
For now, use Type.LongString as the sort key,
which is a complete description of the type.
Type.LongString is relatively expensive,
but we calculate it only once per type,
and signatlist is generally fairly small,
so the performance impact is minimal.
Updates #15756
name old alloc/op new alloc/op delta
Template 39.4MB ± 0% 39.4MB ± 0% ~ (p=0.222 n=5+5)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.151 n=5+5)
GoTypes 113MB ± 0% 113MB ± 0% ~ (p=0.095 n=5+5)
SSA 1.25GB ± 0% 1.25GB ± 0% +0.04% (p=0.008 n=5+5)
Flate 25.3MB ± 0% 25.4MB ± 0% ~ (p=0.056 n=5+5)
GoParser 31.8MB ± 0% 31.8MB ± 0% ~ (p=0.310 n=5+5)
Reflect 78.3MB ± 0% 78.3MB ± 0% ~ (p=0.690 n=5+5)
Tar 26.7MB ± 0% 26.7MB ± 0% ~ (p=0.548 n=5+5)
XML 42.2MB ± 0% 42.2MB ± 0% ~ (p=0.222 n=5+5)
name old allocs/op new allocs/op delta
Template 387k ± 0% 388k ± 0% ~ (p=0.056 n=5+5)
Unicode 320k ± 0% 321k ± 0% +0.32% (p=0.032 n=5+5)
GoTypes 1.14M ± 0% 1.15M ± 0% ~ (p=0.095 n=5+5)
SSA 9.70M ± 0% 9.72M ± 0% +0.18% (p=0.008 n=5+5)
Flate 234k ± 0% 235k ± 0% +0.60% (p=0.008 n=5+5)
GoParser 317k ± 0% 317k ± 0% ~ (p=1.000 n=5+5)
Reflect 982k ± 0% 983k ± 0% ~ (p=0.841 n=5+5)
Tar 252k ± 1% 252k ± 0% ~ (p=0.310 n=5+5)
XML 393k ± 0% 392k ± 0% ~ (p=0.548 n=5+5)
Change-Id: I53a3b95d19cf1a7b7511a94fba896706addf84fb
Reviewed-on: https://go-review.googlesource.com/39710
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It was simply a wrapper around Link.Lookup.
Unwrap everything.
CL prepared using eg with template:
package p
import "cmd/internal/obj"
func before(ctxt *obj.Link, name string, version int) *obj.LSym {
return obj.Linklookup(ctxt, name, version)
}
func after(ctxt *obj.Link, name string, version int) *obj.LSym {
return ctxt.Lookup(name, version)
}
Then one comment in cmd/asm/internal/asm/parse.go
was manually updated (and gofmt'ed!),
and func Linklookup deleted.
Passes toolstash-check (as a sanity measure).
Change-Id: Icc4d56b0b2b5c8888d3184c1898c48359ea1e638
Reviewed-on: https://go-review.googlesource.com/39715
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The node in typenamesym requires neither
a position nor a curfn.
Passes toolstash-check.
Updates #15756
Change-Id: I6d39a8961e5578fe5924aaceb29045b6de2699df
Reviewed-on: https://go-review.googlesource.com/39194
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Convert yyerrors into Fatals.
Remove the goto.
Move variable declaration closer to use.
Unify printing strings a bit.
Convert an int param into a bool.
Passes toolstash-check. No compiler performance impact.
Change-Id: I9017681417b785cf8693d18b124ac4f1ff37f2b5
Reviewed-on: https://go-review.googlesource.com/39170
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Remove one of the many lookup variants.
Change-Id: I4095aa030da4227540badd6724bbf50b728fbe93
Reviewed-on: https://go-review.googlesource.com/38990
Reviewed-by: Dave Cheney <dave@cheney.net>
This makes the overall naming and use of the functions
to create a Type more consistent.
Passes toolstash -cmp.
Change-Id: Ie0d40b42cc32b5ecf5f20502675a225038ea40e4
Reviewed-on: https://go-review.googlesource.com/38354
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
With this change, code like
h := sha1.New()
h.Write(buf)
sum := h.Sum()
gets compiled into static calls rather than
interface calls, because the compiler is able
to prove that 'h' is really a *sha1.digest.
The InterCall re-write rule hits a few dozen times
during make.bash, and hundreds of times during all.bash.
The most common pattern identified by the compiler
is a constructor like
func New() Interface { return &impl{...} }
where the constructor gets inlined into the caller,
and the result is used immediately. Examples include
{sha1,md5,crc32,crc64,...}.New, base64.NewEncoder,
base64.NewDecoder, errors.New, net.Pipe, and so on.
Some existing benchmarks that change on darwin/amd64:
Crc64/ISO4KB-8 2.67µs ± 1% 2.66µs ± 0% -0.36% (p=0.015 n=10+10)
Crc64/ISO1KB-8 694ns ± 0% 690ns ± 1% -0.59% (p=0.001 n=10+10)
Adler32KB-8 473ns ± 1% 471ns ± 0% -0.39% (p=0.010 n=10+9)
On architectures like amd64, the reduction in code size
appears to contribute more to benchmark improvements than just
removing the indirect call, since that branch gets predicted
accurately when called in a loop.
Updates #19361
Change-Id: I57d4dc21ef40a05ec0fbd55a9bb0eb74cdc67a3d
Reviewed-on: https://go-review.googlesource.com/38139
Run-TryBot: Philip Hofer <phofer@umich.edu>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
There were a surprising number of places
in the tree that used yyerror for failed internal
consistency checks. Switch them to Fatalf.
Updates #15756
Updates #19250
Change-Id: Ie4278148185795a28ff3c27dacffc211cda5bbdd
Reviewed-on: https://go-review.googlesource.com/38153
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 4e0c7c3f61.
Reason for revert: The presence-of-optimization test program is fragile, breaks under noopt, and might break if the Go libraries are tweaked. It needs to be (re)written without reference to other packages.
Change-Id: I3aaf1ab006a1a255f961a978e9c984341740e3c7
Reviewed-on: https://go-review.googlesource.com/38097
Reviewed-by: Keith Randall <khr@golang.org>
With this change, code like
h := sha1.New()
h.Write(buf)
sum := h.Sum()
gets compiled into static calls rather than
interface calls, because the compiler is able
to prove that 'h' is really a *sha1.digest.
The InterCall re-write rule hits a few dozen times
during make.bash, and hundreds of times during all.bash.
The most common pattern identified by the compiler
is a constructor like
func New() Interface { return &impl{...} }
where the constructor gets inlined into the caller,
and the result is used immediately. Examples include
{sha1,md5,crc32,crc64,...}.New, base64.NewEncoder,
base64.NewDecoder, errors.New, net.Pipe, and so on.
Some existing benchmarks that change on darwin/amd64:
Crc64/ISO4KB-8 2.67µs ± 1% 2.66µs ± 0% -0.36% (p=0.015 n=10+10)
Crc64/ISO1KB-8 694ns ± 0% 690ns ± 1% -0.59% (p=0.001 n=10+10)
Adler32KB-8 473ns ± 1% 471ns ± 0% -0.39% (p=0.010 n=10+9)
On architectures like amd64, the reduction in code size
appears to contribute more to benchmark improvements than just
removing the indirect call, since that branch gets predicted
accurately when called in a loop.
Updates #19361
Change-Id: Ia9d30afdd5f6b4d38d38b14b88f308acae8ce7ed
Reviewed-on: https://go-review.googlesource.com/37751
Run-TryBot: Philip Hofer <phofer@umich.edu>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
No need to keep as Nodes when they're all Types anyway.
Change-Id: I8157914ba5b09cadf2263247844680a60233a0f2
Reviewed-on: https://go-review.googlesource.com/37886
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes Sym flags consistent with the rest of the code after
the CL 37445.
No functional changes.
Passes toolstash -cmp.
Change-Id: Ica919f2ab98581371c717fff9a70aeb11058ca17
Reviewed-on: https://go-review.googlesource.com/37847
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since switching to SSA, the only remaining use for the Ullman field
was in tracking whether or not an expression contained a function
call. Give it a new name and encode it in our fancy new bitset field.
Passes toolstash-check.
Change-Id: I95b7f9cb053856320c0d66efe14996667e6011c2
Reviewed-on: https://go-review.googlesource.com/37721
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When doing i.(T) for non-empty-interface i and concrete type T,
there's no need to read the type out of the itab. Just compare the
itab to the itab we expect for that interface/type pair.
Also optimize type switches by putting the type hash of the
concrete type in the itab. That way we don't need to load the
type pointer out of the itab.
Update #18492
Change-Id: I49e280a21e5687e771db5b8a56b685291ac168ce
Reviewed-on: https://go-review.googlesource.com/34810
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
The comment for maxPtrmaskBytes implied that the value was still 16,
but that changed in CL 10815.
Change-Id: I86e304bc7d9d1a0a6b22b600fefcc1325e4372d9
Reviewed-on: https://go-review.googlesource.com/36120
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Will also fix type aliases.
Fixes#17766.
For #18130.
Change-Id: I9e1584d47128782152e06abd0a30ef423d5c30d2
Reviewed-on: https://go-review.googlesource.com/35732
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
golang.org/issue/17594 was caused by additab being called more than once for
an itab. golang.org/cl/32131 fixed that by making the itabs local symbols,
but that in turn causes golang.org/issue/18252 because now there are now
multiple itab symbols in a process for a given (type,interface) pair and
different code paths can end up referring to different itabs which breaks
lots of reflection stuff. So this makes itabs global again and just takes
care to only call additab once for each itab.
Fixes#18252
Change-Id: I781a193e2f8dd80af145a3a971f6a25537f633ea
Reviewed-on: https://go-review.googlesource.com/34173
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Introduce R_WEAKADDROFF, a "weak" variation of the R_ADDROFF relocation
that will only reference the type described if it is in some other way
reachable.
Use this for the ptrToThis field in reflect type information where it
is safe to do so (that is, types that don't need to be included for
interface satisfaction, and types that won't cause the compiler to
recursively generate an endless series of ptr-to-ptr-to-ptr-to...
types).
Also fix a small bug in reflect, where StructOf was not clearing the
ptrToThis field of new types.
Fixes#17931
Change-Id: I4d3b53cb9c916c97b3b16e367794eee142247281
Reviewed-on: https://go-review.googlesource.com/33427
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's possible for the pkgPath of a field to be different than that of
the struct type as a whole. In that case, store the field's pkgPath in
the name field. Use the field's pkgPath when setting PkgPath and when
checking for type identity.
Fixes#17952.
Change-Id: Iebaf92f0054b11427c8f6e4158c3bebcfff06f45
Reviewed-on: https://go-review.googlesource.com/33333
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Use a separate symbol for reflect metadata for types with Noalg set.
Fixes#17752.
Change-Id: Icb6cade7e3004fc4108f67df61105dc4085cd7e2
Reviewed-on: https://go-review.googlesource.com/32679
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The exported symbol for a plugin can be the only reference to a
type in a program. In particular, "var F func()" will have
the type *func(), which is uncommon.
Fixes#17140
Change-Id: Ide2104edbf087565f5377374057ae54e0c00c57e
Reviewed-on: https://go-review.googlesource.com/29692
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This feels a bit like a layering violation, but as the bug report shows
it is sometimes necessary.
Fixes#17642
Change-Id: I4ba060bb1ce73a527ce276e5a769c44692b50016
Reviewed-on: https://go-review.googlesource.com/32236
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Otherwise, the way the ELF dynamic linker works means that you can end up with
the same itab being passed to additab twice, leading to the itab linked list
having a cycle in it. Add a test to additab in runtime to catch this when it
happens, not some arbitrary and surprsing time later.
Fixes#17594
Change-Id: I6c82edcc9ac88ac188d1185370242dc92f46b1ad
Reviewed-on: https://go-review.googlesource.com/32131
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of generating typelink symbols in the compiler
mark types that should have typelinks with a flag.
The linker detects this flag and adds the marked types
to the typelink table.
name old s/op new s/op delta
LinkCmdCompile 0.27 ± 6% 0.25 ± 6% -6.93% (p=0.000 n=97+98)
LinkCmdGo 0.30 ± 5% 0.29 ±10% -4.22% (p=0.000 n=97+99)
name old MaxRSS new MaxRSS delta
LinkCmdCompile 112k ± 3% 106k ± 2% -4.85% (p=0.000 n=100+100)
LinkCmdGo 107k ± 3% 103k ± 3% -3.00% (p=0.000 n=100+100)
Change-Id: Ic95dd4b0101e90c1fa262c9c6c03a2028d6b3623
Reviewed-on: https://go-review.googlesource.com/31772
Run-TryBot: Shahar Kohanim <skohanim@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Follow up to CL 29134. Generated with gofmt -r 'Nod -> nod', plus
three manual adjustments to the comments in syntax/parser.go
Change-Id: I02920f7ab10c70b6e850457b42d5fe35f1f3821a
Reviewed-on: https://go-review.googlesource.com/29136
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
After the removal of the old backend many types are no longer referenced
outside internal/gc. Make these functions private so that tools like
honnef.co/go/unused can spot when they become dead code. In doing so
this CL identified several previously public helpers which are no longer
used, so removes them.
This should be the last of the public functions.
Change-Id: I7e9c4e72f86f391b428b9dddb6f0d516529706c3
Reviewed-on: https://go-review.googlesource.com/29134
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
After the removal of the old backend many types are no longer referenced
outside internal/gc. Make these functions private so that tools like
honnef.co/go/unused can spot when they become dead code. In doing so
this CL identified several previously public helpers which are no longer
used, so removes them.
Change-Id: Idc2d485f493206de9d661bd3cb0ecb4684177b32
Reviewed-on: https://go-review.googlesource.com/29133
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Consider repeatedly adding many items to a map
and then deleting them all, as in #16070. The map
itself doesn't need to grow above the high water
mark of number of items. However, due to random
collisions, the map can accumulate overflow
buckets.
Prior to this CL, those overflow buckets were
never removed, which led to a slow memory leak.
The problem with removing overflow buckets is
iterators. The obvious approach is to repack
keys and values and eliminate unused overflow
buckets. However, keys, values, and overflow
buckets cannot be manipulated without disrupting
iterators.
This CL takes a different approach, which is to
reuse the existing map growth mechanism,
which is well established, well tested, and
safe in the presence of iterators.
When a map has accumulated enough overflow buckets
we trigger map growth, but grow into a map of the
same size as before. The old overflow buckets will
be left behind for garbage collection.
For the code in #16070, instead of climbing
(very slowly) forever, memory usage now cycles
between 264mb and 483mb every 15 minutes or so.
To avoid increasing the size of maps,
the overflow bucket counter is only 16 bits.
For large maps, the counter is incremented
stochastically.
Fixes#16070
Change-Id: If551d77613ec6836907efca58bda3deee304297e
Reviewed-on: https://go-review.googlesource.com/25049
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Linker and reflect info generation (reflect.go) relies on formatting
of types (tconv). The fmt.Format based approach introduces extra
allocations, which matter in those cases. Resurrected sconv and tconv
code from commit c85b77c (fmt.go only); and adjusted it slightly.
The formatter-based approach is still used throughout the rest of the
compiler, but reflect.go now uses the tconv method that simply returns
the desired string.
(The timing data below may not be accurate; I've included it only for
comparison with the numbers in issue #16897).
name old time/op new time/op delta
Template 297ms ± 2% 288ms ± 3% -3.12% (p=0.000 n=27+29)
Unicode 155ms ± 5% 150ms ± 5% -3.26% (p=0.000 n=30+30)
GoTypes 1.00s ± 3% 0.95s ± 3% -4.51% (p=0.000 n=28+29)
name old alloc/op new alloc/op delta
Template 46.8MB ± 0% 46.5MB ± 0% -0.65% (p=0.000 n=28+30)
Unicode 37.9MB ± 0% 37.8MB ± 0% -0.24% (p=0.000 n=29+30)
GoTypes 144MB ± 0% 143MB ± 0% -0.68% (p=0.000 n=30+30)
name old allocs/op new allocs/op delta
Template 469k ± 0% 446k ± 0% -5.01% (p=0.000 n=29+30)
Unicode 375k ± 0% 369k ± 0% -1.62% (p=0.000 n=30+28)
GoTypes 1.47M ± 0% 1.37M ± 0% -6.29% (p=0.000 n=30+30)
The code for sconv/tconv in fmt.go now closely match the code from c85b77c
again; except that the functions are now methods. Removing the use of
the bytes.Buffer in tconv and special-caseing interface{} has helped a
small amount as well:
name old time/op new time/op delta
Template 299ms ± 3% 288ms ± 3% -3.83% (p=0.000 n=29+29)
Unicode 156ms ± 5% 150ms ± 5% -3.56% (p=0.000 n=30+30)
GoTypes 960ms ± 2% 954ms ± 3% -0.58% (p=0.037 n=26+29)
name old alloc/op new alloc/op delta
Template 46.6MB ± 0% 46.5MB ± 0% -0.22% (p=0.000 n=30+30)
Unicode 37.8MB ± 0% 37.8MB ± 0% ~ (p=0.075 n=30+30)
GoTypes 143MB ± 0% 143MB ± 0% -0.31% (p=0.000 n=30+30)
name old allocs/op new allocs/op delta
Template 447k ± 0% 446k ± 0% -0.28% (p=0.000 n=30+30)
Unicode 369k ± 0% 369k ± 0% -0.03% (p=0.032 n=30+28)
GoTypes 1.38M ± 0% 1.37M ± 0% -0.35% (p=0.000 n=29+30)
Comparison between c85b77c and now (see issue #16897):
name old time/op new time/op delta
Template 307ms ± 4% 288ms ± 3% -6.24% (p=0.000 n=29+29)
Unicode 164ms ± 4% 150ms ± 5% -8.20% (p=0.000 n=30+30)
GoTypes 1.01s ± 3% 0.95s ± 3% -5.72% (p=0.000 n=30+29)
name old alloc/op new alloc/op delta
Template 46.8MB ± 0% 46.5MB ± 0% -0.66% (p=0.000 n=29+30)
Unicode 37.8MB ± 0% 37.8MB ± 0% -0.13% (p=0.000 n=30+30)
GoTypes 143MB ± 0% 143MB ± 0% -0.11% (p=0.000 n=30+30)
name old allocs/op new allocs/op delta
Template 444k ± 0% 446k ± 0% +0.48% (p=0.000 n=30+30)
Unicode 369k ± 0% 369k ± 0% +0.09% (p=0.000 n=30+28)
GoTypes 1.35M ± 0% 1.37M ± 0% +1.47% (p=0.000 n=30+30)
There's still a small increase (< 1.5%) for GoTypes but pending a complete
rewrite of fmt.go, this seems ok again.
Fixes#16897.
Change-Id: I7e0e56cd1b9f981252eded917f5752259d402354
Reviewed-on: https://go-review.googlesource.com/29087
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- also consistently use %v instead of %s when we have a (gc) Formatter
- rewrite done automatically using Formats test in -u (update) mode
- manual update of format strings that were not single string constants
- updated fmt.go, fmt_test.go accordingly
- fmt_test: permit "%T" always
Change-Id: I8f0704286aba5704600ad0c4a4484005b79b905d
Reviewed-on: https://go-review.googlesource.com/28954
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When possible, emit static data rather than
init functions for interface values.
This:
* cuts 32k off cmd/go
* removes several error values from runtime init
* cuts the size of the image/color/palette compiled package from 103k to 34k
* reduces the time to build the package in #15520 from 8s to 1.5s
Fixes#6289Fixes#15528
Change-Id: I317112da17aadb180c958ea328ab380f83e640b4
Reviewed-on: https://go-review.googlesource.com/26668
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For each exported symbol in package main, add its name and type to
go.plugin.tabs symbol. This is used by the runtime when loading a
plugin to return a typed interface{} value.
Change-Id: I23c39583e57180acb8f7a74d218dae4368614f46
Reviewed-on: https://go-review.googlesource.com/27818
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The compiler was canonicalizing unnamed types of the form
struct { i int }
across packages, even though an unexported field i should not be
accessible from other packages.
The fix requires both qualifying the field name in the string used by
the compiler to distinguish the type, and ensuring the struct's pkgpath
is set in the rtype version of the data when the type being written is
not part of the localpkg.
Fixes#16616
Change-Id: Ibab160b8b5936dfa47b17dbfd48964a65586785b
Reviewed-on: https://go-review.googlesource.com/27791
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
