In general, finishsweep_m must block until any spans that are
concurrently being swept have been swept. It accomplishes this by
looping over all spans, which, as in the previous commit, takes
~1ms/heap GB. Unfortunately, we do this during the STW sweep
termination phase, so multi-gigabyte heaps can push our STW time past
10ms.
However, there's no need to do this wait if the world is stopped
because, in effect, stopping the world already had to wait for
anything that was sweeping (and if it didn't, the wait in
finishsweep_m would deadlock). Hence, we can simply skip this loop if
the world is stopped, such as during sweep termination. In fact,
currently all calls to finishsweep_m are STW, but this hasn't always
been the case and may not be the case in the future, so we keep the
logic around.
For 24GB heaps, this reduces max pause time by 75% relative to tip and
by 90% relative to Go 1.5. Notably, all pauses are now well under
10ms. Here are the results for the garbage benchmark:
------------- max pause ------------
Heap Procs after change before change 1.5.1
24GB 12 3.8ms 16ms 37ms
24GB 4 3.7ms 16ms 37ms
4GB 4 3.7ms 3ms 6.9ms
In the 4GB/4P case, it seems the "before change" run got lucky: the
max went up, but the 99%ile pause time went down from 3ms to 2.04ms.
Change-Id: Ica22189559f231d408ef2815019c9dbb5f38bf31
Reviewed-on: https://go-review.googlesource.com/15071
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In order to compute the sweep ratio, the runtime needs to know how
many pages belong to spans in state _MSpanInUse. Currently it finds
this out by looping over all spans during mark termination. However,
this takes ~1ms/heap GB, so multi-gigabyte heaps can quickly push our
STW time past 10ms.
Replace the loop with an actively maintained count of in-use pages.
For multi-gigabyte heaps, this reduces max mark termination pause time
by 75%–90% relative to tip and by 85%–95% relative to Go 1.5.1. This
shifts the longest pause time for large heaps to the sweep termination
phase, so it only slightly decreases max pause time, though it roughly
halves mean pause time. Here are the results for the garbage
benchmark:
---- max mark termination pause ----
Heap Procs after change before change 1.5.1
24GB 12 1.9ms 18ms 37ms
24GB 4 3.7ms 18ms 37ms
4GB 4 920µs 3.8ms 6.9ms
Fixes#11484.
Change-Id: Ia2d28bb8a1e4f1c3b8ebf79fb203f12b9bf114ac
Reviewed-on: https://go-review.googlesource.com/15070
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reduces pause time by ~25% relative to tip and by ~50% relative
to Go 1.5.1.
Currently one of the steps of STW mark termination is to loop (in
parallel) over all spans to find objects with finalizers in order to
mark all objects reachable from these objects and to treat the
finalizer special as a root. Unfortunately, even if there are no
finalizers at all, this loop takes roughly 1 ms/heap GB/core, so
multi-gigabyte heaps can quickly push our STW time past 10ms.
Fix this by moving this scan from mark termination to concurrent scan,
where it can run in parallel with mutators. The loop itself could also
be optimized, but this cost is small compared to concurrent marking.
Making this scan concurrent introduces two complications:
1) The scan currently walks the specials list of each span without
locking it, which is safe only with the world stopped. We fix this by
speculatively checking if a span has any specials (the vast majority
won't) and then locking the specials list only if there are specials
to check.
2) An object can have a finalizer set after concurrent scan, in which
case it won't have been marked appropriately by concurrent scan. If
the finalizer is a closure and is only reachable from the special, it
could be swept before it is run. Likewise, if the object is not marked
yet when the finalizer is set and then becomes unreachable before it
is marked, other objects reachable only from it may be swept before
the finalizer function is run. We fix this issue by making
addfinalizer ensure the same marking invariants as markroot does.
For multi-gigabyte heaps, this reduces max pause time by 20%–30%
relative to tip (depending on GOMAXPROCS) and by ~50% relative to Go
1.5.1 (where this loop was neither concurrent nor parallel). Here are
the results for the garbage benchmark:
---------------- max pause ----------------
Heap Procs Concurrent scan STW parallel scan 1.5.1
24GB 12 18ms 23ms 37ms
24GB 4 18ms 25ms 37ms
4GB 4 3.8ms 4.9ms 6.9ms
In all cases, 95%ile pause time is similar to the max pause time. This
also improves mean STW time by 10%–30%.
Fixes#11485.
Change-Id: I9359d8c3d120a51d23d924b52bf853a1299b1dfd
Reviewed-on: https://go-review.googlesource.com/14982
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, the GC modes constants are untyped and functions pass them
around as ints. Clean this up by introducing a proper type for these
constant.
Change-Id: Ibc022447bdfa203644921fbb548312d7e2272e8d
Reviewed-on: https://go-review.googlesource.com/14981
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Removed direct link to issue tracker in the README - it makes it too
easy to use it for a question - and it's abused multiple times a day
for questions. It's easy enough to find it if there's a real issue
to report.
Added sentence to point people at golang-nuts and the new forum.
Change-Id: If75bab888cda064aceeefc49ef672fbb964f8f54
Reviewed-on: https://go-review.googlesource.com/15284
Reviewed-by: Jason Buberel <jbuberel@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes a case where the Stmt.Close() function in database/sql discards any error generated by the Close() function of the contained driverStmt.
Fixes#12798
Change-Id: I40384d6165856665b062d15a643e4ecc09d63fda
Reviewed-on: https://go-review.googlesource.com/15178
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change splits signal_unix.go into signal_unix.go and
signal2_unix.go and removes the fake symbol sigfwd from signal
forwarding unsupported platforms for clarification purpose.
Change-Id: I205eab5cf1930fda8a68659b35cfa9f3a0e67ca6
Reviewed-on: https://go-review.googlesource.com/12062
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If the stream is in an inconsistent state, it does not make sense
that Reader.Read can be called and possibly succeed.
Change-Id: I9d1c5a1300b2c2b45232188aa7999e350809dcf2
Reviewed-on: https://go-review.googlesource.com/15177
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The native Go host resolver was behaving differently than libc
and the entries in the /etc/hosts were handled in a case sensitive
way. In order to be compatible with libc's resolver, /etc/hosts
lookups must be case-insensitive.
Fixes#12806.
Change-Id: I3c14001abffadf7458fd1a027c91e6438a87f285
Reviewed-on: https://go-review.googlesource.com/15321
Run-TryBot: Burcu Dogan <jbd@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing test did not take into account the implicit
dereference of &fixedArray and thus heap-escaped when it
was not necessary.
Also added a detailed test for this and related cases.
Fixes#12588
Change-Id: I951e9684a093082ccdca47710f69f4366bd6b3cf
Reviewed-on: https://go-review.googlesource.com/15130
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reduce allocation to avoid running out of memory on the openbsd/arm builder,
until issue/12032 is resolved.
Update issue #12032
Change-Id: Ibd513829ffdbd0db6cd86a0a5409934336131156
Reviewed-on: https://go-review.googlesource.com/15242
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
sysReserve will return nil on failure - correctly handle this case and return
nil to the caller. Currently, a failure will result in h.arena_end being set
to psize, h.arena_used being set to zero and fun times ensue.
On the openbsd/arm builder this has resulted in:
runtime: address space conflict: map(0x0) = 0x40946000
fatal error: runtime: address space conflict
When it should be reporting out of memory instead.
Change-Id: Iba828d5ee48ee1946de75eba409e0cfb04f089d4
Reviewed-on: https://go-review.googlesource.com/15056
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This is a follow-up to a326c3e to avoid reflect being in the API.
Fixes#12801.
Change-Id: Ic4c2e592e2c35b5911f75d88f1d9c44787c80f30
Reviewed-on: https://go-review.googlesource.com/15240
Run-TryBot: David Symonds <dsymonds@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The sparseFileReader is prone to two different forms of
denial-of-service attacks:
* A malicious tar file can cause an infinite loop
* A malicious tar file can cause arbitrary panics
This results because of poor error checking/handling, which this
CL fixes. While we are at it, add a plethora of unit tests to
test for possible malicious inputs.
Change-Id: I2f9446539d189f3c1738a1608b0ad4859c1be929
Reviewed-on: https://go-review.googlesource.com/15115
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reader failed to detect truncated streams since calls to
io.ReadFull did not check if the error is io.EOF.
Change-Id: I86c497519daaaccefc6eb5617ddcd8fd3b99f51b
Reviewed-on: https://go-review.googlesource.com/14835
Reviewed-by: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The memory sanitizer (msan) is a nice compiler feature that can
dynamically check for memory errors in C code. It's not useful for Go
code, since Go is memory safe. But it is useful to be able to use the
memory sanitizer on C code that is linked into a Go program via cgo.
Without this change it does not work, as msan considers memory passed
from Go to C as uninitialized.
To make this work, change the runtime to call the C mmap function when
using cgo. When using msan the mmap call will be intercepted and marked
as returning initialized memory.
Work around what appears to be an msan bug by calling malloc before we
call mmap.
Change-Id: I8ab7286d7595ae84782f68a98bef6d3688b946f9
Reviewed-on: https://go-review.googlesource.com/15170
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Also add a unit test to lock this behavior into the API.
Fixes#12016
Change-Id: Ib6ec6e7948f0705f3504ede9143b5dc4e790fc44
Reviewed-on: https://go-review.googlesource.com/15171
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We've broken periodic GC a few times without noticing because there's
no test for it, partly because you have to wait two minutes to see if
it happens. This exposes control of the periodic GC timeout to runtime
tests and adds a test that cranks it down to zero and sleeps for a bit
to make sure periodic GCs happen.
Change-Id: I3ec44e967e99f4eda752f85c329eebd18b87709e
Reviewed-on: https://go-review.googlesource.com/13169
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
The format for a CSR is horribly underspecified and we had a mistake.
The code was parsing the attributes from the CSR as a
pkix.AttributeTypeAndValueSET, which is only almost correct: it works so
long as the requested extensions don't contain the optional “critical”
flag.
Unfortunately this mistake is exported somewhat in the API and the
Attributes field of a CSR actually has the wrong type. I've moved this
field to the bottom of the structure and updated the comment to reflect
this.
The Extensions and other fields of the CSR structure can be saved
however and this change does that.
Fixes#11897.
Change-Id: If8e2f5c21934800b72b041e38691efc3e897ecf1
Reviewed-on: https://go-review.googlesource.com/12717
Reviewed-by: Rob Pike <r@golang.org>
Platform-specific verification needs the ASN.1 contents of a certificate
but that might not be provided if the Certificate was not created by
ParseCertificate. In order to avoid a panic on Windows, and to make
behaviour consistent across platforms, this change causes verification
to fail when the ASN.1 contents of a certificate are not available.
Fixes#12184
Change-Id: I4395d74934e675c179eaf4cded1094a756e478bb
Reviewed-on: https://go-review.googlesource.com/14053
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As akalin points out in the bug, the comment previously claimed that the
probability that the input is prime given that the function returned
true is 1 - ¼ⁿ. But that's wrong: the correct statement is that the
probability of the function returning false given a composite input is
1 - ¼ⁿ.
This is not nearly as helpful, but at least it's truthful. A number of
other (correct) expressions are suggested on the bug, but I think that
the simplier one is preferable.
This change also notes that the function is not suitable for
adversarial inputs since it's deterministic.
Fixes#12274.
Change-Id: I6a0871d103b126ee5a5a922a8c6993055cb7b1ed
Reviewed-on: https://go-review.googlesource.com/14052
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change causes the types of skipped PEM blocks to be recorded when
no certificate or private-key data is found in a PEM input. This allows
for better error messages to be return in the case of common errors like
switching the certifiate and key inputs to X509KeyPair.
Fixes#11092
Change-Id: Ifc155a811cdcddd93b5787fe16a84c972011f2f7
Reviewed-on: https://go-review.googlesource.com/14054
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a copy of x/tools/cmd/gotype/gotype.go with the corresponding
x/tools/cmd/gotype/doc.go prepended and including a build tag (ignore).
This way, go/types can be built unaffected. If we need the gotype command,
it is trivially built in the go/types directory with: go build gotype.go .
Fixes#12303.
Change-Id: I2d792fcb39719cc5cc300f657e4735901cd20faa
Reviewed-on: https://go-review.googlesource.com/15152
Reviewed-by: Rob Pike <r@golang.org>
(Changed modified by bradfitz from original rsc version)
Change-Id: I8ea40044c325f333a13d48b59b4795b02c579533
Reviewed-on: https://go-review.googlesource.com/14026
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Inlined the last occurrence of stringsCompare into exprcmp.
Passes go build -a -toolexec 'toolstash -cmp' std cmd.
Change-Id: I8fd99e3fbffc84283cc269368595cba950533066
Reviewed-on: https://go-review.googlesource.com/14872
Reviewed-by: Dave Cheney <dave@cheney.net>
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code to strip GOROOT and GOPATH had a bug: it assumed there
were bytes after the GOROOT prefix but there might not be.
Fix this and other issues by taking care the prefix is really a
file name prefix for the path, not just a string prefix, and
handle the case where GOROOT==path.
Change-Id: I8066865fd05f938bb6dbf3bb8ab1fc58e5cf6bb5
Reviewed-on: https://go-review.googlesource.com/15112
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Good enough for now.
Fixes#11241.
Change-Id: Ieb50809f104d20bcbe14daecac503f72486bec92
Reviewed-on: https://go-review.googlesource.com/15111
Reviewed-by: Alan Donovan <adonovan@google.com>
- more uniform naming
- test sign more deliberately
- remove superfluous test (JSON encoder always uses the JSON marshaler if present)
Change-Id: I37b1e367c01fc8bae1e06adbdb72dd366c08d5ce
Reviewed-on: https://go-review.googlesource.com/15110
Reviewed-by: Alan Donovan <adonovan@google.com>
- moved existing package documentation from nat.go to doc.go
- expanded on it
For #11241.
Change-Id: Ie75a2b0178a8904a4154307a1f5080d7efc5489a
Reviewed-on: https://go-review.googlesource.com/15042
Reviewed-by: Alan Donovan <adonovan@google.com>
Main change is that the comment for an item no longer has a blank line
before it, so it looks bound to the item it's about.
Motivating example: go doc.io.read changes from
<
func (l *LimitedReader) Read(p []byte) (n int, err error)
func (r *PipeReader) Read(data []byte) (n int, err error)
Read implements the standard Read interface: it reads data from the pipe,
blocking until a writer arrives or the write end is closed. If the write end
is closed with an error, that error is returned as err; otherwise err is
EOF.
func (s *SectionReader) Read(p []byte) (n int, err error)
>
to
<
func (l *LimitedReader) Read(p []byte) (n int, err error)
func (r *PipeReader) Read(data []byte) (n int, err error)
Read implements the standard Read interface: it reads data from the pipe,
blocking until a writer arrives or the write end is closed. If the write end
is closed with an error, that error is returned as err; otherwise err is
EOF.
func (s *SectionReader) Read(p []byte) (n int, err error)
>
Now the comment about PipeReader.Read doesn't look like it's about
SectionReader.
Based on a suggestion by dsnet@, a slight tweak from a CL he suggested
and abandoned.
Fixes#12756,
Change-Id: Iaf60ee9ae7f644c83c32d5e130acab0312b0c926
Reviewed-on: https://go-review.googlesource.com/14999
Reviewed-by: Andrew Gerrand <adg@golang.org>
This change adds a new "block" keyword that permits the definition
of templates inline inside existing templates, and loosens the
restriction on template redefinition. Templates may now be redefined,
but in the html/template package they may only be redefined before
the template is executed (and therefore escaped).
The intention is that such inline templates can be redefined by
subsequent template definitions, permitting a kind of template
"inheritance" or "overlay". (See the example for details.)
Fixes#3812
Change-Id: I733cb5332c1c201c235f759cc64333462e70dc27
Reviewed-on: https://go-review.googlesource.com/14005
Reviewed-by: Rob Pike <r@golang.org>
When exporting a function using gccgo, we generate two functions: a Go
function with a leading Cgoexp_ prefix, and a C function that calls the
Go function. The Go function has a name that can not be represented in
C, so the C code needs a declaration with an __asm__ qualifier giving
the name of the Go function.
Before this CL we put that declaration in the exported header file.
Because code would sometimes #include "_cgo_export.h", we added a macro
definition for the C function giving it the name of the declaration. We
then added a macro undefine in the actual C code, so that we could
declare the C function we wanted.
This rounadabout process worked OK until we started exporting the header
file for use with -buildmode=c-archive and c-shared. Doing that caused
the code to see the define and thus call the Go function rather than the
C function. That often works fine, but the C function calls
_cgo_wait_runtime_init_done before calling the Go function, and that
sometimes matters. This didn't show up in tests because we don't test
using gccgo. That is something we should fix, but not now.
Fix that by simplifying the code to declare the C function in the header
file as one would expect, and move the __asm__ declaration to the C
code.
Change-Id: I33547e028152ff98e332630994b4f33285feec32
Reviewed-on: https://go-review.googlesource.com/15043
Reviewed-by: Minux Ma <minux@golang.org>
As detailed in #11910, the current implementation attempts to execute an area
of memory with unknown content. If the memory is executable, the result is
unpredictable - instead, make the test deterministic by attempting to execute
an instruction that is known to trigger a trap on the given architecture.
The new implementation is written by iant@ and provided via #11910.
Update issue #11910
Change-Id: Ia698c36e0dd98a9d9d16a701f60f6748c6faf896
Reviewed-on: https://go-review.googlesource.com/15058
Run-TryBot: Joel Sing <jsing@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As stated in FastCGI specifications:
FastCGI transmits a name-value pair as the length of the name,
followed by the length of the value, followed by the name,
followed by the value.
The current implementation trusts the name and value length
provided in the record, leading to a panic if the record
is malformed.
Added an explicit check on the lengths.
Test case and fix suggested by diogin@gmail.com (Jingcheng Zhang)
Fixes#11824
Change-Id: I883a1982ea46465e1fb02e0e02b6a4df9e529ae4
Reviewed-on: https://go-review.googlesource.com/15015
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
openbsd/arm does not support external linking - skip the note reading test that
uses linkmode external on this platform. While here, cleanup the code and
consistently use t.Skipf for all platforms that cannot run this test.
Change-Id: I64f0d9e038bc4c993c3d843fc069a0b723a924d6
Reviewed-on: https://go-review.googlesource.com/15054
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
