Skip to content

G
golang
Switch branch/tag
  1. 06 Nov, 2017 17 commits
  3. 05 Nov, 2017 5 commits
    • Keith Randall's avatar
      cmd/compile: fix test to use correct go binary · 989cc801
      Keith Randall authored 
      Use internal/testenv package to get the right go binary.
Otherwise, I think we're just grabbing an old one from the environment.

Fixes #22560.

Change-Id: Id5b743b24717e15ec8ffbcfae4dc3e5f6a87b9a9
Reviewed-on: https://go-review.googlesource.com/76090
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarDaniel Martí <mvdan@mvdan.cc>
Reviewed-by: 's avatarDavid Chase <drchase@google.com>
      989cc801
    • Daniel Martí's avatar
      cmd/go: skip "exclude all Go files" error in fmt · e5f6051e
      Daniel Martí authored 
      Otherwise, one can't run "go fmt" on a directory containing Go files if
none of them are buildable (e.g. because of build tags). This is
counter-intuitive, as fmt will format all Go files anyway.

If we encounter such a load error, ignore it and carry on. All other
load errors, such as when a package can't be found, should still be
shown to the user.

Add a test for the two kinds of load errors. Use fmt -n so that any
changes to the formatting of the files in testdata don't actually get
applied. The load errors still occur with -n, so the test does its job.

Fixes #22183.

Change-Id: I99d0c0cdd29015b6a3f5286a9bbff50757c78e0d
Reviewed-on: https://go-review.googlesource.com/75930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      e5f6051e
    • David Chase's avatar
      cmd/compile: adjust locationlist lifetimes · d58d9015
      David Chase authored 
      A statement like

  foo = bar + qux

might compile to

  AX := AX + BX

resulting in a regkill for AX before this instruction.
The buggy behavior is to kill AX "at" this instruction,
before it has executed.  (Code generation of no-instruction
values like RegKills applies their effects at the
next actual instruction emitted).

However, bar is still associated with AX until after the
instruction executes, so the effect of the regkill must
occur at the boundary between this instruction and the
next.  Similarly, the new value bound to AX is not visible
until this instruction executes (and in the case of values
that require multiple instructions in code generation, until
all of them have executed).

The ranges are adjusted so that a value's start occurs
at the next following instruction after its evaluation,
and the end occurs after (execution of) the first
instruction following the end of the lifetime as a value.

(Notice the asymmetry; the entire value must be finished
before it is visible, but execution of a single instruction
invalidates.  However, the value *is* visible before that
next instruction executes).

The test was adjusted to make it insensitive to the result
numbering for variables printed by gdb, since that is not
relevant to the test and makes the differences introduced
by small changes larger than necessary/useful.

The test was also improved to present variable probes
more intuitively, and also to allow explicit indication
of "this variable was optimized out"

Change-Id: I39453eead8399e6bb05ebd957289b112d1100c0e
Reviewed-on: https://go-review.googlesource.com/74090
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: 's avatarCherry Zhang <cherryyz@google.com>
      d58d9015
    • David Chase's avatar
      cmd/compile: repair name propagation into aggregate parts · 38c725b1
      David Chase authored 
      For structs, slices, strings, interfaces, etc, propagation of
names to their components (e.g., complex.real, complex.imag)
is fragile (depends on phase ordering) and not done right
for the "dec" pass.

The dec pass is subsumed into decomposeBuiltin,
and then names are pushed into the args of all
OpFooMake opcodes.

compile/ssa/debug_test.go was fixed to pay attention to
variable values, and the reference files include checks
for the fixes in this CL (which make debugging better).

Change-Id: Ic2591ebb1698d78d07292b92c53667e6c37fa0cd
Reviewed-on: https://go-review.googlesource.com/73210
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarHeschi Kreinick <heschi@google.com>
      38c725b1
    • Hugues Bruant's avatar
      cmd/compile: inline closures with captures · c4b65fa4
      Hugues Bruant authored 
      When inlining a closure with captured variables, walk up the
