LookupFieldOrMethod appears as a hotspot when benchmarking gopls'
auto-completion. In particular, instanceLookup.add was allocating in the
common case of structs with no embedding.
This is easily fixed, by using a small array in front of the map inside
of instanceLookup. Do this, and additionally add a microbenchmark.
The benchmark improvement is significant:
name old time/op new time/op delta
LookupFieldOrMethod-12 388µs ± 6% 154µs ± 3% -60.36% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
LookupFieldOrMethod-12 152kB ± 0% 2kB ± 0% -98.77% (p=0.000 n=9+10)
name old allocs/op new allocs/op delta
LookupFieldOrMethod-12 1.41k ± 0% 0.07k ± 0% -95.38% (p=0.000 n=10+10)
It should also be noted that instanceLookup is used elsewhere, in
particular by validType. In those contexts, the scope is not just the
current type but the entire package, and so the newly added buffer is
likely to simply cause extra Identical checks. Nevertheless, those
checks are cheap, and on balance the improved LookupFieldOrMethod
performance leads overall to improved type-checking performance.
Standard library benchmark results varied by package, but type checking
speed for many packages improved by ~5%, with allocations improved by
~10%. If this weren't the case we could let the caller control the
buffer size, but that optimization doesn't seem necessary at this time.
For example:
Check/http/funcbodies/noinfo-12 71.5ms ± 4% 67.3ms ± 2% -5.90% (p=0.000 n=20+20)
Check/http/funcbodies/noinfo-12 244k ± 0% 219k ± 0% -10.36% (p=0.000 n=19+19)
Updates golang/go#53992
Change-Id: I10b6deb3819ab562dbbe1913f12b977cf956dd50
Reviewed-on: https://go-review.googlesource.com/c/go/+/423935
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Using importcfg instead of depending on the existence of .a files for
standard library packages will enable us to remove the .a files in a
future cl.
Change-Id: I6108384224508bc37d82fd990fc4a8649222502c
Reviewed-on: https://go-review.googlesource.com/c/go/+/440222
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
No test case because the problem can only happen for invalid data. Let
the fuzzer find cases like this.
Fixes#56129
Change-Id: I6c81933781384c5e2c8ba0fd99cec50455b9664a
Reviewed-on: https://go-review.googlesource.com/c/go/+/441976
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Since we already provide the error code, the prefix can be deduced
automatically.
Except for the changes in errors.go, the updates were made with
regex find-and-replaces:
check\.error\((.+), InvalidSyntaxTree, invalidAST\+ =>
check.error($1, InvalidSyntaxTree,
check\.errorf\((.+), InvalidSyntaxTree, invalidAST\+ =>
check.errorf($1, InvalidSyntaxTree,
Change-Id: Ia02fc56ac7a8524bdf0c404ff2696435408327e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/441975
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fixes#56150
Change-Id: Id990783562950ba8be7ce9526b7a811625f2190a
Reviewed-on: https://go-review.googlesource.com/c/go/+/442415
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
While we're here, replace a couple uses of os.Environ with cmd.Environ.
For #51317.
Change-Id: Ic5cf4a887a7975a8281223eec0f94df230b6f095
Reviewed-on: https://go-review.googlesource.com/c/go/+/431955
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This change adds the arenas package and a function to reflect for
allocating from an arena via reflection, but all the new API is placed
behind a GOEXPERIMENT.
For #51317.
Change-Id: I026d46294e26ab386d74625108c19a0024fbcedc
Reviewed-on: https://go-review.googlesource.com/c/go/+/423361
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds an API to the runtime for arenas. A later CL can
potentially export it as an experimental API, but for now, just the
runtime implementation will suffice.
The purpose of arenas is to improve efficiency, primarily by allowing
for an application to manually free memory, thereby delaying garbage
collection. It comes with other potential performance benefits, such as
better locality, a better allocation strategy, and better handling of
interior pointers by the GC.
This implementation is based on one by danscales@google.com with a few
significant differences:
* The implementation lives entirely in the runtime (all layers).
* Arena chunks are the minimum of 8 MiB or the heap arena size. This
choice is made because in practice 64 MiB appears to be way too large
of an area for most real-world use-cases.
* Arena chunks are not unmapped, instead they're placed on an evacuation
list and when there are no pointers left pointing into them, they're
allowed to be reused.
* Reusing partially-used arena chunks no longer tries to find one used
by the same P first; it just takes the first one available.
* In order to ensure worst-case fragmentation is never worse than 25%,
only types and slice backing stores whose sizes are 1/4th the size of
a chunk or less may be used. Previously larger sizes, up to the size
of the chunk, were allowed.
* ASAN, MSAN, and the race detector are fully supported.
* Sets arena chunks to fault that were deferred at the end of mark
termination (a non-public patch once did this; I don't see a reason
not to continue that).
For #51317.
Change-Id: I83b1693a17302554cb36b6daa4e9249a81b1644f
Reviewed-on: https://go-review.googlesource.com/c/go/+/423359
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This change makes (*mheap).sysAlloc take an explicit list of hints and a
boolean as to whether or not any newly-created heapArenas should be
registered in the full arena list.
This is a refactoring in preparation for arenas.
For #51317.
Change-Id: I0584a033fce3fcb60c5d0bc033d5fb8bd23b2378
Reviewed-on: https://go-review.googlesource.com/c/go/+/432078
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
No-op change in preparation for arenas.
For #51317.
Change-Id: I0777f21763fcd34957b7e709580cf2b7a962ba67
Reviewed-on: https://go-review.googlesource.com/c/go/+/423365
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change refactors span heap initialization. This change should just
be a no-op and just prepares for adding support for arenas.
For #51317.
Change-Id: Ie6f877ca10f86d26e7b6c4857b223589a351e253
Reviewed-on: https://go-review.googlesource.com/c/go/+/423364
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Dragonfly and FreeBSD both used numerical values for these constants
chosen to be the same as on Solaris. For some reason, NetBSD did not,
and happens to interpret value 0 as P_ALL instead of P_PID
(see 3323ceb782/sys/sys/idtype.h (L43-L44)).
Using the correct value for P_PID should cause wait6 to wait for the
correct process, which may help to avoid the deadlocks reported in
For #50138.
Updates #13987.
Change-Id: I0eacd1faee4a430d431fe48f9ccf837f49c42f39
Reviewed-on: https://go-review.googlesource.com/c/go/+/442478
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Initializes the R3 register with an available address in the stack. The addressed location is used to receive the number of bytes written by WriteFile.
Fixes#56080
Change-Id: I0368eb7a31d2d6a098fa9c26e074eb1114a92704
GitHub-Last-Rev: 23dbdb5378
GitHub-Pull-Request: golang/go#56153
Reviewed-on: https://go-review.googlesource.com/c/go/+/442216
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Skip TestTransportPersistConnLeakShortBody in HTTP/2 mode;
it's flaky and was previously HTTP/1-only.
Don't run TestTransportEventTrace and TestTransportIgnores408
in parallel.
Change-Id: I76bc540fac9317185ef7d414c9deafb35bc926b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/442495
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Delete some unused code, and fix a few warnings from staticcheck.
Change-Id: I3d3a6f13dccffda060449948769c305d93a0389c
Reviewed-on: https://go-review.googlesource.com/c/go/+/441936
Reviewed-by: Bryan Mills <bcmills@google.com>
fakePC uses hash.Sum32, which returns an uint32. However, libfuzzer
trace/hook functions declare fakePC argument as int, causing overflow on
386 archs.
Fixing this by changing fakePC argument to uint to prevent the overflow.
Fixes#56141
Change-Id: I3994c461319983ab70065f90bf61539a363e0a2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/441996
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The errorcalls_test makes sure that we use error instead of errorf
where possible. Copied from types2 and adjusted for go/types.
Change-Id: Ib0572308c87e4415bf89aec8d64e662abe94754b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441958
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Checker.errorf calls now have an error code and thus require at
least 4 arguments.
Change-Id: Id01c30d5d3cc747ab0b3ba4001e88985192f2d80
Reviewed-on: https://go-review.googlesource.com/c/go/+/441957
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This brings go/types error reporting closer to types2.
Except for removing the error functions and one manual correction,
these changes were made by regex-replacing:
check\.invalidAST\((.*), " =>
check.errorf($1, InvalidSyntaxTree, invalidAST+"
check\.invalidOp\((.*), " =>
check.errorf($1, invalidOp+"
check\.invalidArg\((.*), " =>
check.errorf($1, invalidArg+"
A follow-up CL ensures that we use error instead of errorf where
possible.
Change-Id: Iac53dcd9c122b058f98d26d0fb307ef1dfe4e79b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441955
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
When running go install --mod=readonly module@version. modfetch.GoSumFile
was not set, so the checksumOk check that's done when checking whether
we need to set the GoVersion from the go mod file was failing. Bypass
the checksumOk check when there's no GoSumFile.
For #54908
Change-Id: I56cf9d36a505b1223e6bf82a7d455746e2f09849
Reviewed-on: https://go-review.googlesource.com/c/go/+/439855
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Fixes#56156
Change-Id: Ib85ff45f0b0d0eac83c39606ee20b3a312e6e919
Reviewed-on: https://go-review.googlesource.com/c/go/+/442335
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
go list -e -export puts errors running build actions on the load.Package
corresponding to the failed action rather than exiting with a non zero
exit code.
For #25842
Change-Id: I1fea85cc5a0557f514fe9d4ed3b6a858376fdcde
Reviewed-on: https://go-review.googlesource.com/c/go/+/437298
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
As of CL 438347, multiple concurrents calls to Close should be safe.
This removes some indirection and may also make some programs that use
type-assertions marginally more efficient. For example, if a program
calls (*exec.Cmd).StdinPipe to obtain a pipe and then sets that as the
Stdout of another command, that program will now allow the second
command to inherit the file descriptor directly instead of copying
everything through a goroutine.
This will also cause calls to Close after the first to return an error
wrapping os.ErrClosed instead of nil. However, it seems unlikely that
programs will depend on that error behavior: if a program is calling
Write in a loop followed by Close, then if a racing Close occurs it is
likely that the Write would have already reported an error. (The only
programs likely to notice a change are those that call Close — without
Write! — after a call to Wait.)
Updates #56043.
Updates #9307.
Updates #6270.
Change-Id: Iec734b23acefcc7e7ad0c8bc720085bc45988efb
Reviewed-on: https://go-review.googlesource.com/c/go/+/439195
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Delete some unused code, various cleanups to fix staticcheck warnings.
Change-Id: Ie475d57735a83351a4977f0dd4bc1387ce06a20e
Reviewed-on: https://go-review.googlesource.com/c/go/+/441935
Reviewed-by: David Chase <drchase@google.com>
Fold negation into subtraction and avoid double negation.
This removes around 500 instructions from the Go binary on riscv64.
Change-Id: I4aac6c87baa2a0759b180ba87876d488a23df6d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/431105
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Convert SLT/SLTU with a suitably valued constant into a SLTI/SLTIU instruction.
This can reduce instructions and avoid register loads. Now that we generate
more SLTI/SLTIU instructions, absorb these into branches when it makes sense
to do so.
Removes more than 800 instructions from the Go binary on linux/riscv64.
Change-Id: I42c4e00486697acd4da7669d441b5690795f18ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/428499
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Combine masking with a negative value and zero extension into a single
AND operation.
Change-Id: I0b2a735b696d65568839fc4504445eeac3d869a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/428498
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
For #56109
Change-Id: I999763e463fac57732a92f5e396f8fa8c35bd2e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/440297
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
CL 403995 fixed static init of literal contains dynamic exprs, by
ensuring their init are ordered properly. However, we still need to walk
the generated init codes before appending to parent init. Otherwise,
codes that requires desugaring will be unhandled, causing the compiler
backend crashing.
Fixes#56105
Change-Id: Ic25fd4017473f5412c8e960a91467797a234edfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/440455
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Mark the "l1" and "l2" methods on "arenaIdx" with //go:nosplit, since
these methods are called from a nosplit context (for example, from
"spanOf").
Fixes#56044.
Updates #21314.
Change-Id: I48c7aa756b59a13162c89ef21066f83371ae50f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/441859
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This patch fixes up a bug in the inliner's special case code for
coverage counter updates, which was not properly working for
-covermode=atomic compilations.
Updates #56044.
Change-Id: I9e309312b123121c3df02862623bdbab1f6c6a4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441858
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Use InvalidSyntaxError where the zero error code was used before.
Fix a couple of places that didn't set an error code.
Panic in error reporting if no error code is provided.
Change-Id: I3a537d42b720deb5c351bf38871e04919325e231
Reviewed-on: https://go-review.googlesource.com/c/go/+/439566
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Type checkers should use InvalidSyntaxTree as error code
for invalid syntax tree errors instead of zero. This way
the zero value can be used to mark an unset error code.
Also, add an example for BlankPkgName (and adjust the
test harness slightly to make it work).
Change-Id: Ic15fa0e8e46be698e52352f2f0e4915b75e509d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/439565
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds a new dependency to go/types.
Change-Id: I7e40b95c8c1e1356b638fc1aa2ef16ce91e9a496
Reviewed-on: https://go-review.googlesource.com/c/go/+/439563
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
This avoids a conflict when dot-importing this package in
go/types and types2.
Change-Id: Ia6fc45ef21c28ea595b49f5321b5c0d441763e2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/439562
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
The internal/types/errors package defines all error codes used by
the type checkers. This is the 1st step of several that factor out
the error codes from go/types and types2; the package is not yet
used.
- The file codes.go is a copy of go/types/errorcodes.go. The
only change is the updated package name (types -> errors).
- The file codes_test.go is a copy of go/types/errorcodes_test.go
with updated package name (types_test -> errors_test) and minor
changes to walkCodes so that it doesn't require the pkgFiles
helper function (the test only parses a single file).
Change-Id: Idb977b1220737b56b330de1d977f698f022daafc
Reviewed-on: https://go-review.googlesource.com/c/go/+/439560
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
If a generic type declaration is missing a constraint, syntactically
it is an array type declaration with an undefined array length.
Mention the possibility of a missing constraint in the error message
for the undefined array length.
For #56064.
For #55961.
For #51145.
Change-Id: Ic161aeda9ea44faa8aa3bf3e9d62b3b13a95d4c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/439559
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Add "-O2" to all compiler/linker tests. This makes compiler/linker
feature probing better resemble actual compiling later.
Why?
----
zig c++ is a clang front-end[1] that accepts, among other things, the
target over the command line. This command:
zig c++ -target x86_64-linux-gnu main.o -o main
Will:
1. Pre-compile libc++.a.
2. Link the program with libc++.a from (1).
Currently Go only is learning about one flag from the linker, that is,
"--no-gc-sections". The resulting command that tests for the flag
support is this:
c++ -Wl,--no-gc-sections -x c - -o
This causes Zig to pre-compile libc++.a in debug mode. Then the actual
compiler+linker command from CGo adds a few more flags, including "-O2":
c++ <...> -Wl,--no-gc-sections -O2 <...>
From Zig perspective, debug-mode libc++.a is different from the
optimized one; that causes Zig to compile a new libc++.a. Specifically,
Zig adds "-fno-omit-frame-pointer" for debug builds, and
"-fomit-frame-pointer" for optimized builds.
As a result, we have to two sets of libc++.a for every arch/os tuple.
That takes CPU time and a bit of disk space.
Zig performance impact
----------------------
First compilation of a simple CGo application is faster by ~2.5 seconds
or ~60%:
$ CC="zig c++ -target x86_64-linux-gnu.2.28" hyperfine \
--warmup 3 --runs 10 \
--prepare 'rm -fr ~/.cache/zig ~/.cache/go-build /tmp/go-*' \
--parameter-list go go1.19,go1.19-O2 \
'/code/go/bin/{go} build .'
Benchmark 1: /code/go/bin/go1.19 build .
Time (mean ± σ): 6.168 s ± 0.059 s [User: 7.465 s, System: 1.578 s]
Range (min … max): 6.111 s … 6.242 s 10 runs
Benchmark 2: /code/go/bin/go1.19-O2 build .
Time (mean ± σ): 3.816 s ± 0.080 s [User: 4.730 s, System: 1.130 s]
Range (min … max): 3.657 s … 3.958 s 10 runs
Summary
'/code/go/bin/go1.19-O2 build .' ran
1.62 ± 0.04 times faster than '/code/go/bin/go1.19 build .'
If we add C++ to the mix, the difference grows to almost ~23 seconds, or
almost 90%:
$ CC="zig c++ -target x86_64-linux-gnu.2.28" hyperfine \
--warmup 1 --runs 3 \
--prepare 'rm -fr ~/.cache/zig ~/.cache/go-build /tmp/go-*' \
--parameter-list go go1.19,go1.19-O2 \
'/code/go/bin/{go} build .'
Benchmark 1: CC="zig c++ -target x86_64-linux-gnu.2.28" /code/go/bin/go1.19 build .
Time (mean ± σ): 51.137 s ± 0.183 s [User: 234.165 s, System: 15.005 s]
Range (min … max): 50.934 s … 51.288 s 3 runs
Benchmark 2: CC="zig c++ -target x86_64-linux-gnu.2.28" /code/go/bin/go1.19-O2 build .
Time (mean ± σ): 27.102 s ± 0.068 s [User: 119.816 s, System: 8.513 s]
Range (min … max): 27.038 s … 27.174 s 3 runs
Summary
'/code/go/bin/go1.19-O2 build .' ran
1.89 ± 0.01 times faster than '/code/go/bin/go1.19 build .'
The difference is just due to the fact that Zig will not be instructed
to compile libc++.a for debug builds; Go doesn't need that.
Non-Zig performance impact
--------------------------
A.k.a. does "-O2" for this check worsen performance?
No statistically significant performance differences with both clang-15
and gcc-11. Also, it affects only the first compile of a CGo progam, as
the linker tests are cached across invocations. go1.19 binary is the
go1.19 tag; go1.19-O2 is go1.19 + this patch.
$ hyperfine --warmup 3 --runs 20 \
--prepare 'rm -fr ~/.cache/go-build/ /tmp/go-*' \
--parameter-list go go1.19,go1.19-O2 \
--parameter-list cc gcc-11,clang-15 \
'CC={cc} /code/go/bin/{go} build .'
Benchmark 1: CC=gcc-11 /code/go/bin/go1.19 build .
Time (mean ± σ): 681.1 ms ± 13.7 ms [User: 501.6 ms, System: 247.1 ms]
Range (min … max): 654.1 ms … 707.2 ms 20 runs
Benchmark 2: CC=gcc-11 /code/go/bin/go1.19-O2 build .
Time (mean ± σ): 676.8 ms ± 10.2 ms [User: 500.4 ms, System: 245.6 ms]
Range (min … max): 664.4 ms … 696.4 ms 20 runs
Benchmark 3: CC=clang-15 /code/go/bin/go1.19 build .
Time (mean ± σ): 860.1 ms ± 17.1 ms [User: 530.0 ms, System: 394.9 ms]
Range (min … max): 839.4 ms … 920.0 ms 20 runs
Benchmark 4: CC=clang-15 /code/go/bin/go1.19-O2 build .
Time (mean ± σ): 864.5 ms ± 26.6 ms [User: 537.8 ms, System: 390.1 ms]
Range (min … max): 841.9 ms … 955.5 ms 20 runs
Summary
'CC=gcc-11 /code/go/bin/go1.19-O2 build .' ran
1.01 ± 0.03 times faster than 'CC=gcc-11 /code/go/bin/go1.19 build .'
1.27 ± 0.03 times faster than 'CC=clang-15 /code/go/bin/go1.19 build .'
1.28 ± 0.04 times faster than 'CC=clang-15 /code/go/bin/go1.19-O2 build .'
cgo.go
------
package main
// #define _FILE_OFFSET_BITS 64
// #include <unistd.h>
// #include <fcntl.h>
// #include <stdio.h>
// char* hello() { return "hello, world"; }
// void phello() { printf("%s, your lucky number is %p\n", hello(), fcntl); }
import "C"
func main() {
C.phello()
}
func Chello() string {
return C.GoString(C.hello())
}
Alternatives considered
-----------------------
There are a few alternatives:
1. Add "-O2" for linker-only tests. That looks like too much catering to
zig alone. If we can add it, then add for everything.
2. Add "-fomit-frame-pointer" instead of "-O2". This flag does not
universally imply debug mode, thus same argument applies as to (1).
3. Add "-O2" for this particular test (`--no-gc-sections`). This is
brittle and not future-proof: a future linker test may omit this
flag.
Hardware
--------
Tested on a 4-core (8 HT) Intel(R) Core(TM) i7-8665U CPU on Debian 11,
Linux 5.10.0-15-amd64.
[1]: https://andrewkelley.me/post/zig-cc-powerful-drop-in-replacement-gcc-clang.html
Change-Id: I5223a5cf53fc5d2b77ac94a6c5712c32c7fbdf36
GitHub-Last-Rev: 2e998b831a
GitHub-Pull-Request: golang/go#55966
Reviewed-on: https://go-review.googlesource.com/c/go/+/436884
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run "go build ./x" in this workspace:
-- go.work --
use ./y
-- x/go.mod --
module x
go 1.19
-- x/m.go --
package m
It fails with: "go: open /tmp/foo/y/go.mod: no such file or directory".
It's unclear where the name "y" comes from.
This change will emit error like: "go: cannot load module listed in
go.work file: open /tmp/foo/y/go.mod: no such file or directory"
Fixes#55952.
Change-Id: Ia45dd915e3fbd6e33340f352b3d6235c6c31190b
GitHub-Last-Rev: 410de1b4a7
GitHub-Pull-Request: golang/go#56050
Reviewed-on: https://go-review.googlesource.com/c/go/+/438147
Run-TryBot: hopehook <hopehook@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
This permits us to safely support concurrent access to files on Plan 9.
Concurrent access was already safe on other systems.
This does introduce a change: if one goroutine calls a blocking read
on a pipe, and another goroutine closes the pipe, then before this CL
the close would occur. Now the close will be delayed until the blocking
read completes.
Also add tests that concurrent I/O and Close on a pipe are OK.
For #50436
For #56043
Change-Id: I969c869ea3b8c5c2f2ef319e441a56a3c64e7bf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/438347
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
An older version of go compiling a main module that references a
standard library package from a newer release (e.g. net/netip added in
go 1.18) currently produces a confusing error message. This changes adds
a new error message including go version diagnostics.
Fixes#48966
Change-Id: I1e8319dafcf1f67d1b1ca869fe84190c3b3f3c3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/432075
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
With moduleHasRootPackage eliminated in the previous CL, needSum is
now invariantly true at all call sites.
Change-Id: I00e44117e545ea0d3de82604dfa018b013ab8f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/440296
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change moves Linux epoll's syscalls implementation to the
"runtime/internal/syscall" package. The intention in this CL was to
minimise behavioural changes but make the code more generalised. This
also will allow adding new syscalls (like epoll_pwait2) without the
need to implement assembly stubs for each arch.
It also drops epoll_create as not all architectures provide this call.
epoll_create1 was added to the kernel in version 2.6.27 and Go requires
Linux kernel version 2.6.32 or later since Go 1.18. So it is safe to
always use epoll_create1.
This is a resubmit as the previous CL 421994 was reverted due to test
failures after the merge with the master. The issue was fixed in
CL 438615
For #53824
For #51087
Change-Id: I1bd0f23a85b4f9b80178c5dd36fd3e95ff4f9648
Reviewed-on: https://go-review.googlesource.com/c/go/+/440115
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
