Robert Griesemer [Thu, 4 Oct 2018 23:43:34 +0000 (16:43 -0700)]
go/types: remove work-around for issue #26124
This work-around is not needed anymore now that method
signatures are type-checked separately from their receiver
base types: no artificial cycles are introduced anymore
and so there is no need to artificially cut them.
Updates #26124.
Change-Id: I9d50171f12dd8977116a5d3f63ac39a06b1cd492
Reviewed-on: https://go-review.googlesource.com/c/139899 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 4 Oct 2018 00:50:02 +0000 (17:50 -0700)]
go/types: prepare for delayed type-checking of methods to when they are used
Remove assumption that methods associated to concrete (non-interface)
types have a fully set up signature. Such methods are found through
LookupFieldOrMethod or lookupMethod, or indexed method access from
a Named type. Make sure that the method's signature is type-checked
before use in those cases.
(MethodSets also hold methods but the type checker is not using
them but for internal verification. API clients will be using it
after all methods have been type-checked.)
Some functions such as MissingMethod may now have to type-check a
method and for that they need a *Checker. Add helper functions as
necessary to provide the additional (receiver) parameter but permit
it to be nil if the respective functions are invoked through the API
(at which point we know that all methods have a proper signature and
thus we don't need the delayed type-check).
Since all package-level objects eventually are type-checked through
the top-level loop in Checker.packageObjects we are guaranteed that
all methods will be type-checked as well.
Updates #23203.
Updates #26854.
Change-Id: I6e48f0016cefd498aa70b776e84a48215a9042c5
Reviewed-on: https://go-review.googlesource.com/c/139425 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 4 Oct 2018 19:59:09 +0000 (12:59 -0700)]
go/types: fix recvPtr helper (follow-up on https://golang.org/cl/139422)
The prior CL prepared go/types for the situation where methods might
not have a type-checked signature when being looked up. The respective
adjustments to recvPtr were not correct (but because so far method
signatures are type-checked in time, the bug didn't manifest itself).
Updates #23203.
Updates #26854.
Change-Id: I796691d11e6aac84396bdef802ad30715755fcc6
Reviewed-on: https://go-review.googlesource.com/c/139721 Reviewed-by: Alan Donovan <adonovan@google.com>
Austin Clements [Fri, 30 Jun 2017 16:10:01 +0000 (12:10 -0400)]
runtime: improve mheap.alloc doc and let compiler check system stack
The alloc_m documentation refers to concepts that don't exist (and
maybe never did?). alloc_m is also not the API entry point to span
allocation.
Hence, rewrite the documentation for alloc and alloc_m. While we're
here, document why alloc_m must run on the system stack and replace
alloc_m's hand-implemented system stack check with a go:systemstack
annotation.
Change-Id: I30e263d8e53c2774a6614e1b44df5464838cef09
Reviewed-on: https://go-review.googlesource.com/c/139459
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Alex Brainman [Sat, 16 Jun 2018 06:23:52 +0000 (16:23 +1000)]
cmd/link: move .rel symbol from .rdata into .text
.rel symbol type is sym.SELFROSECT, and that makes .rel written
into .rdata section. But .rel stores code - jump table used for
external C functions. So we have to mark whole .rdata section
as executable (IMAGE_SCN_MEM_EXECUTE), because of .rel presence
in it.
Move .rel into .text section, and make .rdata section non executable.
I also had to move code that adjusted the size of .rel symbol
before calling textaddress, otherwise textaddress would not
calculate size of .text section correctly.
Fixes #25926
Change-Id: I4962f5de7b367410154c8709adfcd8472de9ac1a
Reviewed-on: https://go-review.googlesource.com/c/125455
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Thu, 4 Oct 2018 23:32:21 +0000 (16:32 -0700)]
cmd/link: fix deferreturn location on wasm
On wasm, pcln tables are indexed by "resumption point ID" instead of
by pc offset. When finding a deferreturn call, we must find the
associated resumption point ID for the deferreturn call.
Update #27518
Fixes wasm bug introduced in CL 134637.
Jason Keene [Thu, 13 Sep 2018 02:35:58 +0000 (20:35 -0600)]
cmd/go: ensure git attributes are set
This change disables the export-subst and export-ignore attributes when
creating zip files for modules. This is done to prevent the ziphash for
a given repo/revision from differing based on variables such as git
version or size of repo. The full rational for this change is detailed
here:
Ben Shi [Tue, 11 Sep 2018 08:55:10 +0000 (08:55 +0000)]
cmd/internal/obj/arm64: simplify ADD and SUB
Currently "ADD $0x123456, Rs, Rd" will load pre-stored 0x123456
from the constant pool and use it for the addition. Total 12 bytes
are cost. And so does SUB.
This CL breaks it to "ADD 0x123000, Rs, Rd" + "ADD 0x000456, Rd, Rd".
Both "0x123000" and "0x000456" can be directly encoded into the
instruction binary code. So 4 bytes are saved.
1. The total size of pkg/android_arm64 decreases about 0.3KB.
2. The go1 benchmark show little regression (excluding noise).
Alberto Donizetti [Wed, 3 Oct 2018 19:11:31 +0000 (21:11 +0200)]
runtime: skip TestLldbPython when lldb is too old
The TestLldbPython test is known to fail with very old lldb releases
(3.8 and older). Skip the test when the lldb found on the system is
too old.
Fixes #22299
Change-Id: I8f78d6c0d995118f806dae87f3f04a9726473116
Reviewed-on: https://go-review.googlesource.com/c/139397 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Thu, 4 Oct 2018 14:46:20 +0000 (10:46 -0400)]
runtime: clarify table of arena sizes
Currently the table of arena sizes mixes the number of entries in the
L1 with the size of the L2. While the size of the L2 is important,
this makes it hard to see what's actually going on because there's an
implicit factor of sys.PtrSize.
This changes the L2 column to say both the number of entries and the
size that results in. This should hopefully make the relations between
the columns of the table clearer, since they can now be plugged
directly into the given formula.
Robert Griesemer [Wed, 3 Oct 2018 21:40:09 +0000 (14:40 -0700)]
go/types: determine hasPtrRecv property from source rather than type
LookupFieldOrMethod needs to know if a method receiver is a pointer
type. Until now this was computed from the the method signature's
receiver, which required the method signature to be type-checked.
Furthermore, it required the receiver to be set before the method
signature was fully type-checked in some cases (see issue #6638).
This CL remembers this property during object resolution, when we
know it from the source.
With this CL, method signatures don't need to be type-checked before
they can be looked up; this is a first step towards separating
type checking of types and type-checking of associated methods.
Updates #23203.
Updates #26854.
Change-Id: Ie3eb7976e8fe8176ea1b284fa7471a4b7000f80b
Reviewed-on: https://go-review.googlesource.com/c/139422
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Matthew Dempsky [Wed, 3 Oct 2018 22:47:16 +0000 (15:47 -0700)]
cmd/compile: handle TPTR32 like TPTR64 in smallintconst
In preparation for followup CL merging TPTR32 and TPTR64, move TPTR32
from the small-types fast path to the generic 64-bit fallback code so
that it's in the same case clause as TPTR64.
This should be safe, but theoretically it could change semantics
because TPTR32 used to always be assumed to be "small", whereas now it
will only be considered small for values less than 1<<31.
This change is done in a separate CL so that it's more easily
identified by git bisection in case it does introduce regressions.
Change-Id: I6c7bb253d4e4d95c530a6e05a1147905674b55ca
Reviewed-on: https://go-review.googlesource.com/c/139517
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Kir Kolyshkin [Thu, 4 Oct 2018 01:41:18 +0000 (18:41 -0700)]
crypto/x509: fix getting user home dir on darwin
As pointed out in https://github.com/golang/go/issues/26463,
HOME (or equivalent) environment variable (rather than the
value obtained by parsing /etc/passwd or the like) should be
used to obtain user's home directory.
Since commit fa1a49aa556d8 there's a method to obtain
user's home directory -- use it here.
Michael Fraenkel [Thu, 4 Oct 2018 00:42:05 +0000 (20:42 -0400)]
expvar: add Map.Delete
Fixes #13491
Change-Id: Ic0525d8ee90f47d0d23c1485919aee13d2400494
Reviewed-on: https://go-review.googlesource.com/c/139537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Richard Musiol [Tue, 25 Sep 2018 11:45:08 +0000 (13:45 +0200)]
syscall: use asynchronous operations on js/wasm
This commit makes syscall on js/wasm use the asynchronous variants
of functions in Node.js' fs module. This enables concurrency
and allows the API of the fs module to be implemented with an
alternative backend that only supports asynchronous operations.
Brad Fitzpatrick [Wed, 3 Oct 2018 22:18:07 +0000 (22:18 +0000)]
crypto/rand: warn to stderr if blocked 60+ sec on first Reader.Read call
Fixes #22614
Change-Id: I220afbaaeab4dec6d59eeeef12107234a77f1587
Reviewed-on: https://go-review.googlesource.com/c/139419 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick [Wed, 3 Oct 2018 21:21:55 +0000 (21:21 +0000)]
os: add UserHomeDir
Fixes #26463
Change-Id: Iaef1c7456ffaeadeead6027a37d09c44a3d05bd5
Reviewed-on: https://go-review.googlesource.com/c/139418 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Martin Möhrmann [Sun, 26 Aug 2018 12:22:39 +0000 (14:22 +0200)]
strings: correctly handle invalid utf8 sequences in Map
When an invalid UTF-8 byte sequence is decoded in a range loop over a string
a utf8.RuneError rune is returned. This is not distinguishable from decoding
the valid '\uFFFD' sequence representing utf8.RuneError from a string without
further checks within the range loop.
The previous Map code did not do any extra checks and would thereby not map
invalid UTF-8 byte sequences correctly when those were mapping to utf8.RuneError.
Fix this by adding the extra checks necessary to distinguish the decoding
of invalid utf8 byte sequences from decoding the sequence for utf8.RuneError
when the mapping of a rune is utf8.RuneError.
This fix does not result in a measureable performance regression:
name old time/op new time/op delta
ByteByteMap 1.05µs ± 3% 1.03µs ± 3% ~ (p=0.118 n=10+10)
Map/identity/ASCII 169ns ± 2% 170ns ± 1% ~ (p=0.501 n=9+10)
Map/identity/Greek 298ns ± 1% 303ns ± 4% ~ (p=0.338 n=10+10)
Map/change/ASCII 323ns ± 3% 325ns ± 4% ~ (p=0.679 n=8+10)
Map/change/Greek 628ns ± 5% 635ns ± 1% ~ (p=0.460 n=10+9)
MapNoChanges 120ns ± 4% 119ns ± 1% ~ (p=0.496 n=10+9)
This commit adds AIX operating system to internal/poll package for ppc64
architecture.
Updates: #25893
Change-Id: I9b1da9255012de58f16547c1b18f8840485da170
Reviewed-on: https://go-review.googlesource.com/c/138717
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit adds AIX operating system to runtime package for ppc64
architecture.
Only new files and minor changes are in this commit. Others
modifications in files like asm_ppc64.s will be in separate commits.
Updates: #25893
Change-Id: I9c5e073f5f3debb43b004ad1167694a5afd31cfd
Reviewed-on: https://go-review.googlesource.com/c/138716
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Fri, 17 Aug 2018 19:42:19 +0000 (15:42 -0400)]
runtime: fix race between unminit and Windows profile loop
Currently, the Windows profile loop isn't robust against racing with
unminit. For example,
T1 is running profileloop1, T2 is another thread
T1: thread := atomic.Loaduintptr(&T2.thread)
T2: calls unminit, which does CloseHandle(T2.thread)
T1: attempts to suspends T2
In this case the SuspendThread will fail, but currently we ignore this
failure and forge ahead, which will cause further failures and
probably bad profile data.
Handle this race by defending against SuspendThread failing. If
SuspendThread succeeds, then we know the thread is no longer going
anywhere.
Austin Clements [Mon, 27 Aug 2018 01:30:19 +0000 (21:30 -0400)]
runtime: skip debug call injection tests under a debugger
The debug call injection tests will freeze when run under a debugger
because they depend on catching SIGTRAP, which is usually swallowed by
a debugger.
Keith Randall [Tue, 11 Sep 2018 22:14:28 +0000 (15:14 -0700)]
runtime: on a signal, set traceback address to a deferreturn call
When a function triggers a signal (like a segfault which translates to
a nil pointer exception) during execution, a sigpanic handler is just
below it on the stack. The function itself did not stop at a
safepoint, so we have to figure out what safepoint we should use to
scan its stack frame.
Previously we used the site of the most recent defer to get the live
variables at the signal site. That answer is not quite correct, as
explained in #27518. Instead, use the site of a deferreturn call.
It has all the right variables marked as live (no args, all the return
values, except those that escape to the heap, in which case the
corresponding PAUTOHEAP variables will be live instead).
This CL requires stack objects, so that all the local variables
and args referenced by the deferred closures keep the right variables alive.
Keith Randall [Fri, 7 Sep 2018 21:55:09 +0000 (14:55 -0700)]
cmd/compile,runtime: remove ambiguously live logic
The previous CL introduced stack objects. This CL removes the old
ambiguously live liveness analysis. After this CL we're relying
on stack objects exclusively.
Update a bunch of liveness tests to reflect the new world.
Keith Randall [Sun, 2 Sep 2018 03:16:39 +0000 (20:16 -0700)]
cmd/compile,runtime: implement stack objects
Rework how the compiler+runtime handles stack-allocated variables
whose address is taken.
Direct references to such variables work as before. References through
pointers, however, use a new mechanism. The new mechanism is more
precise than the old "ambiguously live" mechanism. It computes liveness
at runtime based on the actual references among objects on the stack.
Each function records all of its address-taken objects in a FUNCDATA.
These are called "stack objects". The runtime then uses that
information while scanning a stack to find all of the stack objects on
a stack. It then does a mark phase on the stack objects, using all the
pointers found on the stack (and ancillary structures, like defer
records) as the root set. Only stack objects which are found to be
live during this mark phase will be scanned and thus retain any heap
objects they point to.
A subsequent CL will remove all the "ambiguously live" logic from
the compiler, so that the stack object tracing will be required.
For this CL, the stack tracing is all redundant with the current
ambiguously live logic.
Matthew Dempsky [Wed, 3 Oct 2018 18:06:55 +0000 (11:06 -0700)]
cmd/compile/internal/gc: remove binary package export format
This CL removes all unused code from bimport.go and bexport.go.
In the interest of keeping this CL strictly delete-only and easier to
review, the task of consolidating the vestigial code elsewhere is left
to future CLs.
Change-Id: Ib757cc27e3fe814cbf534776d026e4d4cddfc6db
Reviewed-on: https://go-review.googlesource.com/c/139338
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Matthew Dempsky [Wed, 3 Oct 2018 17:56:23 +0000 (10:56 -0700)]
cmd/compile/internal/gc: disable binary package export format
The new indexed package export format appears stable, and no reports
of needing to revert back to binary package export.
This CL disables the binary package export format by mechanically
replacing 'flagiexport' with 'true', and then superficial code
cleanups to keep the resulting code idiomatic. The resulting dead code
is removed in a followup CL.
Change-Id: Ic30d85f78778a31d279a56b9ab14e80836d50135
Reviewed-on: https://go-review.googlesource.com/c/139337
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Florian Uekermann [Fri, 1 Dec 2017 19:01:55 +0000 (20:01 +0100)]
time: return first error in unsuccessful calls to LoadLocation
Unsuccessful calls to LoadLocation previously returned the first
error encountered while traversing the default list of sources, but
ignored errors from sources specified by ZONEINFO. Whether errors
indicating missing zones or sources were ignored in this process
differed between kinds of sources.
With this change, unsuccessful calls to LoadLocation always return
the first error, not counting errors indicating missing zones or
sources.
David Url [Thu, 23 Aug 2018 15:28:59 +0000 (17:28 +0200)]
net/http: log call site which causes multiple header writes
If an illegal header write is detected, find the first caller outside of
net/http using runtime.CallersFrames and include the call site in the log
message.
When deadline has already passed,
current context is canceled before return cancel function.
So is unnecessary to call cancel with remove from parent again
in return cancel function.
Cherry Zhang [Wed, 3 Oct 2018 02:04:45 +0000 (22:04 -0400)]
cmd/compile: fix type of OffPtr in some optimization rules
In some optimization rules the type of generated OffPtr was
incorrectly set to the type of the pointee, instead of the
pointer. When the OffPtr value is spilled, this may generate
a spill of the wrong type, e.g. a floating point spill of an
integer (pointer) value. On Wasm, this leads to invalid
bytecode.
Fixes #27961.
Change-Id: I5d464847eb900ed90794105c0013a1a7330756cc
Reviewed-on: https://go-review.googlesource.com/c/139257
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Richard Musiol <neelance@gmail.com>
Anton Gyllenberg [Tue, 2 Oct 2018 22:47:28 +0000 (22:47 +0000)]
cmd/go: prevent infinite loop in QueryPackage()
p = path.Dir(p) converges to either "." or "/". The current
implementation of modload.QueryPackage() has a loop that
terminates only on ".", not "/". This leads to the go command
hanging in an infinite loop if the user manages to supply
a file path starting with "/" as package path.
An example of the issue is if the user (incorrectly) attempts
to use an absolute directory path in an import statement within
a module (import "/home/bob/myproj") and then runs go list.
This commit adds AIX operating system to cmd/dist package for ppc64
architecture.
The stack guard is increased because of syscalls made inside the runtime
which need a larger stack.
Disable cmd/vet/all tests until aix/ppc64 is fully available.
Change-Id: I7e3caf86724249ae564a152d90c1cbd4de288814
Reviewed-on: https://go-review.googlesource.com/c/138715
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Eduard Urbach [Tue, 24 Apr 2018 14:28:19 +0000 (14:28 +0000)]
net/http: explain more how Pusher.Push works
This will clarify that the resources are not completely pushed yet when `Push` returns and that it starts a separate goroutine. This might be implementation dependant but as I believe there is currently only one implementation it should be added to the documentation of the interface which most people will look up first.
net/http: make Transport send WebSocket upgrade requests over HTTP/1
WebSockets requires HTTP/1 in practice (no spec or implementations
work over HTTP/2), so if we get an HTTP request that looks like it's
trying to initiate WebSockets, use HTTP/1, like browsers do.
This is part of a series of commits to make WebSockets work over
httputil.ReverseProxy. See #26937.
Austin Clements [Thu, 16 Aug 2018 16:32:46 +0000 (12:32 -0400)]
runtime: eliminate work.markrootdone and second root marking pass
Before STW and concurrent GC were unified, there could be either one
or two root marking passes per GC cycle. There were several tasks we
had to make sure happened once and only once (whether that was at the
beginning of concurrent mark for concurrent GC or during mark
termination for STW GC). We kept track of this in work.markrootdone.
Now that STW and concurrent GC both use the concurrent marking code
and we've eliminated all work done by the second root marking pass, we
only ever need a single root marking pass. Hence, we can eliminate
work.markrootdone and all of the code that's conditional on it.
Austin Clements [Thu, 23 Aug 2018 17:14:19 +0000 (13:14 -0400)]
runtime: flush mcaches lazily
Currently, all mcaches are flushed during STW mark termination as a
root marking job. This is currently necessary because all spans must
be out of these caches before sweeping begins to avoid races with
allocation and to ensure the spans are in the state expected by
sweeping. We do it as a root marking job because mcache flushing is
somewhat expensive and O(GOMAXPROCS) and this parallelizes the work
across the Ps. However, it's also the last remaining root marking job
performed during mark termination.
This CL moves mcache flushing out of mark termination and performs it
lazily. We keep track of the last sweepgen at which each mcache was
flushed and as each P is woken from STW, it observes that its mcache
is out-of-date and flushes it.
The introduces a complication for spans cached in stale mcaches. These
may now be observed by background or proportional sweeping or when
attempting to add a finalizer, but aren't in a stable state. For
example, they are likely to be on the wrong mcentral list. To fix
this, this CL extends the sweepgen protocol to also capture whether a
span is cached and, if so, whether or not its cache is stale. This
protocol blocks asynchronous sweeping from touching cached spans and
makes it the responsibility of mcache flushing to sweep the flushed
spans.
This eliminates the last mark termination root marking job, which
means we can now eliminate that entire infrastructure.
Updates #26903. This implements lazy mcache flushing.
Austin Clements [Thu, 16 Aug 2018 16:25:38 +0000 (12:25 -0400)]
runtime: eliminate blocking GC work drains
Now work.helperDrainBlock is always false, so we can remove it and
code paths that only ran when it was true. That means we no longer use
the gcDrainBlock mode of gcDrain, so we can eliminate that. That means
we no longer use gcWork.get, so we can eliminate that. That means we
no longer use getfull, so we can eliminate that.
Updates #26903. This is a follow-up to unifying STW GC and concurrent GC.
Austin Clements [Thu, 16 Aug 2018 16:17:32 +0000 (12:17 -0400)]
runtime: clean up remaining mark work check
Now that STW GC marking is unified with concurrent marking, there
should never be mark work remaining in mark termination. Hence, we can
make that check unconditional.
Updates #26903. This is a follow-up to unifying STW GC and concurrent GC.
Austin Clements [Mon, 13 Aug 2018 20:30:54 +0000 (16:30 -0400)]
runtime: implement STW GC in terms of concurrent GC
Currently, STW GC works very differently from concurrent GC. The
largest differences in that in concurrent GC, all marking work is done
by background mark workers during the mark phase, while in STW GC, all
marking work is done by gchelper during the mark termination phase.
This is a consequence of the evolution of Go's GC from a STW GC by
incrementally moving work from STW mark termination into concurrent
mark. However, at this point, the STW code paths exist only as a
debugging mode. Having separate code paths for this increases the
maintenance burden and complexity of the garbage collector. At the
same time, these code paths aren't tested nearly as well, making it
far more likely that they will bit-rot.
This CL reverses the relationship between STW GC, by re-implementing
STW GC in terms of concurrent GC.
This builds on the new scheduled support for disabling user goroutine
scheduling. During sweep termination, it disables user scheduling, so
when the GC starts the world again for concurrent mark, it's really
only "concurrent" with itself.
There are several code paths that were specific to STW GC that are now
vestigial. We'll remove these in the follow-up CLs.
runtime: support disabling goroutine scheduling by class
This adds support for disabling the scheduling of user goroutines
while allowing system goroutines like the garbage collector to
continue running. User goroutines pass through the usual state
transitions, but if we attempt to actually schedule one, it will get
put on a deferred scheduling list.
Updates #26903. This is preparation for unifying STW GC and concurrent
GC.
Updates #25578. This same mechanism can form the basis for disabling
all but a single user goroutine for the purposes of debugger function
call injection.
Austin Clements [Mon, 13 Aug 2018 20:08:03 +0000 (16:08 -0400)]
runtime: add a more stable isSystemGoroutine mode
Currently, isSystemGoroutine varies on whether it considers the
finalizer goroutine a user goroutine or a system goroutine. For the
next CL, we're going to want to always consider the finalier goroutine
a user goroutine, so add a flag that indicates that.
Updates #26903. This is preparation for unifying STW GC and concurrent
GC.
Austin Clements [Thu, 16 Aug 2018 15:47:36 +0000 (11:47 -0400)]
runtime: remove GODEBUG=gcrescanstacks=1 mode
Currently, setting GODEBUG=gcrescanstacks=1 enables a debugging mode
where the garbage collector re-scans goroutine stacks during mark
termination. This was introduced in Go 1.8 to debug the hybrid write
barrier, but I don't think we ever used it.
Now it's one of the last sources of mark work during mark termination.
This CL removes it.
Updates #26903. This is preparation for unifying STW GC and concurrent
GC.
Austin Clements [Tue, 14 Aug 2018 21:04:04 +0000 (17:04 -0400)]
runtime: avoid using STW GC mechanism for checkmarks mode
Currently, checkmarks mode uses the full STW GC infrastructure to
perform mark checking. We're about to remove that infrastructure and,
furthermore, since checkmarks is about doing the simplest thing
possible to check concurrent GC, it's valuable for it to be simpler.
Hence, this CL makes checkmarks even simpler by making it non-parallel
and divorcing it from the STW GC infrastructure (including the
gchelper mechanism).
Updates #26903. This is preparation for unifying STW GC and concurrent
GC.
Austin Clements [Fri, 3 Aug 2018 21:13:09 +0000 (17:13 -0400)]
runtime: eliminate mark 2 and fix mark termination race
The mark 2 phase was originally introduced as a way to reduce the
chance of entering STW mark termination while there was still marking
work to do. It works by flushing and disabling all local work caches
so that all enqueued work becomes immediately globally visible.
However, mark 2 is not only slow–disabling caches makes marking and
the write barrier both much more expensive–but also imperfect. There
is still a rare but possible race (~once per all.bash) that can cause
GC to enter mark termination while there is still marking work. This
race is detailed at
https://github.com/golang/proposal/blob/master/design/17503-eliminate-rescan.md#appendix-mark-completion-race
The effect of this is that mark termination must still cope with the
possibility that there may be work remaining after a concurrent mark
phase. Dealing with this increases STW pause time and increases the
complexity of mark termination.
Furthermore, a similar but far more likely race can cause early
transition from mark 1 to mark 2. This is unfortunate because it
causes performance instability because of the cost of mark 2.
This CL fixes this by replacing mark 2 with a distributed termination
detection algorithm. This algorithm is correct, so it eliminates the
mark termination race, and doesn't require disabling local caches. It
ensures that there are no grey objects upon entering mark termination.
With this change, we're one step closer to eliminating marking from
mark termination entirely (it's still used by STW GC and checkmarks
mode).
This CL does not eliminate the gcBlackenPromptly global flag, though
it is always set to false now. It will be removed in a cleanup CL.
This led to only minor variations in the go1 benchmarks
(https://perf.golang.org/search?q=upload:20180909.1) and compilebench
benchmarks (https://perf.golang.org/search?q=upload:20180910.2).
This significantly improves performance of the garbage benchmark, with
no impact on STW times:
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.21ms ± 1% 2.05ms ± 1% -7.38% (p=0.000 n=18+19)
Garbage/benchmem-MB=1024-12 2.30ms ±16% 2.20ms ± 7% -4.51% (p=0.001 n=20+20)
name old STW-ns/GC new STW-ns/GC delta
Garbage/benchmem-MB=64-12 138k ±44% 141k ±23% ~ (p=0.309 n=19+20)
Garbage/benchmem-MB=1024-12 159k ±25% 178k ±98% ~ (p=0.798 n=16+18)
name old STW-ns/op new STW-ns/op delta
Garbage/benchmem-MB=64-12 4.42k ±44% 4.24k ±23% ~ (p=0.531 n=19+20)
Garbage/benchmem-MB=1024-12 591 ±24% 636 ±111% ~ (p=0.309 n=16+18)
Austin Clements [Wed, 15 Aug 2018 20:19:21 +0000 (16:19 -0400)]
runtime: remove GODEBUG=gctrace=2 mode
It turns out if you set GODEBUG=gctrace=2, it enables an obscure
debugging mode that, in addition to printing gctrace statistics, also
does a second STW GC following each regular GC. This debugging mode
has long since lost its value (you could maybe use it to analyze
floating garbage, except that we don't print the gctrace line on the
second GC), and it interferes substantially with the operation of the
GC by messing up the statistics used to schedule GCs.
It's also a source of mark termination GC work when we're in
concurrent GC mode, so it's going to interfere with eliminating mark
2. And it's going to get in the way of unifying STW and concurrent GC.
This CL removes this debugging mode.
Updates #26903. This is preparation for eliminating mark 2 and
unifying STW GC and concurrent GC.
Austin Clements [Fri, 3 Aug 2018 18:56:55 +0000 (14:56 -0400)]
runtime: flush write barrier buffer to create work
Currently, if the gcWork runs out of work, we'll fall out of the GC
worker, even though flushing the write barrier buffer could produce
more work. While this is not a correctness issue, it can lead to
premature mark 2 or mark termination.
Fix this by flushing the write barrier buffer if the local gcWork runs
out of work and then checking the local gcWork again.
This reduces the number of premature mark terminations during all.bash
by about a factor of 10.
Updates #26903. This is preparation for eliminating mark 2.
Daniel Theophanes [Sun, 30 Sep 2018 05:10:43 +0000 (22:10 -0700)]
database/sql: correctly report MaxIdleClosed stat
Previously the MaxIdleClosed counter was incremented when added
to the free connection list, rather then when it wasn't added
to the free connection list. Flip this logic to correct.
Richard Musiol [Tue, 2 Oct 2018 17:11:14 +0000 (19:11 +0200)]
misc/wasm: add mention of polyfill for Edge support
Edge supports WebAssembly but not TextEncoder or TextDecoder.
This change adds a comment pointing to a polyfill that could
be used. The polyfill is not added by default, because we want to
let the user decide if/how to include the polyfill.
Alex Brainman [Sun, 30 Sep 2018 06:07:27 +0000 (16:07 +1000)]
net: skip TestUnixConnLocalWindows on windows/386
Recent CL 125456 implemented Unix Socket functionality on windows.
But that functionality does not appear to be working when 32-bit
code is used. So disable TestUnixConnLocalWindows.
windows/386 builder does not appear to be complaining about
TestUnixConnLocalWindows, because new functionality requires
Windows 10 Build 17063. windows/386 builder uses Windows 2008.
Fixes #27943
Change-Id: Iea91b86aaa124352d198ca0cd03fff1e7542f949
Reviewed-on: https://go-review.googlesource.com/138676
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Yasuhiro MATSUMOTO <mattn.jp@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Wed, 19 Sep 2018 23:20:51 +0000 (16:20 -0700)]
cmd/compile/internal/gc: add alternative node dumper for debugging
dump/fdump is a reflection-based data structure dumper slightly
customized for the compiler's Node data structure. It dumps the
transitivle closure of Node (and other) data structures using a
recursive descent depth first traversal and permits filtering
options (recursion depth limitation, filtering of struct fields).
I have been using it to diagnose compiler bugs and found it more
useful than the existing node printing code in some cases because
field filtering reduces the output to the interesting parts.
No impact on rest of compiler if functions are not called (which
they only should during a debugging session).
Change-Id: I79d7227f10dd78dbd4bbcdf204db236102fc97a7
Reviewed-on: https://go-review.googlesource.com/136397 Reviewed-by: Alan Donovan <adonovan@google.com>
Ian Davis [Sun, 23 Sep 2018 15:47:05 +0000 (16:47 +0100)]
image: optimize bounds checking for At and Set methods
Use a subslice of the pixel data to give the compiler hints
for bounds checking. Only do this for image formats that
require 4 or more slice reads/writes.
Reason for revert:
Failing Darwin-arm builds because that testing environment does not access testdata
from sibling directories. A future change will likely be made to move this testdata
out of src/testdata to create a solution that doesn't require the single-file directory.
Updates #27151
Change-Id: I8dbf5dd9512c94a605ee749ff4655cb00b0de686
Reviewed-on: https://go-review.googlesource.com/138737 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Keith Randall [Tue, 25 Sep 2018 22:54:11 +0000 (15:54 -0700)]
reflect: ensure correct scanning of return values
During a call to a reflect-generated function or method (via
makeFuncStub or methodValueCall), when should we scan the return
values?
When we're starting a reflect call, the space on the stack for the
return values is not initialized yet, as it contains whatever junk was
on the stack of the caller at the time. The return space must not be
scanned during a GC.
When we're finishing a reflect call, the return values are
initialized, and must be scanned during a GC to make sure that any
pointers in the return values are found and their referents retained.
When the GC stack walk comes across a reflect call in progress on the
stack, it needs to know whether to scan the results or not. It doesn't
know the progress of the reflect call, so it can't decide by
itself. The reflect package needs to tell it.
This CL adds another slot in the frame of makeFuncStub and
methodValueCall so we can put a boolean in there which tells the
runtime whether to scan the results or not.
This CL also adds the args length to reflectMethodValue so the
runtime can restrict its scanning to only the args section (not the
results) if the reflect package says the results aren't ready yet.
Do a delicate dance in the reflect package to set the "results are
valid" bit. We need to make sure we set the bit only after we've
copied the results back to the stack. But we must set the bit before
we drop reflect's copy of the results. Otherwise, we might have a
state where (temporarily) no one has a live copy of the results.
That's the state we were observing in issue #27695 before this CL.
The bitmap used by the runtime currently contains only the args.
(Actually, it contains all the bits, but the size is set so we use
only the args portion.) This is safe for early in a reflect call, but
unsafe late in a reflect call. The test issue27695.go demonstrates
this unsafety. We change the bitmap to always include both args
and results, and decide at runtime which portion to use.
issue27695.go only has a test for method calls. Function calls were ok
because there wasn't a safepoint between when reflect dropped its copy
of the return values and when the caller is resumed. This may change
when we introduce safepoints everywhere.
This truncate-to-only-the-args was part of CL 9888 (in 2015). That
part of the CL fixed the problem demonstrated in issue27695b.go but
introduced the problem demonstrated in issue27695.go.
TODO, in another CL: simplify FuncLayout and its test. stack return
value is now identical to frametype.ptrdata + frametype.gcdata.
Alex Brainman [Sat, 8 Sep 2018 06:05:29 +0000 (16:05 +1000)]
os: use FILE_FLAG_OPEN_REPARSE_POINT in SameFile
SameFile opens file to discover identifier and volume serial
number that uniquely identify the file. SameFile uses Windows
CreateFile API to open the file, and that works well for files
and directories. But CreateFile always follows symlinks, so
SameFile always opens symlink target instead of symlink itself.
This CL uses FILE_FLAG_OPEN_REPARSE_POINT flag to adjust
CreateFile behavior when handling symlinks.
As per https://docs.microsoft.com/en-us/windows/desktop/FileIO/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted
"... If FILE_FLAG_OPEN_REPARSE_POINT is specified and:
If an existing file is opened and it is a symbolic link, the handle
returned is a handle to the symbolic link. ...".
I also added new tests for both issue #21854 and #27225.
Issue #27225 is still to be fixed, so skipping the test on
windows for the moment.
Fixes #21854
Updates #27225
Change-Id: I8aaa13ad66ce3b4074991bb50994d2aeeeaa7c95
Reviewed-on: https://go-review.googlesource.com/134195
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
compress: move benchmark text from src/testdata to src/compress/testdata
This text is used mainly for benchmark compression testing, and in one
net test. The text was prevoiusly in a src/testdata directory, but since
that directory would only include one file, the text is moved to the
existing src/compression/testdata directory.
This does not cause any change to the benchmark results.
mcache.refill doesn't need to run on the system stack; it just needs
to be non-preemptible. Its only caller, mcache.nextFree, also needs to
be non-preemptible, so we can remove the unnecessary systemstack
switch.
runtime: remove redundant locking in mcache.refill
mcache.refill acquires g.m.locks, which is pointless because the
caller itself absolutely must have done so already to prevent
ownership of mcache from shifting.
Also, mcache.refill's documentation is generally a bit out-of-date, so
this cleans this up.
doc: remove "known bug" about global variables in debug_info.
This hasn't been true at least since 1.4. Until golang.org/cl/137235
they were lumped together into a random compile unit, now they are
assigned to the correct one.
Change-Id: Ib66539bd67af3e9daeecac8bf5f32c10e62e11b1
Reviewed-on: https://go-review.googlesource.com/138415 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: David Chase <drchase@google.com>