param chain to find the one that is defined inside the scope
into which the function is being inlined, and map occurrences
of the captures to temporary inlvars, similarly to what is
done for function parameters.

No noticeable impact on compilation speed and binary size.

Minor improvements to go1 benchmarks on darwin/amd64

name                     old time/op    new time/op    delta
BinaryTree17-4              2.59s ± 3%     2.58s ± 1%    ~     (p=0.470 n=19+19)
Fannkuch11-4                3.15s ± 2%     3.15s ± 1%    ~     (p=0.647 n=20+19)
FmtFprintfEmpty-4          43.7ns ± 3%    43.4ns ± 4%    ~     (p=0.178 n=18+20)
FmtFprintfString-4         74.0ns ± 2%    77.1ns ± 7%  +4.13%  (p=0.000 n=20+20)
FmtFprintfInt-4            77.2ns ± 3%    79.2ns ± 6%  +2.53%  (p=0.000 n=20+20)
FmtFprintfIntInt-4          112ns ± 4%     112ns ± 2%    ~     (p=0.672 n=20+19)
FmtFprintfPrefixedInt-4     136ns ± 1%     135ns ± 2%    ~     (p=0.827 n=16+20)
FmtFprintfFloat-4           232ns ± 2%     233ns ± 1%    ~     (p=0.194 n=20+20)
FmtManyArgs-4               490ns ± 2%     484ns ± 2%  -1.28%  (p=0.001 n=20+20)
GobDecode-4                6.68ms ± 2%    6.72ms ± 2%    ~     (p=0.113 n=20+19)
GobEncode-4                5.62ms ± 2%    5.71ms ± 2%  +1.64%  (p=0.000 n=20+19)
Gzip-4                      235ms ± 3%     236ms ± 2%    ~     (p=0.607 n=20+19)
Gunzip-4                   37.1ms ± 2%    36.8ms ± 3%    ~     (p=0.060 n=20+20)
HTTPClientServer-4         61.9µs ± 2%    62.7µs ± 4%  +1.24%  (p=0.007 n=18+19)
JSONEncode-4               12.5ms ± 2%    12.4ms ± 3%    ~     (p=0.192 n=20+20)
JSONDecode-4               51.6ms ± 3%    51.0ms ± 3%  -1.19%  (p=0.008 n=20+19)
Mandelbrot200-4            4.12ms ± 6%    4.06ms ± 5%    ~     (p=0.063 n=20+20)
GoParse-4                  3.12ms ± 5%    3.10ms ± 2%    ~     (p=0.402 n=19+19)
RegexpMatchEasy0_32-4      80.7ns ± 2%    75.1ns ± 9%  -6.94%  (p=0.000 n=17+20)
RegexpMatchEasy0_1K-4       197ns ± 2%     186ns ± 2%  -5.43%  (p=0.000 n=20+20)
RegexpMatchEasy1_32-4      77.5ns ± 4%    71.9ns ± 7%  -7.25%  (p=0.000 n=20+18)
RegexpMatchEasy1_1K-4       341ns ± 3%     341ns ± 3%    ~     (p=0.732 n=20+20)
RegexpMatchMedium_32-4      113ns ± 2%     112ns ± 3%    ~     (p=0.102 n=20+20)
RegexpMatchMedium_1K-4     36.6µs ± 2%    35.8µs ± 2%  -2.26%  (p=0.000 n=18+20)
RegexpMatchHard_32-4       1.75µs ± 3%    1.74µs ± 2%    ~     (p=0.473 n=20+19)
RegexpMatchHard_1K-4       52.6µs ± 2%    52.0µs ± 3%  -1.15%  (p=0.005 n=20+20)
Revcomp-4                   381ms ± 4%     377ms ± 2%    ~     (p=0.067 n=20+18)
Template-4                 57.3ms ± 2%    57.7ms ± 2%    ~     (p=0.108 n=20+20)
TimeParse-4                 291ns ± 3%     292ns ± 2%    ~     (p=0.585 n=20+20)
TimeFormat-4                314ns ± 3%     315ns ± 1%    ~     (p=0.681 n=20+20)
[Geo mean]                 47.4µs         47.1µs       -0.73%

