Daniel Martí [Sun, 30 Dec 2018 17:46:22 +0000 (18:46 +0100)]
cmd/go: avoid compiling most regexes at init
These regexes are all related to commands like get and build, so they're
unnecessary for simpler commands like env. In particular, we need env to
be fast, since libraries like go/packages call it early and often. Some
external Go tools are interactive, so milliseconds matter.
lazyregexp eagerly compiles the patterns when running from within a test
binary, so there's no longer any need to do that as part of non-test
binaries.
Picking up the low-hanging fruit spotted by 'perf record' shaves off
well over a full millisecond off the benchmark on my laptop:
name old time/op new time/op delta
ExecGoEnv-8 4.92ms ± 1% 3.81ms ± 0% -22.52% (p=0.004 n=6+5)
This CL required adding a few more methods to the lazy regexp wrapper.
Arash Bina [Thu, 7 Feb 2019 02:41:55 +0000 (21:41 -0500)]
crypto/x509: improve error when PKCS1, PKCS8, EC keys are mixed up
Improve error messages if ParsePKCS8PrivateKey/ParseECPrivateKey
/ParsePKCS1PrivateKey or ParsePKIXPublicKey/ParsePKCS1PublicKey
are called erroneously instead of one another.
Juraj Sukop [Wed, 27 Feb 2019 17:43:46 +0000 (17:43 +0000)]
math/big: better initial guess for nat.sqrt
The proposed change introduces a better initial guess which is closer to the final value and therefore converges in fewer steps. Consider for example sqrt(8): previously the guess was 8, whereas now it is 4 (and the result is 2). All this change does is it computes the division by two more accurately while it keeps the guess ≥ √x.
Bryan C. Mills [Wed, 27 Feb 2019 17:14:59 +0000 (12:14 -0500)]
cmd/dist: execute misc/cgo/testso{,var} as regular tests, not host tests
These tests use runtime.GOOS and runtime.GOARCH to determine whether
to run, so must be built and run using the destination's — not the
host's — GOOS and GOARCH.
Updates #30228
Change-Id: I6774dacd01c68b395fca8ca61f70d5879270af8a
Reviewed-on: https://go-review.googlesource.com/c/164117
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Bryan C. Mills [Wed, 27 Feb 2019 15:46:18 +0000 (10:46 -0500)]
misc/cgo/test: skip TestCrossPackageTests on Android
This test currently fails in the Android builders, with the message
pkg_test.go:64: go test -short: exec: "go": executable file not found in $PATH
(https://build.golang.org/log/39ec0da5bfb7793359e199cc8e358ca5a8257840)
I was not able to test this change, because I can't get 'gomote
create' to return an instance of anything Android. However, I will
watch the build dashboard after submitting to verify that the fix
works.
Updates #30228
Android appears to lack a 'go' command in the.
Change-Id: Ieacac7f50d19e2cfef2f5d60e79a159e55b5cfa8
Reviewed-on: https://go-review.googlesource.com/c/164097
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Elias Naur <mail@eliasnaur.com>
Daniel Martí [Sun, 30 Dec 2018 17:43:13 +0000 (18:43 +0100)]
cmd/go: add benchmark that execs 'go env GOARCH'
'go env' is used for many quick operations, such as in go/packages to
query GOARCH and GOMOD. It often is a bottleneck; for example,
go/packages doesn't know whether or not to use Go modules until it has
queried GOMOD.
As such, this go command should be fast. Right now it's slower than it
should be. This commit adds a simple benchmark with os/exec, since we're
particularly interested in the cost of cmd/go's large init function.
Updates #29382.
Change-Id: Ifee6fb9997b9b89565fbfc2739a00c86117b1d37
Reviewed-on: https://go-review.googlesource.com/c/155961
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
erifan01 [Wed, 2 Jan 2019 09:14:26 +0000 (09:14 +0000)]
cmd/compile: optimize math/bits Len32 intrinsic on arm64
Arm64 has a 32-bit CLZ instruction CLZW, which can be used for intrinsic Len32.
Function LeadingZeros32 calls Len32, with this change, the assembly code of
LeadingZeros32 becomes more concise.
Daniel Martí [Wed, 27 Feb 2019 12:09:22 +0000 (13:09 +0100)]
internal/lazyregexp: add a lazy Regexp package
This was implemented as part of go/doc, but it's going to be useful in
other packages. In particular, many packages under cmd/go like web and
vcs make somewhat heavy use of global regexes, which add a non-trivial
amount of init work to the cmd/go program.
A lazy wrapper around regexp.Regexp will make it trivial to get rid of
the extra cost with a trivial refactor, so make it possible for other
packages in the repository to make use of it. While naming the package,
give the members better names, such as lazyregexp.New and
lazyregexp.Regexp.
We're also considering adding some form of a lazy API to the public
regexp package, so this internal package will allow us to get some
initial experience across std and cmd.
Clément Chigot [Wed, 27 Feb 2019 12:49:17 +0000 (13:49 +0100)]
cmd/go: fix -Wl,--whole-archive for aix/ppc64
--whole-archive doesn't exist on AIX. It was already removed most of
the time but was still added with c-archive or c-shared buildmodes.
Change-Id: Ia7360638509d4a4d91674b0281ed4b112508a2c9
Reviewed-on: https://go-review.googlesource.com/c/164037
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Gergely Brautigam [Fri, 18 Jan 2019 20:43:56 +0000 (21:43 +0100)]
cmd/compile: suppress typecheck errors in a type switch case with broken type
If a type switch case expression has failed typechecking, the case body is
likely to also fail with confusing or spurious errors. Suppress
typechecking the case body when this happens.
Fixes #28926
Change-Id: Idfdb9d5627994f2fd90154af1659e9a92bf692c4
Reviewed-on: https://go-review.googlesource.com/c/158617
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Tobias Klauser [Fri, 15 Feb 2019 11:15:11 +0000 (12:15 +0100)]
debug/pe: omit panic in (*File).ImportedSymbols on empty optional headers
If a PE file with invalid optional header size (neither
sizeofOptionalHeader32 nor sizeofOptionalHeader64) is passed to NewFile,
the File.OptionalHeader will be nil which leads to a panic in
(*File).ImportedSymbols().
Prior to this change, DATA instructions accepted
the values 1, 2, 4, and 8 as sizes.
The acceptable sizes were further restricted
to 4 and 8 for float constants.
This was both too restrictive and not restrictive enough:
string constants may reasonably have any length,
and address constants should really only accept pointer-length sizes.
Rob Pike [Wed, 27 Feb 2019 00:41:48 +0000 (11:41 +1100)]
time: rewrite ExampleDuration_Nanoseconds to be more idiomatic.
Fix the punctuation and use the proper units for microseconds,
while explaining the incorrect but common variant 'us'.
Change-Id: I9e96694ef27ab4761efccd8616ac7b6700f60d39
Reviewed-on: https://go-review.googlesource.com/c/163917 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Consistent logic for handling both duplicate map keys and case values,
and eliminates ad hoc value hashing code.
Also makes cmd/compile consistent with go/types's handling of
duplicate constants (see #28085), which is at least an improvement
over the status quo even if we settle on something different for the
spec.
As a side effect, this also suppresses cmd/compile's warnings about
duplicate nils in (non-interface expression) switch statements, which
was technically never allowed by the spec anyway.
Updates #28085.
Updates #28378.
Change-Id: I176a251e770c3c5bc11c2bf8d1d862db8f252a17
Reviewed-on: https://go-review.googlesource.com/c/152544
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Matthew Dempsky [Tue, 26 Feb 2019 23:52:59 +0000 (15:52 -0800)]
cmd/compile/internal/ssa: fix bad CL rebase
CL 142497 renamed "statictmp_N" to ".stmp_N", but missed an instance
that was added by CL 151498 (submitted between the window that CL
142497 was reviewed/tested and later rebased/merged).
Brian Kessler [Wed, 13 Feb 2019 20:18:17 +0000 (13:18 -0700)]
math/big: handle alias of cofactor inputs in GCD
If the variables passed in to the cofactor arguments of GCD (x, y)
aliased the input arguments (a, b), the previous implementation would
result in incorrect results for y. This change reorganizes the calculation
so that the only case that need to be handled is when y aliases b, which
can be handled with a simple check.
Tests were added for all of the alias cases for input arguments and and
and irrelevant test case for a previous binary GCD calculation was dropped.
Fixes #30217
Change-Id: Ibe6137f09b3e1ae3c29e3c97aba85b67f33dc169
Reviewed-on: https://go-review.googlesource.com/c/162517
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Wed, 6 Feb 2019 06:52:03 +0000 (22:52 -0800)]
time: read 64-bit data if available
Also store 64-bit data in lib/time/zoneinfo.zip.
The comments argue that we don't need the 64-bit data until 2037 or
2106, but that turns out not to be the case. We also need them for
dates before December 13, 1901, which is time.Unix(-0x80000000, 0).
Bryan Heden [Thu, 14 Feb 2019 02:35:00 +0000 (02:35 +0000)]
fmt: fix an error in documentation for fmt
Original Printf("%d", hi) obviously doesn't produce
%!d(string=hi) unless somewhere before this code
block you have hi := "hi" somewhere, also this change
maintains consistency with the rest of it
Austin Clements [Sat, 3 Nov 2018 21:23:30 +0000 (17:23 -0400)]
cmd/link: delete stale deadcode reference
Back when the linker did code generation after dead code elimination,
it had to know that references to runtime.read_tls_fallback could be
generated at code generation time (and never appear before that). Now
that code generation is done by the compiler, the references to
runtime.read_tls_fallback are obvious in the relocations, so the
linker no longer needs special knowledge of this symbol.
Tobias Klauser [Fri, 8 Feb 2019 08:25:05 +0000 (09:25 +0100)]
runtime: use hw.ncpuonline sysctl in getncpu on openbsd
The number of CPUs reported by the hw.ncpu sysctl is twice as high as
the actual number of CPUs running on OpenBSD 6.4. with hyperthreading
disabled (hw.smt=0). Try hw.cpuonline first and fall back to hw.ncpu
in case it fails (which is the case on older OpenBSD before 6.4).
Keith Randall [Tue, 22 Jan 2019 18:08:10 +0000 (10:08 -0800)]
cmd/compile: don't bother compiling functions named "_"
They can't be used, so we don't need code generated for them. We just
need to report errors in their bodies.
The compiler currently has a bunch of special cases sprinkled about
for "_" functions, because we never generate a linker symbol for them.
Instead, abort compilation earlier so we never reach any of that
special-case code.
Keith Randall [Mon, 25 Feb 2019 22:51:58 +0000 (14:51 -0800)]
cmd/compile: treat slice pointers as non-nil
var a []int = ...
p := &a[0]
_ = *p
We don't need to nil check on the 3rd line. If the bounds check on the 2nd
line passes, we know p is non-nil.
We rely on the fact that any cap>0 slice has a non-nil pointer as its
pointer to the backing array. This is true for all safely-constructed slices,
and I don't see any reason why someone would violate this rule using unsafe.
Keith Randall [Tue, 15 Jan 2019 23:00:43 +0000 (15:00 -0800)]
cmd/compile: update comment about x86 nop instruction generator
The comment about losing the high bits is incorrect. We now use these
nops in places where they really need to be a nop. (Before inline
marks, we used them just before deferreturn calls, so they could
clobber any caller-saved values.)
Alex Brainman [Fri, 25 Jan 2019 07:56:22 +0000 (18:56 +1100)]
runtime: fix syscall.NewCallback to return all bits for uintptr values
syscall.NewCallback mistakenly used MOVL even for windows/amd64,
which only returned the lower 32 bits regardless of the architecture.
This was due to a copy and paste after porting from windows/386.
The code now uses MOVQ, which will return all the available bits.
Also adjust TestReturnAfterStackGrowInCallback to ensure we never
regress.
Russ Cox [Wed, 30 Jan 2019 05:49:33 +0000 (00:49 -0500)]
math/big: add %x float format
big.Float already had %p for printing hex format,
but that format normalizes differently from fmt's %x
and ignores precision entirely.
This CL adds %x to big.Float, matching fmt's behavior:
the verb is spelled 'x' not 'p', the mantissa is normalized
to [1, 2), and precision is respected.
See golang.org/design/19308-number-literals for background.
For #29008.
Change-Id: I9c1b9612107094856797e5b0b584c556c1914895
Reviewed-on: https://go-review.googlesource.com/c/160249 Reviewed-by: Robert Griesemer <gri@golang.org>
Daniel Martí [Sun, 16 Dec 2018 18:11:28 +0000 (19:11 +0100)]
encoding/base32: simplify and speed up decoder
First, we can lift the enc.decodeMap nil check out of the loop.
Second, we can make it clear to the compiler that 'in := src[0]' doesn't
need a bounds check, by making len(src)==0 a single if check that always
stops the loop. This is by far the largest speed-up.
Third, we can use a dst slice index instead of reslicing dst, which
removes work from the loop body.
While at it, we can merge the two 'switch dlen' pieces of code, which
simplifies the code and doesn't affect performance.
name old time/op new time/op delta
DecodeString-8 80.2µs ± 0% 67.5µs ± 0% -15.81% (p=0.002 n=6+6)
name old speed new speed delta
DecodeString-8 163MB/s ± 0% 194MB/s ± 0% +18.78% (p=0.002 n=6+6)
Agniva De Sarker [Mon, 18 Feb 2019 09:13:06 +0000 (14:43 +0530)]
go/doc: skip escaping comments in pre-formatted blocks
CL 150377 made the change of converting smart quotes to their html escaped entities
for ToHTML, and to unicode quotes for ToText. But for ToText, the change
converted the quotes in pre-formatted text too.
This fixes that behavior to not touch any text in pre-formatted blocks, which also
makes the behavior consistent with ToHTML.
Fixes #29730
Change-Id: I58e0216cbdbe189d06d82147e5a02b620af14734
Reviewed-on: https://go-review.googlesource.com/c/162922
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Marat Khabibullin [Wed, 13 Feb 2019 19:19:33 +0000 (19:19 +0000)]
net/textproto: prevent test from failing with nil pointer dereference
The variable err could have nil value when we call err.Error(),
because after we check it for nil above we continue the test
(t.Errorf doesn't stop the test execution).
The special case for ODOTPTR to handle zero-width fields is unneeded.
It is an artifact of the old backend, from which time this code dates.
The Node to SSA converter is careful to insert a nil check.
This is tested in test/nilptr2.go, among other places.
Daniel Martí [Sat, 9 Feb 2019 17:50:02 +0000 (17:50 +0000)]
text/template: error on method calls on nil interfaces
Trying to call a method on a nil interface is a panic in Go. For
example:
var stringer fmt.Stringer
println(stringer.String()) // nil pointer dereference
In https://golang.org/cl/143097 we started recovering panics encountered
during function and method calls. However, we didn't handle this case,
as text/template panics before evalCall is ever run.
In particular, reflect's MethodByName will panic if the receiver is of
interface kind and nil:
panic: reflect: Method on nil interface value
Simply add a check for that edge case, and have Template.Execute return
a helpful error. Note that Execute shouldn't just error if the interface
contains a typed nil, since we're able to find a method to call in that
case.
Finally, add regression tests for both the nil and typed nil interface
cases.
Elias Naur [Mon, 25 Feb 2019 10:18:03 +0000 (11:18 +0100)]
misc/android: copy testdata directories to device before running
We've got away with not copying the testdata directories for the
standard library because the exec wrapper also pushes almost the
entire $GOROOT tree to the device, including testdata directories.
Elias Naur [Mon, 25 Feb 2019 09:52:42 +0000 (10:52 +0100)]
misc/android: serialize adb commands on android emulators
Android emulator builders are soon to join the trybot set. To avoid
flaky runs, work around a longstanding adb bug where concurrent adb
commands sometimes fail.
I haven't seen the problem on actual devices until recently. It seems
that the recently added "adb wait-for-device" can introduce flakyness
with errors such as:
adb: error: failed to get feature set: protocol fault (couldn't read status): Connection reset by peer
Instead of working around that, give up and serialize use of adb
everywhere.
Elias Naur [Sun, 24 Feb 2019 14:18:02 +0000 (15:18 +0100)]
misc/android,cmd/dist: move $GOROOT copying to the exec wrapper
To run the standard library tests on Android, the androidtest.bash
script copies GOROOT to the device. Move that logic to the android
exec wrapper, thereby making androidtest.bash obsolete.
Apart from making Android less special, the sharded builder
infrastructure should now be able to run (emulated) Android builders
and trybots without special treatment.
Elias Naur [Sun, 24 Feb 2019 12:18:13 +0000 (13:18 +0100)]
cmd/dist: build exec wrappers during bootstrap
The androidtest.bash script encodes the additional steps to build
Go and run tests on Android. In order to add sharded builders and
trybots, Android needs to fit into the usual make.bash + cmd/dist test
pattern.
This change moves building the exec wrapper into cmd/dist bootstrap.
Michael Munday [Thu, 24 Jan 2019 17:27:23 +0000 (17:27 +0000)]
math/bits: optimize Reverse32 and Reverse64
Use ReverseBytes32 and ReverseBytes64 to speed up these functions.
The byte reversal functions are intrinsics on most platforms and
generally compile to a single instruction.
name old time/op new time/op delta
Reverse32 2.41ns ± 1% 1.94ns ± 3% -19.60% (p=0.000 n=20+19)
Reverse64 3.85ns ± 1% 2.56ns ± 1% -33.32% (p=0.000 n=17+19)
Change-Id: I160bf59a0c7bd5db94114803ec5a59fae448f096
Reviewed-on: https://go-review.googlesource.com/c/159358
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Daniel Martí [Sun, 9 Dec 2018 17:35:21 +0000 (17:35 +0000)]
text/template: improve nil errors in evalField
If we're accessing a field on a nil struct pointer, and that field is
present in the type, we should print a "nil pointer evaluating X.Y" error
instead of the broader "can't evaluate field Y in X". The latter error
should still be used for the cases where the field is simply missing.
While at it, remove the isNil checks in the struct and map cases. The
indirect func will only return a true isNil when returning a pointer or
interface reflect.Value, so it's impossible for either of these checks
to be useful.
Finally, extend the test suite to test a handful of these edge cases,
including the one shown in the original issue.
Fixes #29137.
Change-Id: I53408ced8a7b53807a0a8461b6baef1cd01d25ae
Reviewed-on: https://go-review.googlesource.com/c/153341
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>