Austin Clements [Wed, 5 Oct 2016 22:32:21 +0000 (18:32 -0400)]
runtime: make markrootSpans time proportional to in-use spans
Currently markrootSpans iterates over all spans ever allocated to find
the in-use spans. Since we now have a list of in-use spans, change it
to iterate over that instead.
This, combined with the previous change, fixes #9265. Before these two
changes, blowing up the heap to 8GB and then shrinking it to a 0MB
live set caused the small-heap portion of the test to run 60x slower
than without the initial blowup. With these two changes, the time is
indistinguishable.
Austin Clements [Wed, 5 Oct 2016 21:50:39 +0000 (17:50 -0400)]
runtime: make sweep time proportional to in-use spans
Currently sweeping walks the list of all spans, which means the work
in sweeping is proportional to the maximum number of spans ever used.
If the heap was once large but is now small, this causes an
amortization failure: on a small heap, GCs happen frequently, but a
full sweep still has to happen in each GC cycle, which means we spent
a lot of time in sweeping.
Fix this by creating a separate list consisting of just the in-use
spans to be swept, so sweeping is proportional to the number of in-use
spans (which is proportional to the live heap). Specifically, we
create two lists: a list of unswept in-use spans and a list of swept
in-use spans. At the start of the sweep cycle, the swept list becomes
the unswept list and the new swept list is empty. Allocating a new
in-use span adds it to the swept list. Sweeping moves spans from the
unswept list to the swept list.
This fixes the amortization problem because a shrinking heap moves
spans off the unswept list without adding them to the swept list,
reducing the time required by the next sweep cycle.
Updates #9265. This fix eliminates almost all of the time spent in
sweepone; however, markrootSpans has essentially the same bug, so now
the test program from this issue spends all of its time in
markrootSpans.
Austin Clements [Tue, 4 Oct 2016 20:22:41 +0000 (16:22 -0400)]
runtime: use len(h.spans) to indicate mapped region
Currently we set the len and cap of h.spans to the full reserved
region of the address space and track the actual mapped region
separately in h.spans_mapped. Since we have both the len and cap at
our disposal, change things so len(h.spans) tracks how much of the
spans array is mapped and eliminate h.spans_mapped. This simplifies
mheap and means we'll get nice "index out of bounds" exceptions if we
do try to go off the end of the spans rather than a SIGSEGV.
Austin Clements [Tue, 4 Oct 2016 20:03:00 +0000 (16:03 -0400)]
runtime: consolidate h_spans and mheap_.spans
Like h_allspans and mheap_.allspans, these were two ways of referring
to the spans array from when the runtime was split between C and Go.
Clean this up by making mheap_.spans a slice and eliminating h_spans.
Austin Clements [Tue, 4 Oct 2016 19:56:19 +0000 (15:56 -0400)]
runtime: eliminate mheap.nspan and use range loops
This was necessary in the C days when allspans was an mspan**, but now
that allspans is a Go slice, this is redundant with len(allspans) and
we can use range loops over allspans.
Austin Clements [Tue, 4 Oct 2016 19:51:31 +0000 (15:51 -0400)]
runtime: consolidate h_allspans and mheap_.allspans
These are two ways to refer to the allspans array that hark back to
when the runtime was split between C and Go. Clean this up by making
mheap_.allspans a slice and eliminating h_allspans.
Cherry Zhang [Thu, 13 Oct 2016 10:57:00 +0000 (06:57 -0400)]
cmd/compile: add a writebarrier phase in SSA
When the compiler insert write barriers, the frontend makes
conservative decisions at an early stage. This may have false
positives which result in write barriers for stack writes.
A new phase, writebarrier, is added to the SSA backend, to delay
the decision and eliminate false positives. The frontend still
makes conservative decisions. When building SSA, instead of
emitting runtime calls directly, it emits WB ops (StoreWB,
MoveWB, etc.), which will be expanded to branches and runtime
calls in writebarrier phase. Writes to static locations on stack
are detected and write barriers are removed.
All write barriers of stack writes found by the script from
issue #17330 are eliminated (except two false positives).
Fixes #17330.
Change-Id: I9bd66333da9d0ceb64dcaa3c6f33502798d1a0f8
Reviewed-on: https://go-review.googlesource.com/31131 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Cherry Zhang [Mon, 24 Oct 2016 02:32:16 +0000 (22:32 -0400)]
cmd/internal/obj: save link register in leaf function with non-empty frame on PPC64, ARM64, S390X
The runtime traceback code assumes non-empty frame has link
link register saved on LR architectures. Make sure it is so in
the assember.
Also make sure that LR is stored before update SP, so the traceback
code will not see a half-updated stack frame if a signal comes
during the execution of function prologue.
Fixes #17381.
Change-Id: I668b04501999b7f9b080275a2d1f8a57029cbbb3
Reviewed-on: https://go-review.googlesource.com/31760 Reviewed-by: Michael Munday <munday@ca.ibm.com>
Lynn Boger [Tue, 25 Oct 2016 20:13:07 +0000 (15:13 -0500)]
cmd/objdump: updates from golang.org/x/arch/ppc64/ppc64asm
Update the ppc64x disassembly code for use by objdump
from golang.org/x/arch/ppc64/ppc64asm commit fcea5ea.
Enable the objdump testcase for external linking on ppc64le
make a minor fix to the expected output.
Rob Pike [Mon, 24 Oct 2016 19:38:06 +0000 (12:38 -0700)]
cmd/doc: show documentation for interface methods when requested explicitly
For historical reasons, the go/doc package does not include
the methods within an interface as part of the documented
methods for that type. Thus,
go doc ast.Node.Pos
gives an incorrect and confusing error message:
doc: no method Node.Pos in package go/ast
This CL does some dirty work to dig down to the methods
so interface methods now present their documentation:
% go doc ast.node.pos
func Pos() token.Pos // position of first character belonging to the node
%
It must largely sidestep the doc package to do this, which
is a shame. Perhaps things will improve there one day.
The change does not handle embeddings, and in principle the
same approach could be done for struct fields, but that is also
not here yet. But this CL fixes the thing that was bugging me.
Change-Id: Ic10a91936da96f54ee0b2f4a4fe4a8c9b93a5b4a
Reviewed-on: https://go-review.googlesource.com/31852 Reviewed-by: Robert Griesemer <gri@golang.org>
shaharko [Wed, 19 Oct 2016 04:33:16 +0000 (07:33 +0300)]
cmd/compile, cmd/link: more efficient typelink generation
Instead of generating typelink symbols in the compiler
mark types that should have typelinks with a flag.
The linker detects this flag and adds the marked types
to the typelink table.
name old s/op new s/op delta
LinkCmdCompile 0.27 ± 6% 0.25 ± 6% -6.93% (p=0.000 n=97+98)
LinkCmdGo 0.30 ± 5% 0.29 ±10% -4.22% (p=0.000 n=97+99)
name old MaxRSS new MaxRSS delta
LinkCmdCompile 112k ± 3% 106k ± 2% -4.85% (p=0.000 n=100+100)
LinkCmdGo 107k ± 3% 103k ± 3% -3.00% (p=0.000 n=100+100)
Change-Id: Ic95dd4b0101e90c1fa262c9c6c03a2028d6b3623
Reviewed-on: https://go-review.googlesource.com/31772
Run-TryBot: Shahar Kohanim <skohanim@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
Mohit Agarwal [Tue, 25 Oct 2016 14:03:50 +0000 (19:33 +0530)]
math/cmplx: prevent infinite loop in tanSeries
The condition to determine if any further iterations are needed is
evaluated to false in case it encounters a NaN. Instead, flip the
condition to keep looping until the factor is greater than the machine
roundoff error.
Updates #17577
Change-Id: I058abe73fcd49d3ae4e2f7b33020437cc8f290c3
Reviewed-on: https://go-review.googlesource.com/31952 Reviewed-by: Robert Griesemer <gri@golang.org>
David Chase [Tue, 25 Oct 2016 15:11:40 +0000 (11:11 -0400)]
cmd/compile: added test to ensure that accidental fix remains
Bug 15141 was apparently fixed by some other change to the
compiler (this is plausible, it was a weird bug dependent
on a particular way of returning a large named array result),
add the test to ensure that it stays fixed.
Lynn Boger [Mon, 24 Oct 2016 19:40:42 +0000 (14:40 -0500)]
cmd/internal: add shift opcodes with shift operands on ppc64x
Some original shift opcodes for ppc64x expected an operand to be
a mask instead of a shift count, preventing some valid shift counts
from being written.
This adds new opcodes for shifts where needed, using mnemonics that
match the ppc64 asm and allowing the assembler to accept the full set
of valid shift counts.
Fixes #15016
Change-Id: Id573489f852038d06def279c13fd0523736878a7
Reviewed-on: https://go-review.googlesource.com/31853
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> Reviewed-by: David Chase <drchase@google.com>
shaharko [Thu, 13 Oct 2016 19:56:38 +0000 (22:56 +0300)]
cmd/compile: only generate ·f symbols when necessary
Before go supported buildmode=shared ·f symbols used to be defined
only when they were used. In order to solve #11480 the strategy
was changed to have these symbols defined on declaration which is
less efficient and generates many unneeded symbols.
With this change the best strategy is chosen for each situation,
improving static linking time:
name old s/op new s/op delta
LinkCmdCompile 0.27 ± 5% 0.25 ± 6% -8.22% (p=0.000 n=98+96)
LinkCmdGo 0.30 ± 6% 0.29 ± 8% -5.03% (p=0.000 n=95+99)
name old MaxRSS new MaxRSS delta
LinkCmdCompile 107k ± 2% 98k ± 3% -8.32% (p=0.000 n=99+100)
LinkCmdGo 106k ± 3% 104k ± 3% -1.94% (p=0.000 n=99+100)
Change-Id: I965eeee30541e724fd363804adcd6fda10f965a4
Reviewed-on: https://go-review.googlesource.com/31031 Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Paul Marks [Tue, 25 Oct 2016 00:49:22 +0000 (17:49 -0700)]
net: add hostname warnings to all first(isIPv4) functions.
In general, these functions cannot behave correctly when given a
hostname, because a hostname may represent multiple IP addresses, and
first(isIPv4) chooses at most one.
Hiroshi Ioka [Sun, 23 Oct 2016 23:52:04 +0000 (08:52 +0900)]
path/filepath: make TestToNorm robust
The old code leaves garbages in a temporary directory because it
cannot remove the current working directory on windows.
The new code changes the directory before calling os.Remove.
Furthermore, the old code assumes that ioutil.TempDir (os.TempDir)
doesn't return a relative path nor an UNC path.
If it isn't the case, the new code calls t.Fatal earlier for preventing
ambiguous errors.
Finally, the old code reassigns the variable which is used by the defer
function. It could cause unexpected results, so avoid that.
Alex Brainman [Mon, 24 Oct 2016 04:06:20 +0000 (15:06 +1100)]
cmd/link/internal/ld: remove goto from ldpe.go
Updates #15345
Change-Id: I447d133512e99a9900928a910e161a85db6e8b75
Reviewed-on: https://go-review.googlesource.com/31792 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
David du Colombier [Mon, 24 Oct 2016 22:31:45 +0000 (00:31 +0200)]
net: handle "dns failure" as errNoSuchHost on Plan 9
CL 31468 added TestLookupNonLDH, which was failing on Plan 9,
because LookupHost was expecting to return errNoSuchHost
on DNS resolution failure, while Plan 9 returned the
"dns failure" string.
In the Plan 9 implementation of lookupHost, we now return
errNoSuchHost instead of the "dns failure" string, so
the behavior is more consistant with other operating systems.
Fixes #17568.
Change-Id: If64f580dc0626a4a4f19e5511ba2ca5daff5f789
Reviewed-on: https://go-review.googlesource.com/31873
Run-TryBot: David du Colombier <0intro@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
crypto/tls: add VerifyPeerCertificate to tls.Config
VerifyPeerCertificate returns an error if the peer should not be
trusted. It will be called after the initial handshake and before
any other verification checks on the cert or chain are performed.
This provides the callee an opportunity to augment the certificate
verification.
If VerifyPeerCertificate is not nil and returns an error,
then the handshake will fail.
Cherry Zhang [Fri, 21 Oct 2016 22:22:13 +0000 (18:22 -0400)]
cmd/internal/obj/mips: store LR before update SP in function prologue
This prevents the traceback code from seeing a half-updated
stack frame when a profiling signal comes during the execution
of function prologue. Also fixes mips64x part of #17381.
Matthew Dempsky [Mon, 24 Oct 2016 18:46:06 +0000 (11:46 -0700)]
cmd/internal/obj: drop Addr's Gotype field
The Gotype field is only used for ATYPE instructions. Instead of
specially storing the Go type symbol in From.Gotype, just store it in
To.Sym like any other 2-argument instruction would.
Alan Donovan [Mon, 24 Oct 2016 15:00:19 +0000 (11:00 -0400)]
cmd/vet: cgo: emit no error for calls to C.CBytes
Fixes issue golang/go#17563
Change-Id: Ibb41ea9419907193526cc601f6afd07d8689b1fe
Reviewed-on: https://go-review.googlesource.com/31810 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Russ Cox [Wed, 19 Oct 2016 04:01:31 +0000 (00:01 -0400)]
path/filepath: fix match of \\?\c:\* on Windows
\\?\c:\ is a "root directory" that is not subject to further matching,
but the ? makes it look like a pattern, which was causing an
infinite recursion. Make sure the code understands the ? is not a pattern.
Fixes #15879.
Change-Id: I3a4310bbc398bcae764b9f8859c875317345e757
Reviewed-on: https://go-review.googlesource.com/31460 Reviewed-by: Quentin Smith <quentin@golang.org>
Russ Cox [Thu, 20 Oct 2016 20:53:49 +0000 (16:53 -0400)]
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
Reviewed-on: https://go-review.googlesource.com/31582 Reviewed-by: Quentin Smith <quentin@golang.org>
Russ Cox [Wed, 19 Oct 2016 14:27:05 +0000 (10:27 -0400)]
html/template, text/template: docs and fixes for template redefinition
All prior versions of Go have allowed redefining empty templates
to become non-empty. Unfortunately, that has never consistently
taken effect in html/template after the first execution:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}{{end}}<a href="{{template "T"}}">`)
t.Execute(w, nil) // <a href="">
When Go 1.6 added {{block...}} to text/template, that loosened the
redefinition rules to allow redefinition at any time. The loosening was
undone a bit in html/template, although inconsistently:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}body{{end}}`)
t.Lookup("T").Execute(ioutil.Discard, nil)
// attempt to redefine
t.Parse(`{{define "T"}}body{{end}}`) // rejected in all Go versions
t.Lookup("T").Parse("body") // OK as of Go 1.6, likely unintentionally
Like in the empty->non-empty case, whether future execution takes
notice of a redefinition basically can't be explained without going into
the details of the template escape analysis.
Address both the original inconsistencies in whether a redefinition
would have any effect and the new inconsistencies about whether a
redefinition is allowed by adopting a new rule: no parsing or modifying
any templates after the first execution of any template in the same set.
Template analysis begins at first execution, and once template analysis
has begun, we simply don't have the right logic to update the analysis
for incremental modifications (and never have).
If this new rule breaks existing uses of templates that we decide need
to be supported, we can try to invalidate all escape analysis for the
entire set after any modifications. But let's wait on that until we know
we need to and why.
Also fix documentation of text/template redefinition policy
(redefinition is always OK).
Russ Cox [Wed, 19 Oct 2016 20:48:21 +0000 (16:48 -0400)]
net: there are no invalid domain names anymore
The Go resolver reports invalid domain name for '!!!.local',
but that is allowed by multicast DNS. In general we can't predict
what future relaxations might come along, and libc resolvers
do not distinguish 'no such host' from 'invalid name', so stop
making that distinction here too. Always use 'no such host'.
Quentin Smith [Fri, 21 Oct 2016 20:48:50 +0000 (16:48 -0400)]
cmd/doc: continue searching after error reading directory
If a directory in GOPATH is unreadable, we should keep looking for other
packages. Otherwise we can give the misleading error "no buildable Go
source files".
Fixes #16240
Change-Id: I38e1037f56ec463d3c141f0508fb74211cb90f13
Reviewed-on: https://go-review.googlesource.com/31713
Run-TryBot: Quentin Smith <quentin@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Russ Cox [Fri, 21 Oct 2016 15:37:54 +0000 (11:37 -0400)]
cmd/go: allow 'go generate' even if imports do not resolve
Maybe the go generate is generating the imports,
or maybe there's some other good reason the code
is incomplete.
The help text already says:
Note that go generate does not parse the file, so lines that look
like directives in comments or multiline strings will be treated
as directives.
We'll still reject Go source files that don't begin with a package statement
or have a syntax error in the import block, but those are I think more
defensible rejections.
Martin Möhrmann [Sat, 22 Oct 2016 13:43:23 +0000 (15:43 +0200)]
cmd/compile: replace ANDL with MOV?ZX
According to "Intel 64 and IA-32 Architectures Optimization Reference
Manual" Section: "3.5.1.13 Zero-Latency MOV Instructions"
MOV?ZX instructions have zero latency on newer processors.
during make.bash:
(ANDLconst [0xFF] x) -> (MOVBQZX x)
applies 422 times
(ANDLconst [0xFFFF] x) -> (MOVWQZX x)
applies 114 times
Updates #15105
Change-Id: I10933af599de3c26449c52f4b5cd859331028f39
Reviewed-on: https://go-review.googlesource.com/31639
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Alex Brainman [Mon, 24 Oct 2016 01:04:12 +0000 (12:04 +1100)]
runtime/cgo: do not link math lib by default on windows
Makes windows same as others.
Change-Id: Ib4651e06d0bd37473ac345d36c91f39aa8f5e662
Reviewed-on: https://go-review.googlesource.com/31791 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Sun, 2 Oct 2016 22:46:02 +0000 (18:46 -0400)]
runtime: make mspan.isFree do what's on the tin
Currently mspan.isFree technically returns whether the object was not
allocated *during this cycle*. Fix it so it actually returns whether
or not the object is allocated so the method is more generally useful
(especially for debugging).
It has one caller, which is carefully written to be insensitive to
this distinction, but this lets us simplify this caller.
Austin Clements [Wed, 19 Oct 2016 22:27:39 +0000 (18:27 -0400)]
runtime: make morestack less subtle
morestack writes the context pointer to gobuf.ctxt, but since
morestack is written in assembly (and has to be very careful with
state), it does *not* invoke the requisite write barrier for this
write. Instead, we patch this up later, in newstack, where we invoke
an explicit write barrier for ctxt.
This already requires some subtle reasoning, and it's going to get a
lot hairier with the hybrid barrier.
Fix this by simplifying the whole mechanism. Instead of writing
gobuf.ctxt in morestack, just pass the value of the context register
to newstack and let it write it to gobuf.ctxt. This is a normal Go
pointer write, so it gets the normal Go write barrier. No subtle
reasoning required.
Brad Fitzpatrick [Sat, 22 Oct 2016 16:47:05 +0000 (09:47 -0700)]
net/http: add NoBody, don't return nil from NewRequest on zero bodies
This is an alternate solution to https://golang.org/cl/31445
Instead of making NewRequest return a request with Request.Body == nil
to signal a zero byte body, add a well-known variable that means
explicitly zero.
Too many tests inside Google (and presumably the outside world)
broke.
Joe Tsai [Wed, 19 Oct 2016 00:22:25 +0000 (17:22 -0700)]
archive/tar: validate sparse headers in parsePAX
According to the GNU manual, the format is:
<<<
GNU.sparse.size=size
GNU.sparse.numblocks=numblocks
repeat numblocks times
GNU.sparse.offset=offset
GNU.sparse.numbytes=numbytes
end repeat
>>>
The logic in parsePAX converts the repeating sequence of
(offset, numbytes) pairs (which is not PAX compliant) into a single
comma-delimited list of numbers (which is now PAX compliant).
Thus, we validate the following:
* The (offset, numbytes) headers must come in the correct order.
* The ',' delimiter cannot appear in the value.
We do not validate that the value is a parsible decimal since that
will be determined later.
Brad Fitzpatrick [Sat, 22 Oct 2016 13:39:12 +0000 (06:39 -0700)]
net/http: document Transport.ExpectContinueTimeout a bit more
Fixes #16003
Change-Id: I76a8da24b9944647ec40ef2ca4fc93c175ff5a25
Reviewed-on: https://go-review.googlesource.com/31723 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick [Sat, 22 Oct 2016 01:03:49 +0000 (18:03 -0700)]
net: clarify LookupAddr docs on libc's behavior, and alternatives
Text from rsc.
Fixes #17093
Change-Id: I13c3018b1584f152b53f8576dd16ebef98aa5182
Reviewed-on: https://go-review.googlesource.com/31720 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Matthew Dempsky [Fri, 21 Oct 2016 21:43:59 +0000 (14:43 -0700)]
cmd/compile: fix detection of duplicate cases for integer ranges
Previously, the check to make sure we only considered constant cases
for duplicates was skipping past integer ranges, because those use
n.List instead of n.Left. Thanks to Emmanuel Odeke for investigating
and helping to identify the root cause.
Fixes #17517.
Change-Id: I46fcda8ed9c346ff3a9647d50b83f1555587b740
Reviewed-on: https://go-review.googlesource.com/31716
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Robert Griesemer <gri@golang.org>