name                     old speed      new speed      delta
GobDecode-4               115MB/s ± 2%   114MB/s ± 2%    ~     (p=0.115 n=20+19)
GobEncode-4               137MB/s ± 2%   134MB/s ± 2%  -1.63%  (p=0.000 n=20+19)
Gzip-4                   82.5MB/s ± 3%  82.4MB/s ± 2%    ~     (p=0.612 n=20+19)
Gunzip-4                  523MB/s ± 2%   528MB/s ± 3%    ~     (p=0.060 n=20+20)
JSONEncode-4              155MB/s ± 2%   156MB/s ± 3%    ~     (p=0.192 n=20+20)
JSONDecode-4             37.6MB/s ± 3%  38.1MB/s ± 3%  +1.21%  (p=0.007 n=20+19)
GoParse-4                18.6MB/s ± 4%  18.7MB/s ± 2%    ~     (p=0.405 n=19+19)
RegexpMatchEasy0_32-4     396MB/s ± 2%   426MB/s ± 8%  +7.56%  (p=0.000 n=17+20)
RegexpMatchEasy0_1K-4    5.18GB/s ± 2%  5.48GB/s ± 2%  +5.79%  (p=0.000 n=20+20)
RegexpMatchEasy1_32-4     413MB/s ± 4%   444MB/s ± 6%  +7.46%  (p=0.000 n=20+19)
RegexpMatchEasy1_1K-4    3.00GB/s ± 3%  3.00GB/s ± 3%    ~     (p=0.678 n=20+20)
RegexpMatchMedium_32-4   8.82MB/s ± 2%  8.90MB/s ± 3%  +0.99%  (p=0.044 n=20+20)
RegexpMatchMedium_1K-4   28.0MB/s ± 2%  28.6MB/s ± 2%  +2.32%  (p=0.000 n=18+20)
RegexpMatchHard_32-4     18.3MB/s ± 3%  18.4MB/s ± 2%    ~     (p=0.482 n=20+19)
RegexpMatchHard_1K-4     19.5MB/s ± 2%  19.7MB/s ± 3%  +1.18%  (p=0.004 n=20+20)
Revcomp-4                 668MB/s ± 4%   674MB/s ± 2%    ~     (p=0.066 n=20+18)
Template-4               33.8MB/s ± 2%  33.6MB/s ± 2%    ~     (p=0.104 n=20+20)
[Geo mean]                124MB/s        126MB/s       +1.54%

Updates #15561
Updates #18270

Change-Id: I980086efe28b36aa27f81577065e2a729ff03d4e
Reviewed-on: https://go-review.googlesource.com/72490Reviewed-by: 's avatarHugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      c4b65fa4
  5. 04 Nov, 2017 7 commits
  7. 03 Nov, 2017 11 commits
    • Russ Cox's avatar
      cmd/go: do not install dependencies during "go install" · 8f70e1f8
      Russ Cox authored 
      This CL makes "go install" behave the way many users expect:
install only the things named on the command line.
Future builds still run as fast, thanks to the new build cache (CL 75473).
To install dependencies as well (the old behavior), use "go install -i".

