Introduces a parallel OutBuf implementation, and POC on amd64. Due to
some of the weird behaviors I saw on MacOS (SIGBUS while calling msync),
I will wait for feedback to port to other architectures.
On my mac, sped up Asmb by ~78% for cmd/compile (below). Will likely
have an appreciable speedup on kubelet benchmark.
Asmb 39.1ms ±11% 8.5ms ±10% -78.17% (p=0.000 n=10+9)
TotalTime 596ms ± 2% 577ms ± 8% -3.07% (p=0.034 n=8+10)
Change-Id: Id2a2577c3f4da155d8dccc862897f43b941877ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/223742
Run-TryBot: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reorganize the linker phase ordering so that addexport() runs before
loadlibfull. In previous CLs addexport() was changed to use loader
APIs but then copy back its work into sym.Symbol, so this change
removes the copying/shim code in question.
Change-Id: I17314a90007909e6242ee00e26393f3e4a02cf25
Reviewed-on: https://go-review.googlesource.com/c/go/+/226362
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Add some new loader.Sym equivalents to the archSyms struct so that we
can run setArchSyms earlier in the pipeline, and add a "mode" to
setArchSyms to control whether it should create loader.Sym symbols or
their *sym.Symbol equivalents.
These change needed for a subsequent patch in which addexport() is run
earlier as well
Change-Id: I0475c9388c39f13e045dd4aa9c90eaec42624810
Reviewed-on: https://go-review.googlesource.com/c/go/+/226361
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Add support for migrating over the symbol Dynid property
when converting loader.Sym symbols to sym.Symbol.
Change-Id: Icc3b91b4adcae6f2ede7d915bb674cc206025217
Reviewed-on: https://go-review.googlesource.com/c/go/+/226360
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This gives some small speedup:
(linking cmd/compile)
Loadlibfull 77.8ms ± 6% 68.6ms ± 5% -11.88% (p=0.008 n=5+5)
Also convert some Relocs.At to At2, which should have been done
earlier.
Change-Id: I2a66aeb5857234c6e645e1b23380149cffc8221f
Reviewed-on: https://go-review.googlesource.com/c/go/+/226363
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
When using the new(old) linker but an old(new) object file is
found, give a better error message.
Change-Id: I94786f1a4b527c15c4f5b00457eab60d215a72a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/225457
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Add accessor methods to get at the symbol {plt,got} value for
PE symbols. Fix a bug in the loaders SetPlt/SetGot methods.
Change-Id: I975bd6b86122622b206487c8798f8290ecd25a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/225199
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The only merge conflict is the addition of -spectre flag on
master and the addition of -go115newobj flag on dev.link.
Resolved trivially.
Change-Id: I5b46c2b25e140d6c3d8cb129acbd7a248ff03bb9
There might be some concurrent (maybe not concurrent, just sequential but in a short time window) and duplicate calls to `netpollBreak`, trying to wake up a net-poller. If one has called `netpollBreak` and that waking event hasn't been received by epollwait/kevent/..., then the subsequent calls of `netpollBreak` ought to be ignored or in other words, these calls should be converged into one.
Benchmarks go1.13.5 darwin/amd64:
benchmark-func time/op (old) time/op (new) delta
BenchmarkNetpollBreak-4 29668ns ±1% 3131ns ±2% -89.45%
mem/B (old) mem/B (new) delta
154B ±13% 0B ±0% -100%
Change-Id: I3cf757a5d6edc5a99adad7aea3baee4b7f2a8f5c
GitHub-Last-Rev: 15bcfbab8a
GitHub-Pull-Request: golang/go#36294
Reviewed-on: https://go-review.googlesource.com/c/go/+/212737
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Installing to GOROOT makes tests non-parallelizable, since each test
depends on the installed contents of GOROOT already being up-to-date
and may reasonably assume that those contents do not change over the
course of the test.
Fixes#37573
Updates #30316
Change-Id: I2afe95ad11347bee3bb7c2d77a657db6d691cf05
Reviewed-on: https://go-review.googlesource.com/c/go/+/225897
Reviewed-by: Michael Matloob <matloob@golang.org>
https://golang.org/doc/contribute.html#quick_test currently suggests
running 'make.bash' and 'run.bash' separately, but 'run.bash'
potentially uses a 'go' command resolved from the wrong GOROOT,
which in turn sets the wrong GOROOT for further commands.
Updates #32674
Updates #17896
Change-Id: I4925d478d0fc7351c4f6d40830ab17d4d688348d
Reviewed-on: https://go-review.googlesource.com/c/go/+/223741
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This reverts CL 223257.
Reason for revert: broke TestScript/mod_gonoproxy on the longtest builders.
Change-Id: I8637c52c5a7d5333a37ed1e9998c49786525ecb1
Reviewed-on: https://go-review.googlesource.com/c/go/+/225757
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The plan9-arm builder has a very slow filesystem and frequently times
out on this test. The api check verifies the API for all supported
GOOS/GOARCH/CGO_ENABLED combination anyway, so if we skip it on one
builder (or even most builders) there should be no loss of coverage.
Updates #37951
Change-Id: I86a93df2ec60a6af6d942e3954eef09ce67bb39e
Reviewed-on: https://go-review.googlesource.com/c/go/+/225662
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Currently, in LoadFull we migrate a symbol's Value to sym.Symbol
only for external symbols. And symbol's Align is not migrated at
all. As we move LoadFull forward, there are already places where
we set symbol's Value and Align (e.g. in doelf). Migrate them
correctly.
Currently I think we only set them on external symbols, but as
we move forward I think we'll need to set them on Go symbols as
well.
Change-Id: I63e97e38fc08b653ba9faefe15697944faf21bed
Reviewed-on: https://go-review.googlesource.com/c/go/+/225658
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
URLs in GOPROXY may now be separated with commas (,) or pipes (|). If
a request to a proxy fails with any error (including connection errors
and timeouts) and the proxy URL is followed by a pipe, the go command
will try the request with the next proxy in the list. If the proxy is
followed by a comma, the go command will only try the next proxy if
the error a 404 or 410 HTTP response.
The go command will determine how to connect to the checksum database
using the same logic. Before accessing the checksum database, the go
command sends a request to <proxyURL>/sumdb/<sumdb-name>/supported.
If a proxy responds with 404 or 410, or if any other error occurs and
the proxy URL in GOPROXY is followed by a pipe, the go command will
try the request with the next proxy. If all proxies respond with 404
or 410 or are configured to fall back on errors, the go command will
connect to the checksum database directly.
This CL does not change the default value or meaning of GOPROXY.
Fixes#37367
Change-Id: If53152ec1c3282c67d4909818b666af58884fb2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/223257
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
EnvForDir does not immediately evoke “append”, and thus may not prompt
the reader to consider the possibility of aliasing bugs (as in
issue #38077). To make this behavior more obvious at the call site, rename
cmd/go/internal/base.EnvForDir to AppendPWD and swap the order of
arguments to a conventional “append” function (similar to those in the
strconv package).
For #38077
Change-Id: I16f09aa0fa8a269d51f0511eb402a44e2759eb94
Reviewed-on: https://go-review.googlesource.com/c/go/+/225578
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Appending to a global slice is only safe if its length is already
equal to its capacity. That property is not guaranteed for slices in
general, and empirically does not hold for this one.
This is a minimal fix to make it easier to backport.
A more robust cleanup of the base.EnvForDir function will be sent in a
subsequent CL.
Fixes#38077
Updates #37940
Change-Id: I731d5bbd0e516642c2cf43e713eeea15402604e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/225577
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Currently, the capping logic for the GC trigger ratio is such that if
gcpercent is low, we may end up setting the trigger ratio far too high,
breaking the promise of SetGCPercent and GOGC has a trade-off knob (we
won't start a GC early enough, and we will use more memory).
This change modifies the capping logic for the trigger ratio by scaling
the minTriggerRatio with gcpercent the same way we scale
maxTriggerRatio.
Fixes#37927.
Change-Id: I2a048c1808fb67186333d3d5a6bee328be2f35da
Reviewed-on: https://go-review.googlesource.com/c/go/+/223937
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.
interface {
Equal(crypto.PublicKey) bool
}
Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.
Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.
Re-landing of CL 223754. Dropped the test that was assuming named curves
are not implemented by CurveParams, because it's not true for all
curves, and anyway is not a property we need to test. There is still a
test to check that different curves make keys not Equal.
Fixes#21704Fixes#38035
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
Change-Id: I736759b145bfb4f7f8eecd78c324315d5a05385c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225460
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As we now have -go115newobj flag, it is better to use go115 in
the object file as well. And it already diverges from the go114
"new" object file format.
Change-Id: I315edf7524158b5c354393fe9a7ab9f6d7cc9808
Reviewed-on: https://go-review.googlesource.com/c/go/+/225458
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This should restore deterministic order of dynexp, and fix
Solaris build.
Change-Id: Icb796babaa3238bff90fd8255ee9f023f2306c26
Reviewed-on: https://go-review.googlesource.com/c/go/+/225538
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
On Solaris, in the runtime it defines the external name of
runtime.etext as _etext (runtime/os3_solaris.go:13). In CL 224939
we changed to put external names in the ELF symbol table more
consistently. In this case it will contain _etext but not
runtime.etext.
To be conservative, this CL defines both runtime.etext and _text
in the linker.
Change-Id: I79f196e87b655042be97b0fbbab02d0ebc8db2fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/225537
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Variable 'procs' used to calculate the threshold of overuse in
TestPhysicalMemoryUtilization should be updated if GOMAXPROCS
gets changed, otherwise the threshold could be a large number,
making the test meaningless.
Change-Id: I876cbf11457529f56bae77af1e35f4538a721f95
Reviewed-on: https://go-review.googlesource.com/c/go/+/210297
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Make the code more consistent with the rest of the file.
Should have caught this in review of CL 225357.
Change-Id: I12824cb436539c31604684e043ebb7587cc92471
Reviewed-on: https://go-review.googlesource.com/c/go/+/225557
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Write the test output to the temporary directory, not the current
directory.
May fix linux-mips64le-mengzhuo builder.
Change-Id: Ibfeb3d2879c11d498abc31df4efe776fc09a6ad6
Reviewed-on: https://go-review.googlesource.com/c/go/+/225440
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Currently if a goroutine is preempted while owning a timer in the
timerModifying state, it could self-deadlock. When the goroutine is
preempted and calls into the scheduler, it could call checkTimers. If
checkTimers encounters the timerModifying timer and calls runtimer on
it, then runtimer will spin, waiting for that timer to leave the
timerModifying state, which it never will.
So far we got lucky that for the most part that there were no preemption
points while timerModifying is happening, however CL 221077 seems to
have introduced one, leading to sporadic self-deadlocks.
This change disables preemption explicitly while a goroutines holds a
timer in timerModifying. Since only checkTimers (and thus runtimer) is
called from the scheduler, this is sufficient to prevent
preemption-based self-deadlocks.
Fixes#38070.
Updates #37894.
Change-Id: Idbfac310889c92773023733ff7e2ff87e9896f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This partially undoes the optimizations of CL 177597, but makes up
some of the difference by caching the package list and import metadata
and making the initial calls concurrently, including in TestMain.
That reduces the critical path from two sequential 'go list'
invocations to just one (run many times concurrently), and eliminates
the need for assumptions about the consistency of the 'std' dependency
graph across platforms (and hard-coded special cases for packages that
violate those assumptions).
In the process, this simplifies and fixes TestBenchmark (which has
been silently broken since CL 164623).
This increases 'time go tool dist test api' on my workstation from
0m8.4s / 0m13.8s / 0m1.7s to 0m10.5s / 0m23.1s / 0m5.1s,
compared to 0m12.4s / 0m23.2s / 0m4.7s before CL 177597.
(That is, this change retains about half of the wall-time speedup, but
almost none of the user-time speedup.)
Tested manually using 'go test -race -bench=. cmd/api'.
Fixes#37951
Change-Id: Icd537e035e725e1ee7c41d97da5c6651233b927e
Reviewed-on: https://go-review.googlesource.com/c/go/+/224619
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
On arm64, a function's address is 16 bytes aligned, and
the assembler aligns the size of function symbol to 16 bytes,
so to keep the consistent, this patch changes the function
alignment in the linker to 16 bytes.
Change-Id: I4d1e89a56200453b7b586fe3f4656bada7544214
Reviewed-on: https://go-review.googlesource.com/c/go/+/225397
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The lookup is only used for DWARF section symbols, for which we
can just link the symbols to the sections.
Change-Id: Id8426fbf59bab2528f57e28e2043e0b405656a9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/225204
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
It is needed in Xcoffadddynrel. And it will be needed once we
move more things to new style (the sym.Symbol parameters will
also need to change).
Change-Id: Ie12683f9b44e21f1e6ea711bf2f4c5c32282e5b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/225203
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Recently, we get requirements of instructions and functions alignment
from the gVisor project. To fit the alignment requirement of interrupt
table, they require an instruction's address to be aligned 128 bytes
and a function's entry address to be aligned 2K bytes. Thus we add
support for PCALIGN directive first. Below is a discussion about this
topic. https://groups.google.com/forum/m/#!topic/golang-dev/RPj90l5x86I
Functions in Go are aligned to 16 bytes on arm64, thus now we only
support 8 and 16 bytes alignment.
This patch adds support for PCALIGN directive. This directive can be
used within Go asm to align instruction by padding NOOP directives.
This patch also adds a test to verify the correnctness of the PCALIGN
directive. The test is contributed by Fannie Zhang <Fannie.Zhang@arm.com>.
Change-Id: I709e6b94847fe9e1824f42f4155355f90c63d523
Reviewed-on: https://go-review.googlesource.com/c/go/+/207117
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The "runindir" tests used "go run", but relied on relative imports
(which are not supported by "go run" in module mode). Instead, such
tests must use fully-qualified imports, which require either a go.mod
file (in module mode) or that the package be in an appropriate
subdirectory of GOPATH/src (in GOPATH mode).
To set up such a directory, we use yet another copy of the same
overlayDir function currently found in the misc subdirectory of this
repository.
Fixes#33912
Updates #30228
Change-Id: If3d7ea2f7942ba496d98aaaf24a90bcdcf4df9f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/225205
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The load and test instructions compare the given value
against zero and will produce a condition code indicating
one of the following scenarios:
0: Result is zero
1: Result is less than zero
2: Result is greater than zero
3: Result is not a number (NaN)
The instruction can be used to simplify floating point comparisons
against zero, which can enable further optimizations.
This CL also reduces the size of .text section of math.test binary by around
0.7 KB (in hexadecimal, from 1358f0 to 135620).
Change-Id: I33cb714f0c6feebac7a1c46dfcc735e7daceff9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/209159
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Convert the linker's "addexport" function to use loader interfaces
for symbol manipulation instead of *sym.Symbol.
At the moment "addexport" happens after loadlibfull (there are other
chunks of functionality in the way that haven't been converted), so
this implementation contains temporary shim code to copy back the
contents of updated loader.Sym's into the corresponding sym.Symbol.
Change-Id: I867b08e66562a2bed51560fd0be2cb64d344709c
Reviewed-on: https://go-review.googlesource.com/c/go/+/224384
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Populate ArchSyms with loader.Sym versions of important symbols,
so as to be able to convert the code that uses ArchSyms to the
new loader interfaces.
Change-Id: If6766f7164642a9dc2e31fcf7f9280a6e95e5d23
Reviewed-on: https://go-review.googlesource.com/c/go/+/224383
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rewrite setupdynexp to work with loader.Sym, and move the call to it
before the body of loadlibfull. After loadlibfull is complete,
construct the old *sym.Symbol version of dynexp, since not all all
clients that access this list are converted to the loader APIs.
Change-Id: I347d24958e2f3e2332fbe33f2eb6ec25cc126bdb
Reviewed-on: https://go-review.googlesource.com/c/go/+/224382
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Update PropagateLoaderChangesToSymbols so that it no longer requires
a sym.Symbols pointer. The intent is to generalize it a little to
allow it to be used in more than just linker Dwarf generation.
Change-Id: I9bddc5d39839eacd9113c945bb59d2873c0b088c
Reviewed-on: https://go-review.googlesource.com/c/go/+/224381
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Add an Errorf method to ErrorReporter. The hope is that we can
consolidate error handling/reporting in this helper, and eventually
do away with Link.Errorf and the global Errorf function (which
can be removed once we've eliminated enough uses of *sym.Symbol).
Change-Id: Ie1147020b8409b9c57acfd71c942b287b214afca
Reviewed-on: https://go-review.googlesource.com/c/go/+/224380
Reviewed-by: Jeremy Faller <jeremy@golang.org>