Actual definitions aside, what most users know and expect of "go install"
is that (1) it installs what you asked, and (2) it's fast, unlike "go build".
It was fast because it installed dependencies, but installing dependencies
confused users repeatedly (see for example #5065, #6424, #10998, #12329,
"go build" and "go test" so that they could be "fast" too, but that only
created new opportunities for confusion. We also had to add -installsuffix
and then -pkgdir, to allow "fast" even when dependencies could not be
installed in the usual place.

The recent introduction of precise content-based staleness logic means that
the go command detects the need for rebuilding packages more often than it
used to, with the consequence that "go install" rebuilds and reinstalls
dependencies more than it used to. This will create more new opportunities
for confusion and will certainly lead to more issues filed like the ones
listed above.

CL 75743 introduced a build cache, separate from the install locations.
That cache makes all operations equally incremental and fast, whether or
not the operation is "install" or "build", and whether or not "-i" is used.

Installing dependencies is no longer necessary for speed, it has confused
users in the past, and the more accurate rebuilds mean that it will confuse
users even more often in the future. This CL aims to end all that confusion
by not installing dependencies by default.

By analogy with "go build -i" and "go test -i", which still install
dependencies, this CL introduces "go install -i", which installs
dependencies in addition to the things named on the command line.

Fixes #5065.
Fixes #6424.
Fixes #10998.
Fixes #12329.
Fixes #18981.
Fixes #22469.

Another step toward #4719.

Change-Id: I3d7bc145c3a680e2f26416e182fa0dcf1e2a15e5
Reviewed-on: https://go-review.googlesource.com/75850
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: 's avatarDavid Crawshaw <crawshaw@golang.org>
      8f70e1f8
    • Russ Cox's avatar
      cmd/go: run vet automatically during go test · 0d188752
      Russ Cox authored 
      This CL adds an automatic, limited "go vet" to "go test".
If the building of a test package fails, vet is not run.
If vet fails, the test is not run.
The goal is that users don't notice vet as part of the "go test"
process at all, until vet speaks up and says something important.
This should help users find real problems in their code faster
(vet can just point to them instead of needing to debug a
test failure) and expands the scope of what kinds of things
vet can help with.

The "go vet" runs in parallel with the linking of the test binary,
so for incremental builds it typically does not slow the overall
"go test" at all: there's spare machine capacity during the link.

all.bash has less spare machine capacity. This CL increases
the time for all.bash on my laptop from 4m41s to 4m48s (+2.5%)

To opt out for a given run, use "go test -vet=off".

The vet checks used during "go test" are a subset of the full set,
restricted to ones that are 100% correct and therefore acceptable
to make mandatory. In this CL, that set is atomic, bool, buildtags,
nilfunc, and printf. Including printf is debatable, but I want to
include it for now and find out what needs to be scaled back.
(It already found one real problem in package os's tests that
previous go vet os had not turned up.)
Now that we can rely on type information it may be that printf
should make its function-name-based heuristic less aggressive
and have a whitelist of known print/printf functions.
Determining the exact set for Go 1.10 is #18085.

Running vet also means that programs now have to type-check
with both cmd/compile and go/types in order to pass "go test".
We don't start vet until cmd/compile has built the test package,
so normally the added go/types check doesn't find anything.
However, there is at least one instance where go/types is more
precise than cmd/compile: declared and not used errors involving
variables captured into closures.

This CL includes a printf fix to os/os_test.go and many declared
and not used fixes in the race detector tests.

Fixes #18084.

Change-Id: I353e00b9d1f9fec540c7557db5653e7501f5e1c9
Reviewed-on: https://go-review.googlesource.com/74356
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: 's avatarRob Pike <r@golang.org>
Reviewed-by: 's avatarDavid Crawshaw <crawshaw@golang.org>
      0d188752
    • Russ Cox's avatar
      cmd/go: cache successful test results · bd95f889
      Russ Cox authored 
      This CL adds caching of successful test results, keyed by the
action ID of the test binary and its command line arguments.

Suppose you run:

	go test -short std
	<edit a typo in a comment in math/big/float.go>
	go test -short std

Before this CL, the second go test would re-run all the tests
for the std packages. Now, the second go test will use the cached
result immediately (without any compile or link steps) for any
packages that do not transitively import math/big, and then
it will, after compiling math/big and seeing that the .a file didn't
change, reuse the cached test results for the remaining packages
without any additional compile or link steps.

Suppose that instead of editing a typo you made a substantive
change to one function, but you left the others (including their
line numbers) unchanged. Then the second go test will re-link
any of the tests that transitively depend on math/big, but it still
will not re-run the tests, because the link will result in the same
test binary as the first run.

The only cacheable test arguments are:

	-cpu
	-list
	-parallel
	-run
	-short
	-v

Using any other test flag disables the cache for that run.
The suggested argument to mean "turn off the cache" is -count=1
(asking "please run this 1 time, not 0").

There's an open question about re-running tests when inputs
like environment variables and input files change. For now we
will assume that users will bypass the test cache when they
need to do so, using -count=1 or "go test" with no arguments.

This CL documents the new cache but also documents the
previously-undocumented distinction between "go test" with
no arguments (now called "local directory mode") and with
arguments (now called "package list mode"). It also cleans up
a minor detail of package list mode buffering that used to change
whether test binary stderr was sent to go command stderr based
on details like exactly how many packages were listed or
how many CPUs the host system had. Clearly the file descriptor
receiving output should not depend on those, so package list mode
now consistently merges all output to stdout, where before it
mostly did that but not always.

Fixes #11193.

Change-Id: I120edef347b9ddd5b10e247bfd5bd768db9c2182
Reviewed-on: https://go-review.googlesource.com/75631
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarDavid Crawshaw <crawshaw@golang.org>
      bd95f889
    • Tobias Klauser's avatar
      math/bits: add examples for right rotation · 89bcbf40
      Tobias Klauser authored 
      Right rotation is achieved using negative k in RotateLeft*(x, k). Add
examples demonstrating that functionality.

Change-Id: I15dab159accd2937cb18d3fa8ca32da8501567d3
Reviewed-on: https://go-review.googlesource.com/75371
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarRobert Griesemer <gri@golang.org>
      89bcbf40
    • Hugues Bruant's avatar
      cmd/compile: fix reassignment check · 483e298d
      Hugues Bruant authored 
      CL 65071 enabled inlining for local closures with no captures.

To determine safety of inlining a call sites, we check whether the
variable holding the closure has any assignments after its original
definition.

Unfortunately, that check did not catch OAS2MAPR and OAS2DOTTYPE,
leading to incorrect inlining when a variable holding a closure was
subsequently reassigned through a type conversion or a 2-valued map
access.

There was another more subtle issue wherein reassignment check would
always return a false positive for closure calls inside other
closures. This was caused by the Name.Curfn field of local variables
pointing to the OCLOSURE node instead of the corresponding ODCLFUNC,
which resulted in reassigned walking an empty Nbody and thus never
seeing any reassignments.

This CL fixes these oversights and adds many more tests for closure
inlining which ensure not only that inlining triggers but also the
correctness of the resulting code.

Updates #15561

Change-Id: I74bdae849c4ecfa328546d6d62b512e8d54d04ce
Reviewed-on: https://go-review.googlesource.com/75770Reviewed-by: 's avatarHugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
      483e298d
    • griesemer's avatar
      go/types: add missing documentation to Object factory functions · d593f85e
      griesemer authored 
      Fixes #22516.

Change-Id: Ib6648cb224e7e85e894263ef79ea81a5850e9a19
Reviewed-on: https://go-review.googlesource.com/75595Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      d593f85e
    • Ian Lance Taylor's avatar
      bytes: set cap of slices returned by Split and Fields and friends · a9e2479a
      Ian Lance Taylor authored 
      This avoids the problem in which appending to a slice returned by
Split can affect subsequent slices.

Fixes #21149.

Change-Id: Ie3df2b9ceeb9605d4625f47d49073c5f348cf0a1
Reviewed-on: https://go-review.googlesource.com/74510Reviewed-by: 's avatarJelte Fennema <github-tech@jeltef.nl>
Reviewed-by: 's avatarRobert Griesemer <gri@golang.org>
      a9e2479a
    • Keith Randall's avatar
      bytes: add more page boundary tests · 3043c355
      Keith Randall authored 
      Make sure Index and IndexByte don't read past the queried byte slice.

Hopefully will be helpful for CL 33597.

Also remove the code which maps/unmaps the Go heap.
Much safer to play with protection bits off-heap.

Change-Id: I50d73e879b2d83285e1bc7c3e810efe4c245fe75
Reviewed-on: https://go-review.googlesource.com/75890Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      3043c355
    • Lynn Boger's avatar
      cmd/compile: add rules to improve consecutive byte loads and stores on ppc64le · bb1fd3b5
      Lynn Boger authored 
      This adds new rules to recognize consecutive byte loads and
stores and lowers them to loads and stores such as lhz, lwz, ld,
sth, stw, std. This change only covers the little endian cases
on little endian machines, such as is found in encoding/binary
UintXX or PutUintXX for little endian. Big endian will be done
later.

Updates were also made to binary_test.go to allow the benchmark
for Uint and PutUint to actually use those functions because
the way they were written, those functions were being
optimized out.

Testcases were also added to cmd/compile/internal/gc/asm_test.go.

Updates #22496

The following improvement can be found in golang.org/x/crypto

poly1305:

Benchmark64-16              142           114           -19.72%
Benchmark1K-16              1717          1424          -17.06%
Benchmark64Unaligned-16     142           113           -20.42%
Benchmark1KUnaligned-16     1721          1428          -17.02%

chacha20poly1305:

BenchmarkChacha20Poly1305Open_64-16     1012       885   -12.55%
BenchmarkChacha20Poly1305Seal_64-16     971        836   -13.90%
BenchmarkChacha20Poly1305Open_1350-16   11113      9539  -14.16%
BenchmarkChacha20Poly1305Seal_1350-16   11013      9392  -14.72%
BenchmarkChacha20Poly1305Open_8K-16     61074      53431 -12.51%
BenchmarkChacha20Poly1305Seal_8K-16     61214      54806 -10.47%

Other improvements of around 10% found in crypto/tls.

Results after updating encoding/binary/binary_test.go:

BenchmarkLittleEndianPutUint64-16     1.87      0.93      -50.27%
BenchmarkLittleEndianPutUint32-16     1.19      0.93      -21.85%
BenchmarkLittleEndianPutUint16-16     1.16      1.03      -11.21%

Change-Id: I7bbe2fbcbd11362d58662fecd907a0c07e6ca2fb
Reviewed-on: https://go-review.googlesource.com/74410
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarMichael Munday <mike.munday@ibm.com>
      bb1fd3b5
    • Hana (Hyang-Ah) Kim's avatar
      runtime/pprof: use new profile format for block/mutex profiles · f99d14e0
      Hana (Hyang-Ah) Kim authored 
      Unlike the legacy text format that outputs the count and the number of
cycles, the pprof tool expects contention profiles to include the count
and the delay time measured in nanoseconds. printCountCycleProfile
performs the conversion from cycles to nanoseconds.
(See parseContention function in
 cmd/vendor/github.com/google/pprof/profile/legacy_profile.go)

Fixes #21474

Change-Id: I8e8fb6ea803822d7eaaf9ecf1df3e236ad225a7b
Reviewed-on: https://go-review.googlesource.com/64410
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      f99d14e0
    • Russ Cox's avatar
      cmd/go: make test binary builds reproducible · 14f2bfd3
      Russ Cox authored 
      The name of the temporary directory containing _testmain.go
was leaking into the binary.

Found with GODEBUG=gocacheverify=1 go test std.

Change-Id: I5b35f049b564f3eb65c6a791ee785d15255c7885
Reviewed-on: https://go-review.googlesource.com/75630
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarDavid Crawshaw <crawshaw@golang.org>
      14f2bfd3