Otherwise the new numbered directories like b028/ appear in the objects,
and they can change from run to run.
Fixes#22514.
Change-Id: I8d0cf65f3622e48b2547d5757febe0ee1301e2ed
Reviewed-on: https://go-review.googlesource.com/74791
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Since Go1.8, different types of GC mark workers were annotated and the
annotation strings were recorded during StartTrace. This change fixes
two issues around the use of traceString from StartTrace here.
1) "failed to parse trace: no consistent ordering of events possible"
This issue is a result of a missing 'batch' event entry. For efficient
tracing, tracer maintains system allocated buffers and once a buffer
is full, it is Flushed out for writing. Moreover, tracing assumes all
the records in the same buffer (batch) are already ordered and implements
more optimization in encoding and defers the completing order
reconstruction till the trace parsing time. Thus, when a Flush happens
and a new buffer is used, the new buffer should contain an event to
indicate the start of a new batch. Before this CL, the batch entry was
written only by traceEvent only when the buffer position is 0 and
wasn't written when flush occurs during traceString.
This CL fixes it by moving the batch entry write to the traceFlush.
2) crash during tracing due to invalid memory access, or during parsing
due to duplicate string entries
This issue is a result of memory allocation during traceString calls.
Execution tracer traces some memory allocation activities. Before this
CL, traceString took the buffer address (*traceBuf) and mutated the buffer.
If memory tracing occurs in the meantime from the same P, the allocation
tracing (traceEvent) will take the same buffer address through the pointer
to the buffer address (**traceBuf), and mutate the buffer.
As a result, one of the followings can happen:
- the allocation record is overwritten by the following trace string
record (data loss)
- if buffer flush occurs during the allocation tracing, traceString
will attempt to write the string record to the old buffer and
eventually causes invalid memory access crash.
- or flush on the same buffer can occur twice (once from the memory
allocation, and once from the string record write), and in this case
the trace can contain the same data twice and the parse will complain
about duplicate string record entries.
This CL fixes the second issue by making the traceString take
**traceBuf (*traceBufPtr).
Change-Id: I24f629758625b38e1916fbfc7d7be6ea210586af
Reviewed-on: https://go-review.googlesource.com/50873
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently, both the background mark worker and the goal GC CPU are
both fixed at 25%. The trigger controller's goal is to achieve the
goal CPU usage, and with the previous commit it can actually achieve
this. But this means there are *no* assists, which sounds ideal but
actually causes problems for the trigger controller. Since the
controller can't lower CPU usage below the background mark worker CPU,
it saturates at the CPU goal and no longer gets feedback, which
translates into higher variability in heap growth.
This commit fixes this by allowing assists 5% CPU beyond the 25% fixed
background mark. This avoids saturating the trigger controller, since
it can now get feedback from both sides of the CPU goal. This leads to
low variability in both CPU usage and heap growth, at the cost of
reintroducing a low rate of mark assists.
We also experimented with 20% background plus 5% assist, but 25%+5%
clearly performed better in benchmarks.
Updates #14951.
Updates #14812.
Updates #18534.
Combined with the previous CL, this significantly improves tail
mutator utilization in the x/bechmarks garbage benchmark. On a sample
trace, it increased the 99.9%ile mutator utilization at 10ms from 26%
to 59%, and at 5ms from 17% to 52%. It reduced the 99.9%ile zero
utilization window from 2ms to 700µs. It also helps the mean mutator
utilization: it increased the 10s mutator utilization from 83% to 94%.
The minimum mutator utilization is also somewhat improved, though
there is still some unknown artifact that causes a miniscule fraction
of mutator assists to take 5--10ms (in fact, there was exactly one
10ms mutator assist in my sample trace).
This has no significant effect on the throughput of the
github.com/dr2chase/bent benchmarks-50.
This has little effect on the go1 benchmarks (and the slight overall
improvement makes up for the slight overall slowdown from the previous
commit):
name old time/op new time/op delta
BinaryTree17-12 2.40s ± 0% 2.41s ± 1% +0.26% (p=0.010 n=18+19)
Fannkuch11-12 2.95s ± 0% 2.93s ± 0% -0.62% (p=0.000 n=18+15)
FmtFprintfEmpty-12 42.2ns ± 0% 42.3ns ± 1% +0.37% (p=0.001 n=15+14)
FmtFprintfString-12 67.9ns ± 2% 67.2ns ± 3% -1.03% (p=0.002 n=20+18)
FmtFprintfInt-12 75.6ns ± 3% 76.8ns ± 2% +1.59% (p=0.000 n=19+17)
FmtFprintfIntInt-12 123ns ± 1% 124ns ± 1% +0.77% (p=0.000 n=17+14)
FmtFprintfPrefixedInt-12 148ns ± 1% 150ns ± 1% +1.28% (p=0.000 n=20+20)
FmtFprintfFloat-12 212ns ± 0% 211ns ± 1% -0.67% (p=0.000 n=16+17)
FmtManyArgs-12 499ns ± 1% 500ns ± 0% +0.23% (p=0.004 n=19+16)
GobDecode-12 6.49ms ± 1% 6.51ms ± 1% +0.32% (p=0.008 n=19+19)
GobEncode-12 5.47ms ± 0% 5.43ms ± 1% -0.68% (p=0.000 n=19+20)
Gzip-12 220ms ± 1% 216ms ± 1% -1.66% (p=0.000 n=20+19)
Gunzip-12 38.8ms ± 0% 38.5ms ± 0% -0.80% (p=0.000 n=19+20)
HTTPClientServer-12 78.5µs ± 1% 78.1µs ± 1% -0.53% (p=0.008 n=20+19)
JSONEncode-12 12.2ms ± 0% 11.9ms ± 0% -2.38% (p=0.000 n=17+19)
JSONDecode-12 52.3ms ± 0% 53.3ms ± 0% +1.84% (p=0.000 n=19+20)
Mandelbrot200-12 3.69ms ± 0% 3.69ms ± 0% -0.19% (p=0.000 n=19+19)
GoParse-12 3.17ms ± 1% 3.19ms ± 1% +0.61% (p=0.000 n=20+20)
RegexpMatchEasy0_32-12 73.7ns ± 0% 73.2ns ± 1% -0.66% (p=0.000 n=17+20)
RegexpMatchEasy0_1K-12 238ns ± 0% 239ns ± 0% +0.32% (p=0.000 n=17+16)
RegexpMatchEasy1_32-12 69.1ns ± 1% 69.2ns ± 1% ~ (p=0.669 n=19+13)
RegexpMatchEasy1_1K-12 365ns ± 1% 367ns ± 1% +0.49% (p=0.000 n=19+19)
RegexpMatchMedium_32-12 104ns ± 1% 105ns ± 1% +1.33% (p=0.000 n=16+20)
RegexpMatchMedium_1K-12 33.6µs ± 3% 34.1µs ± 4% +1.67% (p=0.001 n=20+20)
RegexpMatchHard_32-12 1.67µs ± 1% 1.62µs ± 1% -2.78% (p=0.000 n=18+17)
RegexpMatchHard_1K-12 50.3µs ± 2% 48.7µs ± 1% -3.09% (p=0.000 n=19+18)
Revcomp-12 384ms ± 0% 386ms ± 0% +0.59% (p=0.000 n=19+19)
Template-12 61.1ms ± 1% 60.5ms ± 1% -1.02% (p=0.000 n=19+20)
TimeParse-12 307ns ± 0% 303ns ± 1% -1.23% (p=0.000 n=19+15)
TimeFormat-12 323ns ± 0% 323ns ± 0% -0.12% (p=0.011 n=15+20)
[Geo mean] 47.1µs 47.0µs -0.20%
https://perf.golang.org/search?q=upload:20171030.4
It slightly improve the performance the x/benchmarks:
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.29ms ± 3% 2.22ms ± 2% -2.97% (p=0.000 n=18+18)
Garbage/benchmem-MB=64-12 2.24ms ± 2% 2.21ms ± 2% -1.64% (p=0.000 n=18+18)
HTTP-12 12.6µs ± 1% 12.6µs ± 1% ~ (p=0.690 n=19+17)
JSON-12 11.3ms ± 2% 11.3ms ± 1% ~ (p=0.163 n=17+18)
and fixes some of the heap size bloat caused by the previous commit:
name old peak-RSS-bytes new peak-RSS-bytes delta
Garbage/benchmem-MB=1024-12 1.88G ± 2% 1.77G ± 2% -5.52% (p=0.000 n=20+18)
Garbage/benchmem-MB=64-12 248M ± 8% 226M ± 5% -8.93% (p=0.000 n=20+20)
HTTP-12 47.0M ±27% 47.2M ±12% ~ (p=0.512 n=20+20)
JSON-12 206M ±11% 206M ±10% ~ (p=0.841 n=20+20)
https://perf.golang.org/search?q=upload:20171030.5
Combined with the change to add a soft goal in the previous commit,
the achieves a decent performance improvement on the garbage
benchmark:
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.40ms ± 4% 2.22ms ± 2% -7.40% (p=0.000 n=19+18)
Garbage/benchmem-MB=64-12 2.23ms ± 1% 2.21ms ± 2% -1.06% (p=0.000 n=19+18)
HTTP-12 12.5µs ± 1% 12.6µs ± 1% ~ (p=0.330 n=20+17)
JSON-12 11.1ms ± 1% 11.3ms ± 1% +1.87% (p=0.000 n=16+18)
https://perf.golang.org/search?q=upload:20171030.6
Change-Id: If04ddb57e1e58ef2fb9eec54c290eb4ae4bea121
Reviewed-on: https://go-review.googlesource.com/59971
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, GC pacing is based on a single hard heap limit computed
based on GOGC. In order to achieve this hard limit, assist pacing
makes the conservative assumption that the entire heap is live.
However, in the steady state (with GOGC=100), only half of the heap is
live. As a result, the garbage collector works twice as hard as
necessary and finishes half way between the trigger and the goal.
Since this is a stable state for the trigger controller, this repeats
from cycle to cycle. Matters are even worse if GOGC is higher. For
example, if GOGC=200, only a third of the heap is live in steady
state, so the GC will work three times harder than necessary and
finish only a third of the way between the trigger and the goal.
Since this causes the garbage collector to consume ~50% of the
available CPU during marking instead of the intended 25%, about 25% of
the CPU goes to mutator assists. This high mutator assist cost causes
high mutator latency variability.
This commit improves the situation by separating the heap goal into
two goals: a soft goal and a hard goal. The soft goal is set based on
GOGC, just like the current goal is, and the hard goal is set at a 10%
larger heap than the soft goal. Prior to the soft goal, assist pacing
assumes the heap is in steady state (e.g., only half of it is live).
Between the soft goal and the hard goal, assist pacing switches to the
current conservative assumption that the entire heap is live.
In benchmarks, this nearly eliminates mutator assists. However, since
background marking is fixed at 25% CPU, this causes the trigger
controller to saturate, which leads to somewhat higher variability in
heap size. The next commit will address this.
The lower CPU usage of course leads to longer mark cycles, though
really it means the mark cycles are as long as they should have been
in the first place. This does, however, lead to two potential
down-sides compared to the current pacing policy: 1. the total
overhead of the write barrier is higher because it's enabled more of
the time and 2. the heap size may be larger because there's more
floating garbage. We addressed 1 by significantly improving the
performance of the write barrier in the preceding commits. 2 can be
demonstrated in intense GC benchmarks, but doesn't seem to be a
problem in any real applications.
Updates #14951.
Updates #14812 (fixes?).
Fixes#18534.
This has no significant effect on the throughput of the
github.com/dr2chase/bent benchmarks-50.
This has little overall throughput effect on the go1 benchmarks:
name old time/op new time/op delta
BinaryTree17-12 2.41s ± 0% 2.40s ± 0% -0.22% (p=0.007 n=20+18)
Fannkuch11-12 2.95s ± 0% 2.95s ± 0% +0.07% (p=0.003 n=17+18)
FmtFprintfEmpty-12 41.7ns ± 3% 42.2ns ± 0% +1.17% (p=0.002 n=20+15)
FmtFprintfString-12 66.5ns ± 0% 67.9ns ± 2% +2.16% (p=0.000 n=16+20)
FmtFprintfInt-12 77.6ns ± 2% 75.6ns ± 3% -2.55% (p=0.000 n=19+19)
FmtFprintfIntInt-12 124ns ± 1% 123ns ± 1% -0.98% (p=0.000 n=18+17)
FmtFprintfPrefixedInt-12 151ns ± 1% 148ns ± 1% -1.75% (p=0.000 n=19+20)
FmtFprintfFloat-12 210ns ± 1% 212ns ± 0% +0.75% (p=0.000 n=19+16)
FmtManyArgs-12 501ns ± 1% 499ns ± 1% -0.30% (p=0.041 n=17+19)
GobDecode-12 6.50ms ± 1% 6.49ms ± 1% ~ (p=0.234 n=19+19)
GobEncode-12 5.43ms ± 0% 5.47ms ± 0% +0.75% (p=0.000 n=20+19)
Gzip-12 216ms ± 1% 220ms ± 1% +1.71% (p=0.000 n=19+20)
Gunzip-12 38.6ms ± 0% 38.8ms ± 0% +0.66% (p=0.000 n=18+19)
HTTPClientServer-12 78.1µs ± 1% 78.5µs ± 1% +0.49% (p=0.035 n=20+20)
JSONEncode-12 12.1ms ± 0% 12.2ms ± 0% +1.05% (p=0.000 n=18+17)
JSONDecode-12 53.0ms ± 0% 52.3ms ± 0% -1.27% (p=0.000 n=19+19)
Mandelbrot200-12 3.74ms ± 0% 3.69ms ± 0% -1.17% (p=0.000 n=18+19)
GoParse-12 3.17ms ± 1% 3.17ms ± 1% ~ (p=0.569 n=19+20)
RegexpMatchEasy0_32-12 73.2ns ± 1% 73.7ns ± 0% +0.76% (p=0.000 n=18+17)
RegexpMatchEasy0_1K-12 239ns ± 0% 238ns ± 0% -0.27% (p=0.000 n=13+17)
RegexpMatchEasy1_32-12 69.0ns ± 2% 69.1ns ± 1% ~ (p=0.404 n=19+19)
RegexpMatchEasy1_1K-12 367ns ± 1% 365ns ± 1% -0.60% (p=0.000 n=19+19)
RegexpMatchMedium_32-12 105ns ± 1% 104ns ± 1% -1.24% (p=0.000 n=19+16)
RegexpMatchMedium_1K-12 34.1µs ± 2% 33.6µs ± 3% -1.60% (p=0.000 n=20+20)
RegexpMatchHard_32-12 1.62µs ± 1% 1.67µs ± 1% +2.75% (p=0.000 n=18+18)
RegexpMatchHard_1K-12 48.8µs ± 1% 50.3µs ± 2% +3.07% (p=0.000 n=20+19)
Revcomp-12 386ms ± 0% 384ms ± 0% -0.57% (p=0.000 n=20+19)
Template-12 59.9ms ± 1% 61.1ms ± 1% +2.01% (p=0.000 n=20+19)
TimeParse-12 301ns ± 2% 307ns ± 0% +2.11% (p=0.000 n=20+19)
TimeFormat-12 323ns ± 0% 323ns ± 0% ~ (all samples are equal)
[Geo mean] 47.0µs 47.1µs +0.23%
https://perf.golang.org/search?q=upload:20171030.1
Likewise, the throughput effect on the x/benchmarks is minimal (and
reasonably positive on the garbage benchmark with a large heap):
name old time/op new time/op delta
Garbage/benchmem-MB=1024-12 2.40ms ± 4% 2.29ms ± 3% -4.57% (p=0.000 n=19+18)
Garbage/benchmem-MB=64-12 2.23ms ± 1% 2.24ms ± 2% +0.59% (p=0.016 n=19+18)
HTTP-12 12.5µs ± 1% 12.6µs ± 1% ~ (p=0.326 n=20+19)
JSON-12 11.1ms ± 1% 11.3ms ± 2% +2.15% (p=0.000 n=16+17)
It does increase the heap size of the garbage benchmarks, but seems to
have relatively little impact on more realistic programs. Also, we'll
gain some of this back with the next commit.
name old peak-RSS-bytes new peak-RSS-bytes delta
Garbage/benchmem-MB=1024-12 1.21G ± 1% 1.88G ± 2% +55.59% (p=0.000 n=19+20)
Garbage/benchmem-MB=64-12 168M ± 3% 248M ± 8% +48.08% (p=0.000 n=18+20)
HTTP-12 45.6M ± 9% 47.0M ±27% ~ (p=0.925 n=20+20)
JSON-12 193M ±11% 206M ±11% +7.06% (p=0.001 n=20+20)
https://perf.golang.org/search?q=upload:20171030.2
Change-Id: Ic78904135f832b4d64056cbe734ab979f5ad9736
Reviewed-on: https://go-review.googlesource.com/59970
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The compiler's instrumentation pass has some out-of-date comments
about the write barrier and some confusing comments about
typedslicecopy. Update these comments and add a comment to
typedslicecopy explaining why it's manually instrumented while none of
the other operations are.
Change-Id: I024e5361d53f1c3c122db0c85155368a30cabd6b
Reviewed-on: https://go-review.googlesource.com/74430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The content-based staleness code means that
go run -gcflags=-l helloworld.go
recompiles all of helloworld.go's dependencies with -gcflags=-l,
whereas before it would have assumed installed packages were
up-to-date. In this test, that means every race iteration rebuilds
the runtime and maybe a few other packages. Instead, install them
to a temporary location for reuse.
This speeds the test from 17s to 9s on my MacBook Pro.
Change-Id: Ied136ce72650261083bb19cc7dee38dac0ad05ca
Reviewed-on: https://go-review.googlesource.com/73992
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This cuts 23 seconds from all.bash on my MacBook Pro.
Change-Id: Ibc4d7c01660b9e9ebd088dd55ba993f0d7ec6aa3
Reviewed-on: https://go-review.googlesource.com/73991
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If the go install doesn't use the same flags as the main build
it can overwrite the installed standard library, leading to
flakiness and slow future tests.
Force uses of 'go install' etc to propagate $GO_GCFLAGS
or disable them entirely, to avoid problems.
As I understand it, the main place this happens is the ssacheck builder.
If there are other uses that need to run some of the now-disabled
tests we can reenable fixed tests in followup CLs.
Change-Id: Ib860a253539f402f8a96a3c00ec34f0bbf137c9a
Reviewed-on: https://go-review.googlesource.com/74470
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Memory accesses on z are at least as ordered as they are on AMD64.
Change-Id: Ia515430e571ebd07e9314de05c54dc992ab76b95
Reviewed-on: https://go-review.googlesource.com/74010
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
This modifies bulkBarrierPreWrite to use the buffered write barrier
instead of the eager write barrier. This reduces the number of system
stack switches and sanity checks by a factor of the buffer size
(currently 256). This affects both typedmemmove and typedmemclr.
Since this is purely a runtime change, it applies to all arches
(unlike the pointer write barrier).
name old time/op new time/op delta
BulkWriteBarrier-12 7.33ns ± 6% 4.46ns ± 9% -39.10% (p=0.000 n=20+19)
Updates #22460.
Change-Id: I6a686a63bbf08be02b9b97250e37163c5a90cdd8
Reviewed-on: https://go-review.googlesource.com/73832
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, typedslicecopy meticulously performs a typedmemmove on
every element of the slice. This probably used to be necessary because
we only had an individual element's type, but now we use the heap
bitmap, so we only need to know whether the type has any pointers and
how big it is. Hence, this CL rewrites typedslicecopy to simply
perform one bulk barrier and one memmove.
This also has a side-effect of eliminating two unnecessary write
barriers per slice element that were coming from updates to dstp and
srcp, which were stored in the parent stack frame. However, most of
the win comes from eliminating the loops.
name old time/op new time/op delta
BulkWriteBarrier-12 7.83ns ±10% 7.33ns ± 6% -6.45% (p=0.000 n=20+20)
Updates #22460.
Change-Id: Id3450e9f36cc8e0892f268319b136f0d8f5464b8
Reviewed-on: https://go-review.googlesource.com/73831
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This adds a benchmark of typedslicecopy and its bulk write barriers.
For #22460.
Change-Id: I439ca3b130bb22944468095f8f18b464e5bb43ca
Reviewed-on: https://go-review.googlesource.com/74051
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This implements runtime support for buffered write barriers on amd64.
The buffered write barrier has a fast path that simply enqueues
pointers in a per-P buffer. Unlike the current write barrier, this
fast path is *not* a normal Go call and does not require the compiler
to spill general-purpose registers or put arguments on the stack. When
the buffer fills up, the write barrier takes the slow path, which
spills all general purpose registers and flushes the buffer. We don't
allow safe-points or stack splits while this frame is active, so it
doesn't matter that we have no type information for the spilled
registers in this frame.
One minor complication is cgocheck=2 mode, which uses the write
barrier to detect Go pointers being written to non-Go memory. We
obviously can't buffer this, so instead we set the buffer to its
minimum size, forcing the write barrier into the slow path on every
call. For this specific case, we pass additional information as
arguments to the flush function. This also requires enabling the cgo
write barrier slightly later during runtime initialization, after Ps
(and the per-P write barrier buffers) have been initialized.
The code in this CL is not yet active. The next CL will modify the
compiler to generate calls to the new write barrier.
This reduces the average cost of the write barrier by roughly a factor
of 4, which will pay for the cost of having it enabled more of the
time after we make the GC pacer less aggressive. (Benchmarks will be
in the next CL.)
Updates #14951.
Updates #22460.
Change-Id: I396b5b0e2c5e5c4acfd761a3235fd15abadc6cb1
Reviewed-on: https://go-review.googlesource.com/73711
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently systemstack always calls its argument, even if we're already
on the system stack. Unfortunately, traceback with _TraceJump stops at
the first systemstack it sees, which often cuts off runtime stacks
early in profiles.
Fix this by performing a tail call if we're already on the system
stack. This eliminates it from the traceback entirely, so it won't
stop prematurely (or all get mushed into a single node in the profile
graph).
Change-Id: Ibc69e8765e899f8d3806078517b8c7314da196f4
Reviewed-on: https://go-review.googlesource.com/74050
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Errors occur in runtime test testCgoPprofPIE when the test
is built by passing -pie to the external linker with code
that was not built as PIC. This occurs on ppc64le because
non-PIC is the default, and fails only on newer distros
where the address range used for programs is high enough
to cause relocation overflow. This test should be built
with -buildmode=pie since that correctly generates PIC
with -pie.
Related issues are #21954 and #22126.
Updates #22459
Change-Id: Ib641440bc9f94ad2b97efcda14a4b482647be8f7
Reviewed-on: https://go-review.googlesource.com/73970
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
recordspan has two remaining write barriers from writing to the
pointer to the backing store of h.allspans. However, h.allspans is
always backed by off-heap memory, so let the compiler know this.
Unfortunately, this isn't quite as clean as most go:notinheap uses
because we can't directly name the backing store of a slice, but we
can get it done with some judicious casting.
For #22460.
Change-Id: I296f92fa41cf2cb6ae572b35749af23967533877
Reviewed-on: https://go-review.googlesource.com/73414
Reviewed-by: Rick Hudson <rlh@golang.org>
We're about to start tracking nowritebarrierrec through systemstack
calls, which detects that we're calling markroot (which has write
barriers) from gchelper, which is called from the scheduler during STW
apparently without a P.
But it turns out that func helpgc, which wakes up blocked Ms to run
gchelper, installs a P for gchelper to use. This means there *is* a P
when gchelper runs, so it is allowed to have write barriers. Tell the
compiler this by marking gchelper go:yeswritebarrierrec. Also,
document the call to gchelper so I don't have to spend another half a
day puzzling over how on earth this could possibly work before
discovering the spooky action-at-a-distance in helpgc.
Updates #22384.
For #22460.
Change-Id: I7394c9b4871745575f87a2d4fbbc5b8e54d669f7
Reviewed-on: https://go-review.googlesource.com/72772
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We're about to start tracking nowritebarrierrec through systemstack
calls, which will reveal write barriers in persistentalloc prohibited
by various callers.
The pointers manipulated by persistentalloc are always to off-heap
memory, so this removes these write barriers statically by introducing
a new go:notinheap type to represent generic off-heap memory.
Updates #22384.
For #22460.
Change-Id: Id449d9ebf145b14d55476a833e7f076b0d261d57
Reviewed-on: https://go-review.googlesource.com/72771
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We're about to start tracking nowritebarrierrec through systemstack
calls, which will reveal write barriers in startpanic_m prohibited by
various callers.
We actually can allow write barriers here because the write barrier is
a no-op when we're panicking. Let the compiler know.
Updates #22384.
For #22460.
Change-Id: Ifb3a38d3dd9a4125c278c3680f8648f987a5b0b8
Reviewed-on: https://go-review.googlesource.com/72770
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently most of these are marked go:nowritebarrier as a hint, but
it's actually important that these not invoke write barriers
recursively. The danger is that some gcWork method would invoke the
write barrier while the gcWork is in an inconsistent state and that
the write barrier would in turn invoke some other gcWork method, which
would crash or permanently corrupt the gcWork. Simply marking the
write barrier itself as go:nowritebarrierrec isn't sufficient to
prevent this if the write barrier doesn't use the outer method.
Thankfully, this doesn't cause any build failures, so we were getting
this right. :)
For #22460.
Change-Id: I35a7292a584200eb35a49507cd3fe359ba2206f6
Reviewed-on: https://go-review.googlesource.com/72554
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, newstack and gogo have write barriers for maintaining the
context register saved in g.sched.ctxt. This is troublesome, because
newstack can be called from go:nowritebarrierrec places that can't
allow write barriers. It happens to be benign because g.sched.ctxt
will always be nil on entry to newstack *and* it so happens the
incoming ctxt will also always be nil in these contexts (I
think/hope), but this is playing with fire. It's also desirable to
mark newstack go:nowritebarrierrec to prevent any other, non-benign
write barriers from creeping in, but we can't do that right now
because of this one write barrier.
Fix all of this by observing that g.sched.ctxt is really just a saved
live pointer register. Hence, we can shade it when we scan g's stack
and otherwise move it back and forth between the actual context
register and g.sched.ctxt without write barriers. This means we can
save it in morestack along with all of the other g.sched, eliminate
the save from newstack along with its troublesome write barrier, and
eliminate the shenanigans in gogo to invoke the write barrier when
restoring it.
Once we've done all of this, we can mark newstack
go:nowritebarrierrec.
Fixes#22385.
For #22460.
Change-Id: I43c24958e3f6785b53c1350e1e83c2844e0d1522
Reviewed-on: https://go-review.googlesource.com/72553
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TestParallelRWMutexReaders has a non-preemptible loop in it that can
deadlock if GC triggers. "Fix" it like we've fixed similar tests.
Updates #10958.
Change-Id: I13618f522f5ef0c864e7171ad2f655edececacd7
Reviewed-on: https://go-review.googlesource.com/73710
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Otherwise low-res timers cause problems at call sites that expect to
be able to use 0 as meaning "no time set" and therefore expect that
nanotime never returns 0 itself. For example, sched.lastpoll == 0
means no last poll.
Fixes#22394.
Change-Id: Iea28acfddfff6f46bc90f041ec173e0fea591285
Reviewed-on: https://go-review.googlesource.com/73410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The BigEndian constant is only used in boolean context so assign it
boolean constants.
Change-Id: If19d61dd71cdfbffede1d98b401f11e6535fba59
Reviewed-on: https://go-review.googlesource.com/73270
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make netpollopen return what Windows GetLastError API returns.
It is probably copy / paste error from long time ago.
Change-Id: I28f78718c15fef3e8b5f5d11a259533d7e9c6185
Reviewed-on: https://go-review.googlesource.com/72592
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Increasing the map size with the benchmark iteration count
introduced non-linearities and made benchmark runs slow when
increasing benchtime.
Rework the benchmark to use a map size independent of the
iteration count and instead re-fill it when it becomes empty.
Fixes#21546
Change-Id: Iafb6eb225e81830263f30b3aba0d449c361aec32
Reviewed-on: https://go-review.googlesource.com/57650
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On systems that use kqueue, we always register descriptors for both
EVFILT_READ and EVFILT_WRITE. On at least FreeBSD and OpenBSD, when
the write end of a pipe is registered for EVFILT_READ and EVFILT_WRITE
events, and the read end of the pipe is closed, kqueue reports an
EVFILT_READ event with EV_EOF set, but does not report an EVFILT_WRITE
event. Since the write to the pipe is waiting for an EVFILT_WRITE
event, closing the read end of a pipe can cause the write end to hang
rather than attempt another write which will fail with EPIPE.
Fix this by treating EVFILT_READ with EV_EOF set as making both reads
and writes ready to proceed.
The real test for this is in CL 71770, which tests using various
timeouts with pipes.
Updates #22114
Change-Id: Ib23fbaaddbccd8eee77bdf18f27a7f0aa50e2742
Reviewed-on: https://go-review.googlesource.com/71973
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently mmap returns an unsafe.Pointer that encodes OS errors as
values less than 4096. In practice this is okay, but it borders on
being really unsafe: for example, the value has to be checked
immediately after return and if stack copying were ever to observe
such a value, it would panic. It's also not remotely idiomatic.
Fix this by making mmap return a separate pointer value and error,
like a normal Go function.
Updates #22218.
Change-Id: Iefd965095ffc82cc91118872753a5d39d785c3a6
Reviewed-on: https://go-review.googlesource.com/71270
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes Darwin 386 build. It turns out that the Darwin pthread_create
function saves the SSE registers, and therefore requires an aligned stack.
This worked before https://golang.org/cl/70530 because the stack sizes
were chosen to leave the stack aligned.
Change-Id: I911a9e8dcde4e41e595d5ef9b9a1ca733e154de6
Reviewed-on: https://go-review.googlesource.com/71432
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
CL 46037 and CL 46038 implemented termination of
locked OS threads when the goroutine exits.
However, this behavior leads to crashes of Go programs
using runtime.LockOSThread on Plan 9. This is notably
the case of the os/exec and net packages.
This change disables termination of locked OS threads
on Plan 9.
Updates #22227.
Change-Id: If9fa241bff1c0b68e7e9e321e06e5203b3923212
Reviewed-on: https://go-review.googlesource.com/71230
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 46033 added a "template thread" mechanism to
allow creation of thread with a known-good state
from a thread of unknown state.
However, we are experiencing issues on Plan 9
with programs using the os/exec and net package.
These package are relying on runtime.LockOSThread.
Updates #22227.
Change-Id: I85b71580a41df9fe8b24bd8623c064b6773288b0
Reviewed-on: https://go-review.googlesource.com/70231
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Unify the 386 entry point code as much as possible.
The main function could not be unified because on Windows 386 it is
called _main. Putting main in asm_386.s caused multiple definition
errors when using the external linker.
Add the _lib entry point to various operating systems. A future CL
will enable c-archive/c-shared mode for those targets.
Fix _rt0_386_windows_lib_go--it was passing arguments as though it
were amd64.
Change-Id: Ic73f1c95cdbcbea87f633f4a29bbc218a5db4f58
Reviewed-on: https://go-review.googlesource.com/70530
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Use TEXT pseudo-instruction to adjust SP instead of a SUB instruction
so that the assembler knows how to fill in the pcsp table and the frame
description entry correctly.
Updates #21569
Change-Id: I436c840b2af99bbb3042ecd38a7d7c1ab4d7372a
Reviewed-on: https://go-review.googlesource.com/70937
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The code was commented out by https://golang.org/cl/13234050 in 2013.
Let's just remove it.
Change-Id: I46ae1f07386719e991458e782d236214c40bdce1
Reviewed-on: https://go-review.googlesource.com/70770
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
STREX does not permit using the same register for the value to store
and the place where the result is returned. Also the code was wrong
anyhow if the first store failed.
Fixes#22248
Change-Id: I96013497410058514ffcb771c76c86faa1ec559b
Reviewed-on: https://go-review.googlesource.com/70911
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently only a single P can run a fractional mark worker at a time.
This doesn't let us spread out the load, so it gets concentrated on
whatever unlucky P picks up the token to run a fractional worker. This
can significantly delay goroutines on that P.
This commit changes this scheduling rule so each P separately
schedules fractional workers. This can significantly reduce the load
on any individual P and allows workers to self-preempt earlier. It
does have the downside that it's possible for all Ps to be in
fractional workers simultaneously (an effect STW).
Updates #21698.
Change-Id: Ia1e300c422043fa62bb4e3dd23c6232d81e4419c
Reviewed-on: https://go-review.googlesource.com/68574
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently fractional workers run until preempted by the scheduler,
which means they typically run for 20ms. During this time, all other
goroutines on that P are blocked, which can introduce significant
latency variance.
This modifies fractional workers to self-preempt shortly after
achieving the fractional utilization goal. In practice this means they
preempt much sooner, and the scale of their preemption is on the order
of how often the user goroutine block (so, if the application is
compute-bound, the fractional workers will also run for long times,
but if the application blocks frequently, the fractional workers will
also preempt quickly).
Fixes#21698.
Updates #18534.
Change-Id: I03a5ab195dae93154a46c32083c4bb52415d2017
Reviewed-on: https://go-review.googlesource.com/68573
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We haven't used non-zero gcForcePreemptNS for ages. Remove it and
declutter the code.
Change-Id: Id5cc62f526d21ca394d2b6ca17d34a72959535da
Reviewed-on: https://go-review.googlesource.com/68572
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
When GOMAXPROCS is not small, fractional workers don't add much to
throughput, but they do add to the latency of individual goroutines.
In this case, it makes sense to just use dedicated workers, even if we
can't exactly hit the 25% CPU goal with dedicated workers.
This implements this logic by computing the number of dedicated mark
workers that will us closest to the 25% target. We only fall back to
fractional workers if that would be more than 30% off of the target
(less than 17.5% or more than 32.5%, which in practice happens for
GOMAXPROCS <= 3 and GOMAXPROCS == 6).
Updates #21698.
Change-Id: I484063adeeaa1190200e4ef210193a20e635d552
Reviewed-on: https://go-review.googlesource.com/68571
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently these are the same constant, but are separate concepts.
Split them into two constants for easier experimentation and better
documentation.
Change-Id: I121854d4fd1a4a827f727c8e5153160c24aacda7
Reviewed-on: https://go-review.googlesource.com/68570
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This change adds support for accelerating time.Now by using
the __vdso_clock_gettime fast-path via the vDSO on linux/386
if it is available.
When the vDSO path to the clocks is available, it is typically
5x-10x faster than the syscall path (see benchmark extract
below). Two such calls are made for each time.Now() call
on most platforms as of go 1.9.
- Add vdso_linux_386.go, containing the ELF32 definitions
for use by vdso_linux.go, the maximum array size, and
the symbols to be located in the vDSO.
- Modify runtime.walltime and runtime.nanotime to check for
and use the vDSO fast-path if available, or fall back to
the existing syscall path.
- Reduce the stack reservations for runtime.walltime and
runtime.monotime from 32 to 16 bytes. It appears the syscall
path actually only needed 8 bytes, but 16 is now needed to
cover the syscall and vDSO paths.
- Remove clearing DX from the syscall paths as clock_gettime
only takes 2 args (BX, CX in syscall calling convention),
so there should be no need to clear DX.
The included BenchmarkTimeNow was run with -cpu=1 -count=20
on an "Intel(R) Celeron(R) CPU J1900 @ 1.99GHz", comparing
released go 1.9.1 vs this change. This shows a gain in
performance on linux/386 (6.89x), and that no regression
occurred on linux/amd64 due to this change.
Kernel: linux/i686, GOOS=linux GOARCH=386
name old time/op new time/op delta
TimeNow 978ns ± 0% 142ns ± 0% -85.48% (p=0.000 n=16+20)
Kernel: linux/x86_64, GOOS=linux GOARCH=amd64
name old time/op new time/op delta
TimeNow 125ns ± 0% 125ns ± 0% ~ (all equal)
Gains are more dramatic in virtualized environments,
presumably due to the overhead of virtualizing the syscall.
Fixes#22190
Change-Id: I2f83ce60cb1b8b310c9ced0706bb463c1b3aedf8
Reviewed-on: https://go-review.googlesource.com/69390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is a preparation step for adding vDSO support on linux/386.
This change relocates the elf64 and amd64 specifics from
vdso_linux.go to a new vdso_linux_amd64.go.
This should enable vdso_linux.go to be used for vDSO
support on linux architectures other than amd64.
- Relocate the elf64X structure definitions appropriate to amd64,
and change their names to elfX so that the code in vdso_linux.go
is ELFnn-agnostic.
- Relocate the sym_keys and corresponding __vdso_* variables
appropriate to amd64.
- Provide an amd64-specific constant for the maximum byte size of
an array, and use this in vdso_linux.go to compute constants for
sizing the elf structure arrays traversed in the loaded vDSO.
Change-Id: I1edb4e4ec9f2d79b7533aa95fbd09f771fa4edef
Reviewed-on: https://go-review.googlesource.com/69391
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.
To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.
Fixes#22175
Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The .debug_aranges section is an odd vestige of DWARF, since its
contents are easy and efficient for a debugger to reconstruct from the
attributes of the top-level compilation unit DIEs. Neither GCC nor
clang emit it by default these days. GDB and Delve ignore it entirely.
LLDB will use it if present, but is happy to construct the index from
the compilation unit attributes (and, indeed, a remarkable variety of
other ways if those aren't available either).
We're about to split up the compilation units by package, which means
they'll have discontiguous PC ranges, which is going to make
.debug_aranges harder to construct (and larger).
Rather than try to maintain this essentially unused code, let's
simplify things and remove it.
Change-Id: I8e0ccc033b583b5b8908cbb2c879b2f2d5f9a50b
Reviewed-on: https://go-review.googlesource.com/69972
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The alternative signal stack doesn't work on ios, so the setup of
the alternative stack was skipped. The corresponding unminitSignals
was effectively a no-op on ios until CL 70130. Skip unminitSignals
on ios to restore the previous behaviour.
For the ios builders.
Change-Id: I5692ca7f5997e6b9d10cc5f2383a5a37c42b133c
Reviewed-on: https://go-review.googlesource.com/70270
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
CL 69292 unified the amd64 entry-points, but Dragonfly doesn't follow
the same entry-point argument conventions as most other amd64
platforms. Fix the Dragonfly entry point.
Change-Id: I0f84e2e4101ce68217af185ee9baaf455b8b6dad
Reviewed-on: https://go-review.googlesource.com/70212
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Since CL 46037, the runtime is crashing after calling
exitThread on Plan 9.
The exitThread function shouldn't be called on
Plan 9, because the system manages thread stacks.
Fixes#22221.
Change-Id: I5d61c9660a87dc27e4cfcb3ca3ddcb4b752f2397
Reviewed-on: https://go-review.googlesource.com/70190
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Android's libc creates a signal stack for every thread it creates. In
Go, minitSignalStack picks up this existing signal stack and puts it
in m.gsignal.stack. However, if we later try to exit a thread (because
a locked goroutine is exiting), we'll attempt to stackfree this
libc-allocated signal stack and panic.
Fix this by clearing gsignal.stack when we unminitSignals in such a
situation.
This should fix the Android build, which is currently broken.
Change-Id: Ieea8d72ef063d22741c54c9daddd8bb84926a488
Reviewed-on: https://go-review.googlesource.com/70130
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds the _lib entry point to various GOOS_amd64.s files.
A future CL will enable c-archive/c-shared mode for those targets.
As far as I can tell, the newosproc0 function in os_darwin.go was
passing the wrong arguments to bsdthread_create. The newosproc0
function is never called in the current testsuite.
Change-Id: Ie7c1c2e326cec87013e0fea84f751091b0ea7f51
Reviewed-on: https://go-review.googlesource.com/69711
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
All of the amd64 entry point code is the same except for Plan 9.
Unify it all into asm_amd64.s.
Change-Id: Id47ce3a7bb2bb0fd48f326a2d88ed18b17dee456
Reviewed-on: https://go-review.googlesource.com/69292
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Right now users have to infer why they would want LockOSThread and
when it may or may not be appropriate to call UnlockOSThread. This
requires some understanding of Go's internal thread pool
implementation, which is unfortunate.
Improve the situation by making the documentation on these functions
more prescriptive so users can figure out when to use them even if
they don't know about the scheduler.
Change-Id: Ide221791e37cb5106dd8a172f89fbc5b3b98fe32
Reviewed-on: https://go-review.googlesource.com/52871
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
runtime.LockOSThread is sometimes used when the caller intends to put
the OS thread into an unusual state. In this case, we never want to
return this thread to the runtime thread pool. However, currently
exiting the goroutine implicitly unlocks its OS thread.
Fix this by terminating the locked OS thread when its goroutine exits,
rather than simply returning it to the pool.
Fixes#20395.
Change-Id: I3dcec63b200957709965f7240dc216fa84b62ad9
Reviewed-on: https://go-review.googlesource.com/46038
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently, threads created by the runtime exist until the whole
program exits. For #14592 and #20395, we want to be able to exit and
clean up threads created by the runtime. This commit implements that
mechanism.
The main difficulty is how to clean up the g0 stack. In cgo mode and
on Solaris and Windows where the OS manages thread stacks, we simply
arrange to return from mstart and let the system clean up the thread.
If the runtime allocated the g0 stack, then we use a new exitThread
syscall wrapper that arranges to clear a flag in the M once the stack
can safely be reaped and call the thread termination syscall.
exitThread is based on the existing exit1 wrapper, which was always
meant to terminate the calling thread. However, exit1 has never been
used since it was introduced 9 years ago, so it was broken on several
platforms. exitThread also has the additional complication of having
to flag that the stack is unused, which requires some tricks on
platforms that use the stack for syscalls.
This still leaves the problem of how to reap the unused g0 stacks. For
this, we move the M from allm to a new freem list as part of the M
exiting. Later, allocm scans the freem list, finds Ms that are marked
as done with their stack, removes these from the list and frees their
g0 stacks. This also allows these Ms to be garbage collected.
This CL does not yet use any of this functionality. Follow-up CLs
will. Likewise, there are no new tests in this CL because we'll need
follow-up functionality to test it.
Change-Id: Ic851ee74227b6d39c6fc1219fc71b45d3004bc63
Reviewed-on: https://go-review.googlesource.com/46037
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently, since Ms never exit, the number of Ms, the number of Ms
ever created, and the ID of the next M are all the same and must be
small. That's about to change, so rename sched.mcount to sched.mnext
to make it clear it's the number of Ms ever created (and the ID of the
next M), change its type to int64, and use mcount() for the number of
Ms. In the next commit, mcount() will become slightly less trivial.
For #20395.
Change-Id: I9af34d36bd72416b5656555d16e8085076f1b196
Reviewed-on: https://go-review.googlesource.com/68750
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
mstart is the entry point for new threads, so it certainly can't
interact with GC enough to have write barriers. We move the one small
piece that is allowed to have write barriers out into its own
function.
Change-Id: Id9c31d6ffac31d0051fab7db15eb428c11cadbad
Reviewed-on: https://go-review.googlesource.com/46035
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This field is really a *m (modulo its bottom bit). Change it from
uintptr to muintptr to document this fact.
Change-Id: I2d181a955ef1d2c1a268edf20091b440d85726c9
Reviewed-on: https://go-review.googlesource.com/46034
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Applications that need to manipulate kernel thread state are currently
on thin ice in Go: they can use LockOSThread to prevent other
goroutines from running on the manipulated thread, but Go may clone
this manipulated state into a new thread that's put into the runtime's
thread pool along with other threads.
Fix this by never starting a new thread from a locked thread or a
thread that may have been started by C. Instead, the runtime starts a
"template thread" with a known-good state. If it then needs to start a
new thread but doesn't know that the current thread is in a good
state, it forwards the thread creation to the template thread.
Fixes#20676.
Change-Id: I798137a56e04b7723d55997e9c5c085d1d910643
Reviewed-on: https://go-review.googlesource.com/46033
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This is a preparation step for adding vDSO support on linux/386.
In a follow-on change, the vDSO ELF symbol lookup code in this
file will be refactored so it can be used on multiple architectures.
First, move the file to an architecture-neutral file name so that
the change history is preserved. Build tags are added so that the
build behaves as it did before.
vdso_linux_amd64.go will be recreated later, just containing the
amd64 specifics.
If the move and refactor were combined in a single change, then the
history to date would be lost because git would see the existing code
as a new file.
Change-Id: Iddb5da0d7faf141fd7cc835fe6a80c80153897e9
Reviewed-on: https://go-review.googlesource.com/69710
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add a compiler intrinsic for getcallersp. So we are able to get
rid of the argument (not done in this CL).
Change-Id: Ic38fda1c694f918328659ab44654198fb116668d
Reviewed-on: https://go-review.googlesource.com/69350
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
CL 68490 reworked the way the panicmem function is exposed to the
C mach expection catcher. However, //go:cgo_export_static isn't enough:
the underlying assembly functions must not start with the middle dot.
Without the middle dot, the panicmem function's exported name is
not prefixed with its package; rename it to xx_cgo_panicmem to decrease
the chance of a symbol name clash.
Finally, mark the overridden C symbol weak to avoid duplicate symbol
errors from the host linker.
For the ios builders.
Change-Id: Ib87789fecec9314e398cf1bd8c04ba0b3a6642af
Reviewed-on: https://go-review.googlesource.com/69113
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The core dump reader would like a bunch of ideal int
constants to be available in dwarf.
Makes the go binary 0.9% bigger.
Update #14517
Change-Id: I00cdfc7f53bcdc56fccba576c1d33010f03bdd95
Reviewed-on: https://go-review.googlesource.com/69270
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
These have never had a use - not even going back to when they were added
in C.
Change-Id: I143b6902b3bacb1fa83c56c9070a8adb9f61a844
Reviewed-on: https://go-review.googlesource.com/69119
Reviewed-by: Dave Cheney <dave@cheney.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The //go:nosplit directive was visible in GoDoc because the function
that it preceeded (Gosched) is exported. This change moves the directive
above the documentation, hiding it from the output.
Change-Id: I281fd7573f11d977487809f74c9cc16b2af0dc88
Reviewed-on: https://go-review.googlesource.com/69120
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
exit1 calls the bsdthread_terminate system call on Darwin. Currently
it passes no arguments on 386, arm, and arm64, and an exit status on
amd64. None of these are right. The signature of bsdthread_terminate
is:
int bsdthread_terminate(user_addr_t stackaddr, size_t freesize, uint32_t port, uint32_t sem);
Fix all of the Darwin exit1 implementations to call
bsdthread_terminate with 0 for all of these arguments so it doesn't
try to unmap some random memory, free some random port, or signal a
random semaphore.
This isn't a problem in practice because exit1 is never called.
However, we're about to start using exit1.
Change-Id: Idc534d196e3104e5253fc399553f21eb608693d7
Reviewed-on: https://go-review.googlesource.com/46036
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The core dump reader wants to know the layout of this type.
No variable has this type, so it wasn't previously dumped
to DWARF output.
Change-Id: I982040b81bff202976743edc7fe53247533a9d81
Reviewed-on: https://go-review.googlesource.com/68312
Reviewed-by: Austin Clements <austin@google.com>
Currently, there is a single bit for LockOSThread, so two calls to
LockOSThread followed by one call to UnlockOSThread will unlock the
thread. There's evidence (#20458) that this is almost never what
people want or expect and it makes these APIs very hard to use
correctly or reliably.
Change this so LockOSThread/UnlockOSThread can be nested and the
calling goroutine will not be unwired until UnlockOSThread has been
called as many times as LockOSThread has. This should fix the vast
majority of incorrect uses while having no effect on the vast majority
of correct uses.
Fixes#20458.
Change-Id: I1464e5e9a0ea4208fbb83638ee9847f929a2bacb
Reviewed-on: https://go-review.googlesource.com/45752
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Init functions are problematic because we want cmd/link to be
able to insert an import of runtime/cgo for external linking.
For all the other systems that just means putting runtime/cgo into
the binary. The linker is not set up to generate calls to init functions,
and luckily this one can be avoided entirely.
This means people don't have to import _ "runtime/cgo" in their
iOS programs anymore. The linker's default import is now enough.
This CL also adjusts cmd/go to record the linker's default import,
now that the explicit import is gone.
Change-Id: I81d23476663e03664f90d531c24db2e4f2e6c66b
Reviewed-on: https://go-review.googlesource.com/68490
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(*sigctxt).fault() currently returns either uintptr, uint32, or uint64
depending on the platform. Make them all return uintptr.
For #10958 (but a nice change on its own).
Change-Id: I7813e779d0edcba112dd47fda776f4ce6e50e227
Reviewed-on: https://go-review.googlesource.com/68015
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Before CL 62530 fastrand always returned non-zero value, and one
condition in sema.go depends on this behavior.
fastrand is used to generate random weight for treap of sudog, and
it is checked against zero to verify sudog were inserted into treap or
wait queue.
Since its precision is not very important for correctness, lets just
always set its lowest bit in this place.
Updates #22047
Updates #21806
Change-Id: Iba0b56d81054e6ef9c49ffd293fc5d92a6a31e9b
Reviewed-on: https://go-review.googlesource.com/68050
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was only waiting 0.1 seconds for the two GCs it wanted.
Let it wait 1 second.
Change-Id: Ib3cdc8127cbf95694a9f173643c02529a85063af
Reviewed-on: https://go-review.googlesource.com/68151
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
No need to type this global as an unsafe.Pointer, we know
what type the referent is.
Change-Id: I7b1374065b53ccf1373754a21d54adbedf1fd587
Reviewed-on: https://go-review.googlesource.com/67990
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Latest macOS High Sierra changed how the wall time information
is exported into the commpage. Backward compatibility was partly
preserved, that is previous Go versions are basically forced to
go through a syscall which is much slower and is not able to
get nanosecond precision.
Implement the new commpage layout and wall time computation,
using a version check to fallback to the previous code on
older operating systems.
Fixes#22037
Change-Id: I8c2176eaca83a5d7be23443946a6b4c653ec7f68
Reviewed-on: https://go-review.googlesource.com/67332
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
High Sierra has a new commpage layout (this is issue #3188), so
we need to adjust the code to handle multiple versions of the
layout.
In preparation for this change, we rename the existing offset
macros with a prefix that identifies the commpage version they
refer to.
Updates #22037
Change-Id: Idca4b7a855a2ff6dbc434cd12453fc3194707aa8
Reviewed-on: https://go-review.googlesource.com/67331
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A new testcase TestSelectFairness was recently added, and
since then the ppc64le build tests have intermittently failed.
This adds a change to skip this test on ppc64le using
SkipFlaky to help determine if the problem is with the
test or something else with that commit.
Updates #22047
Change-Id: Idfef72ed791c5bd45c42ff180947fea3df280ea7
Reviewed-on: https://go-review.googlesource.com/67631
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The function dumpbvtypes has no use case anymore, so we remove it with
this change.
Change-Id: I1e0323542be2bcc683b75dffde76b222e087c285
Reviewed-on: https://go-review.googlesource.com/66370
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently the FreeBSD CPU affinity code assumes that the maximum
GOMAXPROCS is 256, but we just removed that limit.
This commit rewrites the FreeBSD CPU affinity code to raise the CPU
count limit to 65,536, like the Linux CPU affinity code, and to
degrade more gracefully if we do somehow go over that.
Change-Id: Ic4ca7f88bd8b9448aae4dbd43ef21a6c1b8fea63
Reviewed-on: https://go-review.googlesource.com/66291
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
allp now has length gomaxprocs, which means none of allp[i] are nil or
in state _Pdead. This lets replace several different styles of loops
over allp with normal range loops.
for i := 0; i < gomaxprocs; i++ { ... } loops can simply range over
allp. Likewise, range loops over allp[:gomaxprocs] can just range over
allp.
Loops that check for p == nil || p.state == _Pdead don't need to check
this any more.
Loops that check for p == nil don't have to check this *if* dead Ps
don't affect them. I checked that all such loops are, in fact,
unaffected by dead Ps. One loop was potentially affected, which this
fixes by zeroing p.gcAssistTime in procresize.
Updates #15131.
Change-Id: Ifa1c2a86ed59892eca0610360a75bb613bc6dcee
Reviewed-on: https://go-review.googlesource.com/45575
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now that allp is dynamically allocated, there's no need for a hard cap
on GOMAXPROCS.
Fixes#15131.
Change-Id: I53eee8e228a711a818f7ebce8d9fd915b3865eed
Reviewed-on: https://go-review.googlesource.com/45574
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes it possible to eliminate the hard cap on GOMAXPROCS.
Updates #15131.
Change-Id: I4c422b340791621584c118a6be1b38e8a44f8b70
Reviewed-on: https://go-review.googlesource.com/45573
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
syscall.Exit and runtime.exit do the same thing.
Why duplicate code?
CL 45115 fixed bug where windows runtime.exit was correct,
but syscall.Exit was broken. So CL 45115 fixed windows
syscall.Exit by calling runtime.exit.
Austin suggested that all OSes should do the same, and
this CL implements his idea.
While making changes, I discovered that nacl syscall.Exit
returned error
func Exit(code int) (err error)
and I changed it into
func Exit(code int)
like all other OSes. I assumed it was a mistake and it
is OK to do because cmd/api does not complain about it.
Also I changed plan9 runtime.exit to accept int32 just
like all other OSes do.
Change-Id: I12f6022ad81406566cf9befcc6edc382eebd413b
Reviewed-on: https://go-review.googlesource.com/66170
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Currently, the priority of checks in (gcTrigger).test() puts the
gcpercent<0 test above gcTriggerCycle, which is used for runtime.GC().
This is an unintentional change from 1.8 and before, where
runtime.GC() triggered a GC even if GOGC=off.
Fix this by rearranging the priority so the gcTriggerCycle test
executes even if gcpercent < 0.
Fixes#22023.
Change-Id: I109328d7b643b6824eb9d79061a9e775f0149575
Reviewed-on: https://go-review.googlesource.com/65994
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The previous comment of nextSample didn't mention Poisson processes,
which is the reason why it needed to create an exponential
distribution, so it was hard to follow the reasoning for people
not highly familiar with statistics.
Since we're at it, we also make it clear that we are just creating
a random number with exponential distribution by moving the
bulk of the function into a new fastexprand().
No functional changes.
Change-Id: I9c275e87edb3418ee0974257af64c73465028ad7
Reviewed-on: https://go-review.googlesource.com/65657
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
strings.IndexByte was introduced in go1.2 and it can be used
effectively wherever the second argument to strings.Index is
exactly one byte long.
This avoids generating unnecessary string symbols and saves
a few calls to strings.Index.
Change-Id: I1ab5edb7c4ee9058084cfa57cbcc267c2597e793
Reviewed-on: https://go-review.googlesource.com/65930
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some (all?) versions of gdb on windows output "\r\n" as line ending
instead of "\n".
Fixes#22012
Change-Id: I798204fd9f616d6d2c9c28eb5227fadfc63c0d45
Reviewed-on: https://go-review.googlesource.com/65850
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The compiler generates wrapper methods to forward interface method
calls (which are always pointer-based) to value methods. These
wrappers appear in the call stack even though they are an
implementation detail. This leaves ugly "<autogenerated>" functions in
stack traces and can throw off skip counts for stack traces.
Fix this by considering these runtime frames in printed stack traces
so they will only be printed if runtime frames are being printed, and
by eliding them from the call stack expansion used by CallersFrames
and Caller.
This removes the test for issue 4388 since that was checking that
"<autogenerated>" appeared in the stack trace instead of something
even weirder. We replace it with various runtime package tests.
Fixes#16723.
Change-Id: Ice3f118c66f254bb71478a664d62ab3fc7125819
Reviewed-on: https://go-review.googlesource.com/45412
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
panicwrap currently uses runtime.Callers and runtime.CallersFrames to
find the name of its caller. Simplify this by using getcallerpc.
This will be important for #16723, since to fix that we're going to
make CallersFrames skip the wrapper method, which is exactly what
panicwrap needs to see.
Change-Id: Icb0776d399966e31595f3ee44f980290827e32a6
Reviewed-on: https://go-review.googlesource.com/45411
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that getcallerpc is a compiler intrinsic on x86 and non-x86
platforms don't need the argument, we can drop it.
Sadly, this doesn't let us remove any dummy arguments since all of
those cases also use getcallersp, which still takes the argument
pointer, but this is at least an improvement.
Change-Id: I9c34a41cf2c18cba57f59938390bf9491efb22d2
Reviewed-on: https://go-review.googlesource.com/65474
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This no longer appears to be reproducible on windows/386. Try putting
it back and we'll see if the builders still don't like it.
Fixes#19319.
Change-Id: Ia47b034e18d0a5a1951125c00542b021aacd5e8d
Reviewed-on: https://go-review.googlesource.com/47936
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
First step towards removing the mandatory argument for
getcallerpc, which solves certain problems for the runtime.
This might also slightly improve performance.
Intrinsic enabled on 386, amd64, amd64p32,
runtime asm implementation removed on those architectures.
Now-superfluous argument remains in getcallerpc signature
(for a future CL; non-386/amd64 asm funcs ignore it).
Added getcallerpc to the "not a real function" test
in dcl.go, that story is a little odd with respect to
unexported functions but that is not this CL.
Fixes#17327.
Change-Id: I5df1ad91f27ee9ac1f0dd88fa48f1329d6306c3e
Reviewed-on: https://go-review.googlesource.com/31851
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
For a trivial benchmark with a do-nothing cgo call:
name old time/op new time/op delta
Call-4 64.5ns ± 7% 63.0ns ± 6% -2.25% (p=0.027 n=20+16)
Because Windows uses the cgocall mechanism to make system calls,
and passes arguments in a struct held in the m,
we need to do the lockOSThread/unlockOSThread in that code.
Because deferreturn was getting a nosplit stack overflow error,
change it to avoid calling typedmemmove.
Updates #21827.
Change-Id: I9b1d61434c44faeb29805b46b409c812c9acadc2
Reviewed-on: https://go-review.googlesource.com/64070
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Do the low-hanging fruit - tiny Less functions that are used exactly
once. This reduces the amount of code and puts the logic in a single
place.
Change-Id: I9d4544cd68de5a95e55019bdad1fca0a1dbfae9c
Reviewed-on: https://go-review.googlesource.com/63171
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
https://golang.org/cl/22598 made nextFreeFast inlinable.
But during https://golang.org/cl/63611 it was discovered, that it is no longer inlinable.
Reduce number of statements below inlining threshold to make it inlinable again.
Also update tests, to prevent regressions.
Doesn't reduce readability.
Change-Id: Ia672784dd48ed3b1ab46e390132f1094fe453de5
Reviewed-on: https://go-review.googlesource.com/65030
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
In general, there are no guarantee that `go` command exist on $PATH.
This CL tries to get `go` command from $GOROOT/bin instead.
There are three kinds of code we should handle:
For normal code, the CL implements goCmd() or goCmdName().
For unit tests, the CL uses testenv.GoTool() or testenv.GoToolPath().
For integration tests, the CL sets PATH=$GOROOT/bin:$PATH in cmd/dist.
Note that make.bash sets PATH=$GOROOT/bin:$PATH in the build process.
So this change is only useful when we use toolchain manually.
Updates #21875
Change-Id: I963b9f22ea732dd735363ececde4cf94a5db5ca2
Reviewed-on: https://go-review.googlesource.com/64650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current generator is a simple LSFR, which showed strong
correlation in higher bits, as manifested by fastrandn().
Change it with xorshift64+, which is slightly more complex,
has a larger state, but has a period of 2^64-1 and is much better
at statistical tests. The version used here is capable of
passing Diehard and even SmallCrush.
Speed is slightly worse but is probably insignificant:
name old time/op new time/op delta
Fastrand-4 0.77ns ±12% 0.91ns ±21% +17.31% (p=0.048 n=5+5)
FastrandHashiter-4 13.6ns ±21% 15.2ns ±17% ~ (p=0.160 n=6+5)
Fastrandn/2-4 2.30ns ± 5% 2.45ns ±15% ~ (p=0.222 n=5+5)
Fastrandn/3-4 2.36ns ± 7% 2.45ns ± 6% ~ (p=0.222 n=5+5)
Fastrandn/4-4 2.33ns ± 8% 2.61ns ±30% ~ (p=0.126 n=6+5)
Fastrandn/5-4 2.33ns ± 5% 2.48ns ± 9% ~ (p=0.052 n=6+5)
Fixes#21806
Change-Id: I013bb37b463fdfc229a7f324df8fe2da8d286f33
Reviewed-on: https://go-review.googlesource.com/62530
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Return early from deltimer, with false as the result,
to indicate that we couldn't delete the timer since its
timersBucket was nil(not set) in the first place.
That happens in such a case where a user created
the timer from a Ticker with:
t := time.Ticker{C: c}
The above usage skips the entire setup of assigning
the appropriate underlying runtimeTimer and timersBucket,
steps that are done for us by time.NewTicker.
CL 34784 introduced this bug with an optimization, by changing
stopTimer to retrieve the timersBucket from the timer itself
(which is unset with the mentioned usage pattern above),
whereas the old behavior relied on indexing
by goroutine ID into the global slice of runtime
timers, to retrieve the appropriate timersBucket.
Fixes#21874
Change-Id: Ie9ccc6bdee685414b2430dc4aa74ef618cea2b33
Reviewed-on: https://go-review.googlesource.com/63970
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change has no real effect in itself. This is to prepare for a
followup change that will call lockOSThread during a cgo callback when
there is no p assigned, and therefore when lockOSThread can not use a
write barrier.
Change-Id: Ia122d41acf54191864bcb68f393f2ed3b2f87abc
Reviewed-on: https://go-review.googlesource.com/63630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Move it from the runtime package, as we will soon add more packages and
functions for it to check.
The test used the testEnv func, which cleaned certain environment
variables from a command, so it was moved to internal/testenv under a
more descriptive (and less ambiguous) name. Add a simple godoc to it
too.
For #21851.
Change-Id: I6f39c1f23b45377718355fafe66ffd87047d8ab6
Reviewed-on: https://go-review.googlesource.com/63550
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
This makes it easier to deduce from the field names which overflow
field corresponds to h.buckets and which to h.oldbuckets by aligning
the naming with the buckets fields in hmap.
Change-Id: I8d6a729229a190db0212bac012ead1a3c13cf5d0
Reviewed-on: https://go-review.googlesource.com/62411
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Move the check near the code in evacuate that relies on
the relation evacuateX+1 == evacuateY.
If the relation is fullfilled the check is known to be true
at compile time and removed by the compiler.
Change-Id: I711b75e09047bf347819ccaeec41d244a5883867
Reviewed-on: https://go-review.googlesource.com/62410
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
After the number of lost extra events are written to the the cpuprof log,
the number of lost extra events should be set to zero, or else, the next
time time addExtra is logged, lostExtra will be overcounted. This change
resets lostExtra after its value is written to the log.
Fixes#21836
Change-Id: I8a6ac9c61e579e7a5ca7bdb0f3463f8ae8b9f863
Reviewed-on: https://go-review.googlesource.com/63270
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While some map literals were marked non-escaping that information
was lost when creating the corresponding OMAKE node which made map
literals always heap allocated.
Copying the escape information to the corresponding OMAKE node allows
stack allocation of hmap and a map bucket for non escaping map literals.
Fixes#21830
Change-Id: Ife0b020fffbc513f1ac009352f2ecb110d6889c9
Reviewed-on: https://go-review.googlesource.com/62790
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
1. remove broken verification
The runtime check assumes that no-pcln symbol entry have zero value,
but the linker emit no entries if the symbol is no-pcln.
As a result, if there are no-pcln symbols at the very end of pcln
table, it will panic.
2. correct condition of export
Handle special chracters in pluginpath correcty.
Export "go.itab.*", so different plugins can share the same itab.
Fixes#18190
Change-Id: Ia4f9c51d83ce8488a9470520f1ee9432802cfc1d
Reviewed-on: https://go-review.googlesource.com/61091
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Along the way, track bad modules. Make sure they don't end up on
the active modules list, and aren't accidentally reprocessed as
new plugins.
Fixes#19004
Change-Id: I8a5e7bb11f572f7b657a97d521a7f84822a35c07
Reviewed-on: https://go-review.googlesource.com/61171
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
runtime.makemap will allocate map buckets on the heap for hints larger
than the number of elements a single map bucket can hold.
Do not allocate any map bucket on the stack if it is known at compile time
that hint is larger than the number of elements one map bucket can hold.
This avoids zeroing and reserving memory on the stack that will not be used.
Change-Id: I1a5ab853fb16f6a18d67674a77701bf0cf29b550
Reviewed-on: https://go-review.googlesource.com/60450
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler and reflect already zero hiter before mapiterinit.
While here expand the documentation for mapiterinit.
Change-Id: I78b05d4d14bf78e8091e5353cdac80ffed30ca1e
Reviewed-on: https://go-review.googlesource.com/60673
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Remove the runtime ismapkey check from makemap and
add a check that the map key type supports comparison
to the hmap construction in the compiler.
Move the ismapkey check for the reflect code path
into reflect_makemap.
Change-Id: I718f79b0670c05b63ef31721e72408f59ec4ae86
Reviewed-on: https://go-review.googlesource.com/61035
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Move memclr to a separate file to make it consistent
with other platforms asm function to file organization.
Remove nacl from the memmove filename as the implementation
is generic for the amd64p32 platform even if currently only
nacl is supported for amd64p32.
Change-Id: I8930b76da430a5cf2664801974e4f5185fc0f82f
Reviewed-on: https://go-review.googlesource.com/61031
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
overLoadFactor wasn't really doing what it says it does.
It was reporting overOrEqualToLoadFactor. That's actually what we
want when adding an entry to a map, but it isn't what we want when
constructing a map in the first place.
The impetus for this change is that if you make a map with a hint
of exactly 8 (which happens, for example, with the unitMap in
time/format.go), we allocate 2 buckets for it instead of 1.
Instead, make overLoadFactor really report when it is > the max
allowed load factor, not >=. Adjust the callers who want to ensure
that the map is no more than the max load factor after an insertion
by adding a +1 to the current (pre-addition) size.
Change-Id: Ie8d85344800a9a870036b637b1031ddd9e4b93f9
Reviewed-on: https://go-review.googlesource.com/61053
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Matches other architectures by using names for syscalls instead of
numbers directly.
Fixes#20499.
Change-Id: I63d606b0b1fe6fb517fd994a7542a3f38d80dd54
Reviewed-on: https://go-review.googlesource.com/44213
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Between go1.7 and go1.8, a performance regression was introduced in some of the
BenchmarkCompareBytes benchmarks.
Go1.7 vs Go1.8:
BenchmarkCompareBytesToNil-8 7.44 8.44 +13.44%
BenchmarkCompareBytesIdentical-8 6.96 11.5 +65.23%
BenchmarkCompareBytesBigIdentical-8 6.65 47112 +708351.13%
This change fixes the problem by optimizing the case where the byte slices being
compared are equal:
Go1.9 vs current:
BenchmarkCompareBytesToNil-8 7.35 7.00 -4.76%
BenchmarkCompareBytesIdentical-8 11.4 6.81 -40.26%
BenchmarkCompareBytesBigIdentical-8 48396 9.26 -99.98%
runtime.cmpstring can benefit from the same approach and is also changed.
Change-Id: I3cb25f59d8b940a83a2cf687eea764cfeff90688
Reviewed-on: https://go-review.googlesource.com/59650
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
This change implements the convention for generated code header agreed upon in https://golang.org/s/generatedcode.
Additionally run go generate.
Also update some comments.
Updates #13560
Change-Id: If45f91b93aaa0d43280c2c4630823bc4d2dc7d3a
Reviewed-on: https://go-review.googlesource.com/60250
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Check map invariants, type size and alignments during compile time.
Keep runtime checks for reflect by adding them to reflect_makemap.
Change-Id: Ia28610626591bf7fafb7d5a1ca318da272e54879
Reviewed-on: https://go-review.googlesource.com/59914
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Right now we only kind of sort of trace GC STW events. We emit events
around mark termination, but those start well after stopping the world
and end before starting it again, and we don't emit any events for
sweep termination.
Fix this by generalizing EvGCScanStart/EvGCScanDone. These were
already re-purposed to indicate mark termination (despite the names).
This commit renames them to EvGCSTWStart/EvGCSTWDone, adds an argument
to indicate the STW reason, and shuffles the runtime to generate them
right before stopping the world and right after starting the world,
respectively.
These events will make it possible to generate precise minimum mutator
utilization (MMU) graphs and could be useful in detecting
non-preemptible goroutines (e.g., #20792).
Change-Id: If95783f370781d8ef66addd94886028103a7c26f
Reviewed-on: https://go-review.googlesource.com/55411
Reviewed-by: Rick Hudson <rlh@golang.org>
Found with mvdan.cc/unindent. It skipped the cases where parentheses
would need to be added, where comments would have to be moved elsewhere,
or where actions and simple logic would mix.
One of them was of the form "err != nil && err == io.EOF", so the first
part was removed.
Change-Id: Ie504c2b03a2c87d10ecbca1b9270069be1171b91
Reviewed-on: https://go-review.googlesource.com/57690
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 57291 broke on solaris because it depends on signal forwarding
working for signals raised by dieFromSignal.
Call sigtrampgo instead of sighandler directly, like the other
unix platforms.
Fixes the solaris builders.
Change-Id: I6bf314c436d1edeaecc4b03f15a9155270919524
Reviewed-on: https://go-review.googlesource.com/59811
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit adds an example to the runtime/trace package
on how to use the trace.Start and trace.Stop functions
to trace the execution of a Go program and write
its trace output to a file.
Change-Id: Idf920398f1c3b9d185af9df5ce9293f2361db022
Reviewed-on: https://go-review.googlesource.com/51170
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This change could improve the hit rate on itabTable during growth.
While we are here patch comments to refer to existing functions.
Change-Id: I76f81c860a3d6107e077e7e3932550858a8b7651
Reviewed-on: https://go-review.googlesource.com/55912
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 49590 made it possible for external signal handlers to catch
signals from a crashing Go process. This CL extends that support
to handlers registered after the Go runtime has initialized.
Updates #20392 (and possibly fix it).
Change-Id: I18eccd5e958a505f4d1782a7fc51c16bd3a4ff9c
Reviewed-on: https://go-review.googlesource.com/57291
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
All of the mapfast key types are reflexive.
Change-Id: I8595aed2a9d945cda1b5d08e2067dce0f1c0d585
Reviewed-on: https://go-review.googlesource.com/59132
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
The newly added routines are exact copies of the generic routines,
except for the function names and that growWork_fastX calls evacuate_fastX.
Actual optimization will happen in subsequent CLs.
This is intended to ease reviewing.
Change-Id: I52ef7dd40b2bdfc9cba2496544c0604e6e71cf7f
Reviewed-on: https://go-review.googlesource.com/59130
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Went mainly for the ones that make no sense, such as the ones
mid-sentence or after commas.
Change-Id: Ie245d2c19cc7428a06295635cf6a9482ade25ff0
Reviewed-on: https://go-review.googlesource.com/57293
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Prior to this change, we use typedmemmove to write the key
value to its new location in mapassign_fast32 and mapassign_fast64.
(The use of typedmemmove was a last-minute fix in the 1.9 cycle;
see #21297 and CL 53414.)
This is significantly less inefficient than direct assignment or
calling writebarrierptr directly.
Fortunately, there aren't many cases to consider.
On systems with 32 bit pointers:
* A 32 bit AMEM value either is a single pointer or has no pointers.
* A 64 bit AMEM value may contain a pointer at the beginning,
a pointer at 32 bits, or two pointers.
On systems with 64 bit pointers:
* A 32 bit AMEM value contains no pointers.
* A 64 bit AMEM value either is a single pointer or has no pointers.
All combinations except the 32 bit pointers / 64 bit AMEM value are
cheap and easy to handle, and the problematic case is likely rare.
The most popular map keys appear to be ints and pointers.
So we handle them exhaustively. The sys.PtrSize checks are constant branches
and are eliminated by the compiler.
An alternative fix would be to return a pointer to the key,
and have the calling code do the assignment, at which point the compiler
would have full type information.
Initial tests suggest that the performance difference between these
strategies is negligible, and this fix is considerably simpler,
and has much less impact on binary size.
Fixes#21321
Change-Id: Ib03200e89e2324dd3c76d041131447df66f22bfe
Reviewed-on: https://go-review.googlesource.com/59110
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 36428 changed the way nanotime works so on Darwin and Windows it
now depends on runtime.startNano, which is computed at runtime.init
time. Unfortunately, the `runtimeInitTime = nanotime()` initialization
happened *before* runtime.init, so on these platforms runtimeInitTime
is set incorrectly. The one (and only) consequence of this is that the
start time printed in gctrace lines is bogus:
gc 1 18446653480.186s 0%: 0.092+0.47+0.038 ms clock, 0.37+0.15/0.81/1.8+0.15 ms cpu, 4->4->1 MB, 5 MB goal, 8 P
To fix this, this commit moves the runtimeInitTime initialization to
shortly after runtime.init, at which point nanotime is safe to use.
This also requires changing the condition in newproc1 that currently
uses runtimeInitTime != 0 simply to detect whether or not the main M
has started. Since runtimeInitTime could genuinely be 0 now, this
introduces a separate flag to newproc1.
Fixes#21554.
Change-Id: Id874a4b912d3fa3d22f58d01b31ffb3548266d3b
Reviewed-on: https://go-review.googlesource.com/58690
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This eliminates a nil check of b while evaluating b.tophash,
which is in the inner loop of many hot map functions.
It also makes the code a bit clearer.
Also remove some gotos in favor of labeled breaks.
On non-x86 architectures, this change introduces a pointless reg-reg move,
although the cause is well-understood (#21572).
Change-Id: Ib7ee58b59ea5463b92e1590c8b8f5c0ef87d410a
Reviewed-on: https://go-review.googlesource.com/58372
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This is a crude compiler pass to eliminate stores to auto variables
that are only ever written to.
Eliminates an unnecessary store to x from the following code:
func f() int {
var x := 1
return *(&x)
}
Fixes#19765.
Change-Id: If2c63a8ae67b8c590b6e0cc98a9610939a3eeffa
Reviewed-on: https://go-review.googlesource.com/38746
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On 386 the below code triggered an infinite loop in growslice:
x = make([]byte, 1<<30-1, 1<<30-1)
x = append(x, x...)
Check for overflow when calculating the new slice capacity
and set the new capacity to the requested capacity when an overflow
is detected to avoid an infinite loop.
No automatic test added due to requiring to allocate 1GB of memory
on a 32bit plaform before use of append is able to trigger the
overflow check.
Fixes#21441
Change-Id: Ia871cc9f88479dacf2c7044531b233f83d2fcedf
Reviewed-on: https://go-review.googlesource.com/57950
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
This slightly improves the generated code on x86 architectures,
including on many hot paths.
It is a no-op on other architectures.
Change-Id: I86336fd846bc5805a27bbec572e8c73dcbd0d567
Reviewed-on: https://go-review.googlesource.com/57411
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This is necessary when you aren't actively changing the runtime. Oops.
Also, run the tests on the builders, to avoid silent failures (#17472).
Change-Id: I1fc03790cdbddddb07026a772137a79919dcaac7
Reviewed-on: https://go-review.googlesource.com/58050
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When deleting entries from a map, only clear the key and value
if they contain pointers. And use memclrHasPointers to do so.
While we're here, specialize key clearing in mapdelete_faststr,
and fix another missed usage of add in mapdelete.
Benchmarking impeded by #21546.
Change-Id: I3f6f924f738d6b899b722d6438e9e63f52359b84
Reviewed-on: https://go-review.googlesource.com/57630
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Move the tophash checks after the equality/length checks.
For fast32/fast64, since we've done a full equality check already,
just check whether tophash is empty instead of checking tophash.
This is cheaper and allows us to skip calculating tophash.
These changes are modeled on the changes in CL 57590,
which were polished based on benchmarking.
Benchmarking directly is impeded by #21546.
Change-Id: I0e17163028e34720310d1bf8f95c5ef42d223e00
Reviewed-on: https://go-review.googlesource.com/57611
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This better matches the style of the rest of the runtime.
Change-Id: I6abb755df50eb3d9086678629c0d184177e1981f
Reviewed-on: https://go-review.googlesource.com/57610
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
During rebase of golang.org/cl/55152 the bucket argument
which was removed in golang.org/cl/56290 from makemap
was not removed from the argument list of makemap64.
This did lead to "pointer in unallocated span" errors
on 32bit platforms since the compiler did only generate
calls to makemap64 without the bucket argument.
Fixes#21568
Change-Id: Ia964a3c285837cd901297f4e16e40402148f8c1c
Reviewed-on: https://go-review.googlesource.com/57990
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The intent is to allow more aggressive refactoring
in the runtime without silent performance changes.
The test would be useful for many functions.
I've seeded it with the runtime functions tophash and add;
it will grow organically (or wither!) from here.
Updates #21536 and #17566
Change-Id: Ib26d9cfd395e7a8844150224da0856add7bedc42
Reviewed-on: https://go-review.googlesource.com/57410
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Where possible generate calls to runtime makemap with int hint argument
during compile time instead of makemap with int64 hint argument.
This eliminates converting the hint argument for calls to makemap with
int64 hint argument for platforms where int64 values do not fit into
an argument of type int.
A similar optimization for makeslice was introduced in CL
golang.org/cl/27851.
386:
name old time/op new time/op delta
NewEmptyMap 53.5ns ± 5% 41.9ns ± 5% -21.56% (p=0.000 n=10+10)
NewSmallMap 182ns ± 1% 165ns ± 1% -8.92% (p=0.000 n=10+10)
Change-Id: Ibd2b4c57b36f171b173bf7a0602b3a59771e6e44
Reviewed-on: https://go-review.googlesource.com/55142
Reviewed-by: Keith Randall <khr@golang.org>
eqstring is only called for strings with equal lengths.
Instead of pushing a pointer and length for each argument string
on the stack we can omit pushing one of the lengths on the stack.
Changing eqstrings signature to eqstring(*uint8, *uint8, int) bool
to implement the above optimization would make it very similar to the
existing memequal(*any, *any, uintptr) bool function.
Since string lengths are positive we can avoid code redundancy and
use memequal instead of using eqstring with an optimized signature.
go command binary size reduced by 4128 bytes on amd64.
name old time/op new time/op delta
CompareStringEqual 6.03ns ± 1% 5.71ns ± 1% -5.23% (p=0.000 n=19+18)
CompareStringIdentical 2.88ns ± 1% 3.22ns ± 7% +11.86% (p=0.000 n=20+20)
CompareStringSameLength 4.31ns ± 1% 4.01ns ± 1% -7.17% (p=0.000 n=19+19)
CompareStringDifferentLength 0.29ns ± 2% 0.29ns ± 2% ~ (p=1.000 n=20+20)
CompareStringBigUnaligned 64.3µs ± 2% 64.1µs ± 3% ~ (p=0.164 n=20+19)
CompareStringBig 61.9µs ± 1% 61.6µs ± 2% -0.46% (p=0.033 n=20+19)
Change-Id: Ice15f3b937c981f0d3bc8479a9ea0d10658ac8df
Reviewed-on: https://go-review.googlesource.com/53650
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
If there are no pointers, then clearing memory doesn't help GC,
and the memory is otherwise dead, so don't bother clearing it.
Change-Id: I953f4a3264939f2825e82292030eda2e835cbb97
Reviewed-on: https://go-review.googlesource.com/57350
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Because profile labels are copied from the goroutine into the tag
buffer by the signal handler, there's a carefully-crafted set of race
detector annotations to create the necessary happens-before edges
between setting a goroutine's profile label and retrieving it from the
profile tag buffer.
Given the constraints of the signal handler, we have to approximate
the true synchronization behavior. Currently, that approximation is
too weak.
Ideally, runtime_setProfLabel would perform a store-release on
&getg().labels and copying each label into the profile would perform a
load-acquire on &getg().labels. This would create the necessary
happens-before edges through each individual g.labels object.
Since we can't do this in the signal handler, we instead synchronize
on a "labelSync" global. The problem occurs with the following
sequence:
1. Goroutine 1 calls setProfLabel, which does a store-release on
labelSync.
2. Goroutine 2 calls setProfLabel, which does a store-release on
labelSync.
3. Goroutine 3 reads the profile, which does a load-acquire on
labelSync.
The problem is that the load-acquire only synchronizes with the *most
recent* store-release to labelSync, and the two store-releases don't
synchronize with each other. So, once goroutine 3 touches the label
set by goroutine 1, we report a race.
The solution is to use racereleasemerge. This is like a
read-modify-write, rather than just a store-release. Each RMW of
labelSync in runtime_setProfLabel synchronizes with the previous RMW
of labelSync, and this ultimately carries forward to the load-acquire,
so it synchronizes with *all* setProfLabel operations, not just the
most recent.
Change-Id: Iab58329b156122002fff12cfe64fbeacb31c9613
Reviewed-on: https://go-review.googlesource.com/56670
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Minor refactoring. This is a step towards specializing evacuate
for mapfast key types.
Change-Id: Icffe2759b7d38e5c008d03941918d5a912ce62f6
Reviewed-on: https://go-review.googlesource.com/56933
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Since oldbucket == h.nevacuate, we can just increment h.nevacuate here.
This removes oldbucket from scope, which will be useful shortly.
Change-Id: I70f81ec3995f17845ebf5d77ccd20ea4338f23e6
Reviewed-on: https://go-review.googlesource.com/56932
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The number of times that alg has to be spilled
and restored makes it better to just reload it.
Change-Id: I2674752a889ecad59dab54da1d68fad03db1ca85
Reviewed-on: https://go-review.googlesource.com/56931
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new code is not quite equivalent to the old,
in that if newbit was very large it might have altered the new tophash.
The old behavior is unnecessary and probably undesirable.
Change-Id: I7fb3222520cb61081a857adcddfbb9078ead7122
Reviewed-on: https://go-review.googlesource.com/56930
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
After the key and value arrays, we have an overflow pointer.
So there's no way a past-the-end key or value pointer could point
past the end of the containing bucket.
So we don't need this additional protection.
Update #21459
Change-Id: I7726140033b06b187f7a7d566b3af8cdcaeab0b0
Reviewed-on: https://go-review.googlesource.com/56772
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Avelino <t@avelino.xxx>
Found with mvdan.cc/unindent. Prioritized the ones with the biggest wins
for now.
Change-Id: I2b032e45cdd559fc9ed5b1ee4c4de42c4c92e07b
Reviewed-on: https://go-review.googlesource.com/56470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use mallogc instead of newarray to save some overhead since
makechan already checks for _MaxMem constraints.
Flattens the if else construct that determines if buf and hchan struct
should be allocated in one mallocgc call and where buf should point to.
Uses maxSliceCap to avoid divisions similar to makeslice.
name old time/op new time/op delta
MakeChan/Byte 82.0ns ± 8% 81.4ns ± 7% ~ (p=0.643 n=10+10)
MakeChan/Int 97.9ns ± 2% 96.6ns ± 2% -1.40% (p=0.009 n=10+10)
MakeChan/Ptr 128ns ± 3% 120ns ± 1% -6.63% (p=0.000 n=10+10)
MakeChan/Struct/0 66.7ns ± 4% 66.4ns ± 2% ~ (p=0.697 n=10+10)
MakeChan/Struct/32 136ns ± 1% 130ns ± 0% -4.42% (p=0.000 n=10+10)
MakeChan/Struct/40 150ns ± 1% 150ns ± 1% ~ (p=0.725 n=10+10)
Change-Id: Ibb5675d0843a072aae2bfa58ecd39cf4cd926533
Reviewed-on: https://go-review.googlesource.com/55132
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Stack allocated hmap structs are explicitly zeroed before being
passed by pointer to makemap.
Heap allocated hmap structs are created with newobject
which also zeroes on allocation.
Therefore, setting the hmap fields to 0 or nil is redundant
since they will have been zeroed when hmap was allocated.
Change-Id: I5fc55b75e9dc5ba69f5e3588d6c746f53b45ba66
Reviewed-on: https://go-review.googlesource.com/56291
Reviewed-by: Keith Randall <khr@golang.org>
We use a call to strncpy to work around a TSAN bug (wherein TSAN only
delivers asynchronous signals when the thread receiving the signal
calls a libc function). Unfortunately, GCC 7 inlines the call,
avoiding the TSAN libc trap entirely.
Per Ian's suggestion, use global variables as strncpy arguments: that
way, the compiler can't make any assumptions about the concrete values
and can't inline the call away.
fixes#21196
Change-Id: Ie95f1feaf9af1a8056f924f49c29cfc8515385d7
Reviewed-on: https://go-review.googlesource.com/55872
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The code was adding race.Errors to t.raceErrors before checking
Failed, but Failed was using t.raceErrors+race.Errors. We don't want
to change Failed, since that would affect tests themselves, so modify
the harness to not unnecessarily change t.raceErrors.
Updates #19851Fixes#21338
Change-Id: I7bfdf281f90e045146c92444f1370d55c45221d4
Reviewed-on: https://go-review.googlesource.com/54050
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rather than emitting spaces and newlines for println
as we walk the expression, construct it all up front.
This enables further optimizations.
This requires using printstring instead of print in
the implementation of printsp and printnl,
on pain of infinite recursion.
That's ok; it's more efficient anyway, and just as simple.
While we're here, do it for other print routines as well.
Change-Id: I61d7df143810e00710c4d4d948d904007a7fd190
Reviewed-on: https://go-review.googlesource.com/55097
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Writing to selectdone on the stack of another goroutine meant a
pretty subtle dance between the select code and the stack copying
code. Instead move the selectdone variable into the g struct.
Change-Id: Id246aaf18077c625adef7ca2d62794afef1bdd1b
Reviewed-on: https://go-review.googlesource.com/53390
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I noticed that we don't set an itab's function pointers at compile
time. Instead, we currently do it at executable startup.
Set the function pointers at compile time instead. This shortens
startup time. It has no effect on normal binary size. Object files
will have more relocations, but that isn't a big deal.
For PIE there are additional pointers that will need to be adjusted at
load time. There are already other pointers in an itab that need to be
adjusted, so the cache line will already be paged in. There might be
some binary size overhead to mark these pointers. The "go test -c
-buildmode=pie net/http" binary is 0.18% bigger.
Update #20505
Change-Id: I267c82489915b509ff66e512fc7319b2dd79b8f7
Reviewed-on: https://go-review.googlesource.com/44341
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Currently, GC captures the start-the-world time stamp after
startTheWorldWithSema returns. This is problematic for two reasons:
1. It's possible to get preempted between startTheWorldWithSema
starting the world and calling nanotime.
2. startTheWorldWithSema does several clean-up tasks after the world
is up and running that on rare occasions can take upwards of 10ms.
Since the runtime uses the start-the-world time stamp to compute the
STW duration, both of these can significantly inflate the reported STW
duration.
Fix this by having startTheWorldWithSema itself call nanotime once the
world is started.
Change-Id: I114630234fb73c9dabae50a2ef1884661f2459db
Reviewed-on: https://go-review.googlesource.com/55410
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
struct32 and struct40 structs are already declared, remove them to
make runtime tests build.
Change-Id: I3814f2b850dcb15c4002a3aa22e2a9326e5a5e53
Reviewed-on: https://go-review.googlesource.com/55614
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Instead of comparing if the number of elements will
not fit into memory check if the memory size of the
slices backing memory is higher then the memory limit.
This avoids a division or maxElems lookup.
With et.size > 0:
uintptr(newcap) > maxSliceCap(et.size)
-> uintptr(int(capmem / et.size)) > _MaxMem / et.size
-> capmem / et.size > _MaxMem / et.size
-> capmem > _MaxMem
Note that due to integer division from capmem > _MaxMem
it does not follow that uintptr(newcap) > maxSliceCap(et.size).
Consolidated runtime GrowSlice benchmarks by using sub-benchmarks and
added more struct sizes to show performance improvement when division
is avoided for element sizes larger than 32 bytes.
AMD64:
GrowSlice/Byte 38.9ns ± 2% 38.9ns ± 1% ~ (p=0.974 n=20+20)
GrowSlice/Int 58.3ns ± 3% 58.0ns ± 2% ~ (p=0.154 n=20+19)
GrowSlice/Ptr 95.7ns ± 2% 95.1ns ± 2% -0.60% (p=0.034 n=20+20)
GrowSlice/Struct/24 95.4ns ± 1% 93.9ns ± 1% -1.54% (p=0.000 n=19+19)
GrowSlice/Struct/32 110ns ± 1% 108ns ± 1% -1.76% (p=0.000 n=19+20)
GrowSlice/Struct/40 138ns ± 1% 128ns ± 1% -7.09% (p=0.000 n=20+20)
Change-Id: I1c37857c74ea809da373e668791caffb6a5cbbd3
Reviewed-on: https://go-review.googlesource.com/53471
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
We weren't initializing this field for dynamically-generated itabs.
Turns out it doesn't matter, as any time we use this field we also
generate a static itab for the interface type / concrete type pair.
But we should initialize it anyway, just to be safe.
Performance on the benchmarks in CL 44339:
benchmark old ns/op new ns/op delta
BenchmarkItabFew-12 1040585 26466 -97.46%
BenchmarkItabAll-12 228873499 4287696 -98.13%
Change-Id: I58ed2b31e6c98b584122bdaf844fee7268b58295
Reviewed-on: https://go-review.googlesource.com/44475
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
We don't use it any more, remove it.
Change-Id: I76ce1a4c2e7048fdd13a37d3718b5abf39ed9d26
Reviewed-on: https://go-review.googlesource.com/44474
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Just use fun[0]==0 to indicate a bad itab.
Change-Id: I28ecb2d2d857090c1ecc40b1d1866ac24a844848
Reviewed-on: https://go-review.googlesource.com/44473
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Keep itabs in a growable hash table.
Use a simple open-addressable hash table, quadratic probing, power
of two sized.
Synchronization gets a bit more tricky. The common read path now
has two atomic reads, one to get the table pointer and one to read
the entry out of the table.
I set the max load factor to 75%, kind of arbitrarily. There's a
space-speed tradeoff here, and I'm not sure where we should land.
Because we use open addressing the itab.link field is no longer needed.
I'll remove it in a separate CL.
Fixes#20505
Change-Id: Ifb3d9a337512d6cf968c1fceb1eeaf89559afebf
Reviewed-on: https://go-review.googlesource.com/44472
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Last runtime use was removed in https://golang.org/cl/133700043,
September 2014.
Replace plan9 syscall uses with plan9-specific variable.
Change-Id: Ifb910c021c1419a7c782959f90b054ed600d9e19
Reviewed-on: https://go-review.googlesource.com/55450
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The preceding cleanup made it clear that two cases
(have golden data, unreachable key) are handled identically.
Simplify the control flow to reflect that.
Simplifies the code and generates shorter machine code.
Change-Id: Id612e0da6679813e855506f47222c58ea6497d70
Reviewed-on: https://go-review.googlesource.com/55093
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This change unifies the x and y cases.
It shrinks evacuate's machine code by ~25% and its stack size by ~15%.
It also eliminates a critical branch.
Whether an entry should go to x or y is designed to be unpredictable.
As a result, half of the branch predictions for useX were wrong.
Mispredicting that branch can easily incur an expensive cache miss.
Switching to an xy array allows elimination of that branch,
which in turn reduces cache misses.
Change-Id: Ie9cef53744b96c724c377ac0985b487fc50b49b1
Reviewed-on: https://go-review.googlesource.com/54653
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Make the calculation of k and v a bit lazier.
None of the following code cares about indirect-vs-direct k,
and it happens on all code paths, so check t.indirectkey earlier.
Simplifies the code and reduces both machine code and stack size.
Change-Id: I5ea4c0772848d7a4b15383baedb9a1f7feb47201
Reviewed-on: https://go-review.googlesource.com/55092
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This avoids division and multiplication.
Instrumentation suggests that this is a very common case.
Change-Id: I2d5d5012d4f4df4c4af1f9f85ca9c323c9889c0e
Reviewed-on: https://go-review.googlesource.com/54657
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This avoids the never triggered capacity checks in newarray.
Change-Id: Ib72b204adcb9e3fd3ab963defe0cd40e22d5d492
Reviewed-on: https://go-review.googlesource.com/54731
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This makes sure that its argument is marked live on entry.
We need its arg to be live so defers of KeepAlive get
scanned correctly by the GC.
Fixes#21402
Change-Id: I906813e433d0e9726ca46483723303338da5b4d7
Reviewed-on: https://go-review.googlesource.com/55150
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change replaces the current runtime capabilities check for ppc64x with the
new internal/cpu package. It also adds support for the new POWER9 ISA and
capabilities.
Updates #15403
Change-Id: I5b64a79e782f8da3603e5529600434f602986292
Reviewed-on: https://go-review.googlesource.com/53830
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
This generates better code.
Masking B in the return statement should be unnecessary,
but the compiler is understandably not yet clever enough to see that.
Someday, it'd also be nice for the compiler to generate
a CMOV for the saturation if statement.
Change-Id: Ie1c157b21f5212610da1f3c7823a93816b3b61b9
Reviewed-on: https://go-review.googlesource.com/54656
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Combine conditions into a single if statement.
This is more readable.
It should generate identical machine code, but it doesn't.
The new code is shorter.
Change-Id: I9bf52f8f288b0df97a2b9b4e4183f6ca74175e8a
Reviewed-on: https://go-review.googlesource.com/54651
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
This reduces the wall time to run these benchmarks by about 30%.
Change-Id: I494a93c93e5acb1514510d85f65796f62e1629a5
Reviewed-on: https://go-review.googlesource.com/54650
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Currently we only support finding symbols in the VDSO using the old
DT_HASH. These days everything uses DT_GNU_HASH instead. To keep up
with the times and future-proof against DT_HASH disappearing from the
VDSO in the future, this commit adds support for DT_GNU_HASH and
prefers it over DT_HASH.
Tested by making sure it found a DT_GNU_HASH section and all of the
expected symbols in it, and then disabling the DT_GNU_HASH path and
making sure the old DT_HASH path still found all of the symbols.
Fixes#19649.
Change-Id: I508c8b35a019330d2c32f04f3833b69cb2686f13
Reviewed-on: https://go-review.googlesource.com/45511
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The dieFromSignal runtime function attempts to forward crashing
signals to a signal handler registered before the runtime was
initialized, if any. However, on Darwin, a special signal handler
trampoline is invoked, even for non-Go signal handlers.
Clear the crashing signal's handlingSig entry to ensure sigtramp
forwards the signal.
Fixes the darwin/386 builder.
Updates #20392
Updates #19389
Change-Id: I441a3d30c672cdb21ed6d8f1e1322d7c0e5b9669
Reviewed-on: https://go-review.googlesource.com/55032
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
According to http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html,
pthread_key_create return an error number which is greater than or equal
to 0. I don't know the scenario that pthread_setspecific would fail, but
also don't know the future. Add some error handlings just in case.
Change-Id: I0774b79ef658d67e300f4a9aab1f2e3879acc7ee
Reviewed-on: https://go-review.googlesource.com/54811
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
_main has an early check to verify if a binary is statically or dynamically
linked that depends on R0 being zero. R0 is not guaranteed to be zero at that
point and this was breaking Go on Alpine for ppc64le.
Change-Id: I4a1059ff7fd3db6fc489e7dcfe631c1814dd965b
Reviewed-on: https://go-review.googlesource.com/54730
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Although mincore is declared in stubs.go, mincore isn't used by any
OSes except linux. Move it to os_linux.go and clean up unused code.
Change-Id: I6cfb0fed85c0317a4d091a2722ac55fa79fc7c9a
Reviewed-on: https://go-review.googlesource.com/54910
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This computes the maximum possible waste in a size class due to both
internal and external fragmentation as a percent of the span size.
This parallels the reasoning about overhead in the comment at the top
of mksizeclasses.go and confirms that comment's assertion that (except
for the few smallest size classes), none of the size classes have
worst-case internal and external fragmentation simultaneously.
Change-Id: Idb66fe6c241d56f33d391831d4cd5a626955562b
Reviewed-on: https://go-review.googlesource.com/49370
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Before this CL, whenever the Go runtime wanted to kill its own
process with a signal dieFromSignal would reset the signal handler
to _SIG_DFL.
Unfortunately, if any signal handler were installed before the Go
runtime initialized, it wouldn't be invoked either.
Instead, use whatever signal handler was installed before
initialization.
The motivating use case is Crashlytics on Android. Before this CL,
Crashlytics would not consider a crash from a panic() since the
corresponding SIGABRT never reached its signal handler.
Updates #11382
Updates #20392 (perhaps even fixes it)
Fixes#19389
Change-Id: I0c8633329433b45cbb3b16571bea227e38e8be2e
Reviewed-on: https://go-review.googlesource.com/49590
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
To avoid gigantic core dumps, the runtime avoids raising SIGABRT
on crashes on 64-bit Darwin systems. Mobile OS'es (probably) don't
generate huge core dumps, so to aid crash reporters, allow SIGABRT
on crashes on darwin/arm64.
Change-Id: I4a29608f400967d76f9bd0643fea22244c2da9df
Reviewed-on: https://go-review.googlesource.com/49770
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change enables buildmode c-shared on ppc64le.
A bug was fixed in runtime/rt0_linux_ppc64le.s that was necessary to
make this work. In _rt0_ppc64le_linux_lib, there is code to store
the value of r2 onto the caller's stack. However, if this file
is compiled using a build mode that maintains the TOC address in
r2, then instructions will be inserted at the beginning of this
function to generate the r2 value for the callee, not the caller.
That means the r2 value for the callee is stored onto the caller's
stack. If caller and callee don't have the same r2 values, then
the caller will restore the wrong r2 value after it returns. This
situation can happen when using dlopen since the caller of this
function will be in ld64.so and will definitely have a different
TOC.
Updates #20756
Change-Id: I6e165e0d0716e73721bbbcc520e8302e4856e3ba
Reviewed-on: https://go-review.googlesource.com/53890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We use lock-free reads from mheap.spans, but the safety of these is
somewhat subtle. Document this.
Change-Id: I928c893232176135308e38bed788d5f84ff11533
Reviewed-on: https://go-review.googlesource.com/54310
Reviewed-by: Rick Hudson <rlh@golang.org>
The compiler is now smart enough not to insert a bounds check.
Not only is this simpler, it eliminates a LEAQ from the
generated code.
Change-Id: Ie90cbd11584542edd99edd5456d9b02c406e8063
Reviewed-on: https://go-review.googlesource.com/53892
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It appears that this was just missed
by accident in the original implementation.
Change-Id: Id87147bcb7a685d624eac7034342a305ad644e7a
Reviewed-on: https://go-review.googlesource.com/53891
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Avelino <t@avelino.xxx>
According to http://infocenter.arm.com:
* ARM Cortex-A53 (Raspberry Pi 3, Pine A64)
* ARM Cortex-A57 (Opteron A1100, Tegra X1)
* ARM Cortex-A72
all have a cache line size of 64 bytes.
Change-Id: I4b333e930792fb1a221b3ca6f395bfa1b7762afa
Reviewed-on: https://go-review.googlesource.com/43250
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The only non test user of the assembler prefetch functions is the
heapBits.prefetch function which is itself unused.
The runtime prefetch functions have no functionality on most platforms
and are not inlineable since they are written in assembler. The function
call overhead eliminates the performance gains that could be achieved with
prefetching and would degrade performance for platforms where the functions
are no-ops.
If prefetch functions are needed back again later they can be improved
by avoiding the function call overhead and implementing them as intrinsics.
Change-Id: I52c553cf3607ffe09f0441c6e7a0a818cb21117d
Reviewed-on: https://go-review.googlesource.com/44370
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We need to make sure that when the key contains a pointer, we use
a write barrier to update the key.
Also mapdelete_* should use typedmemclr.
Fixes#21297
Change-Id: I63dc90bec1cb909c2c6e08676c9ec853d736cdf8
Reviewed-on: https://go-review.googlesource.com/53414
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The activeModules function is called by the cgo pointer checking code,
which is called by the write barrier (when GODEBUG=cgocheck=2), and as
such must be nosplit/nowritebarrier.
Fixes#21306
Change-Id: I57f2124f14de7f3872b2de9532abab15df95d45a
Reviewed-on: https://go-review.googlesource.com/53352
Reviewed-by: Austin Clements <austin@google.com>
We lazily map the bitmap and spans areas as the heap grows. However,
right now we're very slightly too lazy. Specifically, the following
can happen on 32-bit:
1. mallocinit fails to allocate any heap arena, so
arena_used == arena_alloc == arena_end == bitmap.
2. There's less than 256MB between the end of the bitmap mapping and
the next mapping.
3. On the first allocation, mheap.sysAlloc sees that there's not
enough room in [arena_alloc, arena_end) because there's no room at
all. It gets a 256MB mapping from somewhere *lower* in the address
space than arena_used and sets arena_alloc and arena_end to this
hole.
4. Since the new arena_alloc is lower than arena_used, mheap.sysAlloc
doesn't bother to call mheap.setArenaUsed, so we still don't have a
bitmap mapping or a spans array mapping.
5. mheap.grow, which called mheap.sysAlloc, attempts to fill in the
spans array and crashes.
Fix this by mapping the metadata regions for the initial arena_used
when the heap is initialized, rather than trying to wait for an
allocation. This maintains the intended invariant that the structures
are always mapped for [arena_start, arena_used).
Fixes#21044.
Change-Id: I4422375a6e234b9f979d22135fc63ae3395946b0
Reviewed-on: https://go-review.googlesource.com/51714
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Right now, if it's possible to grow the arena reservation but
mheap.sysAlloc fails to get 256MB more of memory, it simply fails.
However, on 32-bit we have a fallback path that uses much smaller
mmaps that could take in this situation, but fail to.
This commit fixes mheap.sysAlloc to use a common failure path in case
it can't grow the reservation. On 32-bit, this path includes the
fallback.
Ideally, mheap.sysAlloc would attempt smaller reservation growths
first, but taking the fallback path is a simple change for Go 1.9.
Updates #21044 (fixes one of two issues).
Change-Id: I1e0035ffba986c3551479d5742809e43da5e7c73
Reviewed-on: https://go-review.googlesource.com/51713
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
64bit atomics on mips/mipsle are implemented using spinlocks. If SIGPROF
is received while the program is in the critical section, it will try to
write the sample using the same spinlock, creating a deadloop.
Prevent it by creating a counter of SIGPROFs during atomic64 and
postpone writing the sample(s) until called from elsewhere, with
pc set to _LostSIGPROFDuringAtomic64.
Added a test case, per Cherry's suggestion. Works around #20146.
Change-Id: Icff504180bae4ee83d78b19c0d9d6a80097087f9
Reviewed-on: https://go-review.googlesource.com/42652
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The runtime tests may be invoked from a parent that has SIGQUIT
blocked. For example, Java invokes subprocesses this way. In this
situation, TestCrashDumpsAllThreads and TestPanicSystemstack will fail
because they depend on SIGQUIT to get tracebacks, and any subprocess
test that times out will fail to kill the subprocess.
Fix this by detecting if SIGQUIT is blocked and, if so, skipping tests
that depend on it and using SIGKILL to kill timed-out subprocesses.
Based on a fix by Carl Henrik Lunde in
https://golang.org/issue/19196#issuecomment-316145733Fixes#19196.
Change-Id: Ia20bf15b96086487d0ef6b75239dcc260c21714c
Reviewed-on: https://go-review.googlesource.com/50330
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If we are using vfork, and if something (such as TSAN) is intercepting
the sigaction function, then we must call the system call, not the
libc function. Otherwise the intercepted sigaction call in the child
may trash the data structures in the parent.
Change-Id: Id9588bfeaa934f32c920bf829c5839be5cacf243
Reviewed-on: https://go-review.googlesource.com/50251
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Currently we trace mark assists even if they're satisfied entirely by
stealing. This means even if background marking is keeping up with
allocation, we'll still emit a trace event every N bytes of
allocation. The event will be a few microseconds, if that, but they're
frequent enough that, when zoomed out in the trace view, it looks like
all of the time is spent in mark assists even if almost none is.
Change this so we only emit a trace event if the assist actually has
to do assisting. This makes the traces of these events far more
useful.
Change-Id: If4aed1c413b814341ef2fba61d2f10751d00451b
Reviewed-on: https://go-review.googlesource.com/50030
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
tSweepTerm and pauseStart are supposed to be when STW was triggered,
but right now they're captured a bit before STW. Move these down to
immediately before we trigger STW.
Fixes#19590.
Change-Id: Icd48a5c4d45c9b36187ff986e4f178b5064556c1
Reviewed-on: https://go-review.googlesource.com/49612
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, Windows stacks are either 128kB or 2MB depending on whether
the binary uses cgo. This is because we assume that Go system stacks
and the small amount of C code invoked by the standard library can
operate within smaller stacks, but general Windows C code assumes
larger stacks.
However, it's easy to call into arbitrary C code using the syscall
package on Windows without ever importing cgo into a binary. Such
binaries need larger system stacks even though they don't use cgo.
Fix this on 64-bit by increasing the system stack size to 2MB always.
This only costs address space, which is free enough on 64-bit to not
worry about. We keep (for now) the existing heuristic on 32-bit, where
address space comes at more of a premium.
Updates #20975.
Change-Id: Iaaaa9a2fcbadc825cddc797aaaea8d34ef8debf2
Reviewed-on: https://go-review.googlesource.com/49331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
kicking off contributing again with a classic
Change-Id: Ifb0aed8f1dc854f85751ce0495967a3c4315128d
Reviewed-on: https://go-review.googlesource.com/49016
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It seems that when too much other code is running on the system,
the testprogcgo code can overrun its timeouts.
Updates #18598.
Not marking the issue as fixed until it doesn't recur for some time.
Change-Id: Ieaf106b41986fdda76b1d027bb9d5e3fb805cc3b
Reviewed-on: https://go-review.googlesource.com/48233
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
SysV semaphore undo lists should be shared by threads, just like
several other resources listed in cloneFlags. Currently we don't do
this, but it probably doesn't affect anything because 1) probably
nobody uses SysV semaphores from Go and 2) Go-created threads never
exit until the process does. Beyond being the right thing to do,
user-level QEMU requires this flag because it depends on glibc to
create new threads and glibc uses this flag.
Fixes#20763.
Change-Id: I1d1dafec53ed87e0f4d4d432b945e8e68bb72dcd
Reviewed-on: https://go-review.googlesource.com/48170
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestStackGrowth is currently a parallel test. However, it depends on a
20 second timeout, which is already dubious in a parallel test, and
became really problematic on slow builders when runtime.GC switched to
triggering concurrent GC instead of STW GC. Before that change, the
test spent much of its time in STW GC, so it wasn't *really* parallel.
After that change, it was competing with all of the other parallel
tests and GC likely started taking ~4 times longer. On most builders
the whole test runs in well under a second, but on the slow builders
that was enough to push it over the 20 second timeout.
Fix this by making the test serial.
Updates #19381 (probably fixes it, but we'll have to wait and see).
Change-Id: I21af7cf543ab07f1ec1c930bfcb355b0df75672d
Reviewed-on: https://go-review.googlesource.com/48110
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The current description refers to the outermost "frame" which can be
misleading. A user reading it can think it means a stack frame.
Change-Id: Ie2c7cb4b4db8f41572df206478ce3b46a0245a5d
Reviewed-on: https://go-review.googlesource.com/47850
Reviewed-by: Austin Clements <austin@google.com>
Currently, sysmon waits 60 ms during idle before relaxing. This is
primarily to avoid reducing the precision of short-duration timers. Of
course, if there are no short-duration timers, this wastes 60 ms
running the timer at high resolution.
Improve this by instead inspecting the time until the next timer fires
and relaxing the timer resolution immediately if the next timer won't
fire for a while.
Updates #20937.
Change-Id: If4ad0a565b65a9b3e8c4cdc2eff1486968c79f24
Reviewed-on: https://go-review.googlesource.com/47833
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, sysmon relaxes the Windows timer resolution as soon as the
Go process becomes idle. However, if it's going idle because of a
short sleep (< 15.6 ms), this can turn that short sleep into a long
sleep (15.6 ms).
To address this, wait for 60 ms of idleness before relaxing the timer
resolution. It would be better to check the time until the next wakeup
and relax immediately if it makes sense, but there's currently no
interaction between sysmon and the timer subsystem, so adding this
simple delay is a much simpler and safer change for late in the
release cycle.
Fixes#20937.
Change-Id: I817db24c3bdfa06dba04b7bc197cfd554363c379
Reviewed-on: https://go-review.googlesource.com/47832
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
R11 is callee-save in the C ABI, but the temporary register in the Go
ABI. Currently it's being clobbered by runtime.addmoduledata, which
has to follow the C ABI. The observed effect of this was that
dl_open_worker was returning to a bad PC because after it failed to
restore its SP because it was using R11 as a frame pointer.
Fix this by saving R11 around addmoduledata.
Fixes#19674.
Change-Id: Iaacbcc76809a3aa536e9897770831dcbcb6c8245
Reviewed-on: https://go-review.googlesource.com/47831
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently only the rwmutex write lock prevents descheduling. The read
lock does not. This leads to the following situation:
1. A reader acquires the lock and gets descheduled.
2. GOMAXPROCS writers attempt to acquire the lock (or at least one
writer does, followed by readers). This blocks all of the Ps.
3. There is no 3. The descheduled reader never gets to run again
because there are no Ps, so it never releases the lock and the system
deadlocks.
Fix this by preventing descheduling while holding the read lock. This
requires also rewriting TestParallelRWMutexReaders to always create
enough GOMAXPROCS and to use non-blocking operations for
synchronization.
Fixes#20903.
Change-Id: Ibd460663a7e5a555be5490e13b2eaaa295fac39f
Reviewed-on: https://go-review.googlesource.com/47632
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The name LabelList was changed to LabelSet during the development of the
proposal [1], except in one function comment. This commit fixes that.
Fixes#20905.
[1] https://github.com/golang/go/issues/17280
Change-Id: Id4f48d59d7d513fa24b2e42795c2baa5ceb78f36
Reviewed-on: https://go-review.googlesource.com/47470
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
mheap.allocLarge just calls bestFitTreap and is the only caller of
bestFitTreap. Flatten these into a single function. Also fix their
comments: allocLarge claims to return exactly npages but can in fact
return a larger span, and h.freelarge is not in fact indexed by span
start address.
Change-Id: Ia20112bdc46643a501ea82ea77c58596bc96f125
Reviewed-on: https://go-review.googlesource.com/47315
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The Func type has allowed calling the Func.Name method on a nil pointer
since Go1.2, where it returned an empty string. A regression caused by
CL/37331 caused this behavior to change. This breaks code that lazily
does runtime.FuncForPC(myPtr).Name() without first checking that myPtr
is actually non-nil.
Fixes#20872
Change-Id: Iae9a2ebabca5e9d1f5a2cdaf2f30e9c6198fec4f
Reviewed-on: https://go-review.googlesource.com/47354
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently the execLock is a mutex, which has the unfortunate
side-effect of serializing all thread creation. This replaces it with
an rwmutex so threads can be created in parallel, but exec still
blocks thread creation.
Fixes#20738.
Change-Id: Ia8f30a92053c3d28af460b0da71176abe5fd074b
Reviewed-on: https://go-review.googlesource.com/47072
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently runtime.rwmutex is written to block the calling goroutine
rather than the calling thread. However, rwmutex was intended to be
used in the scheduler, which means it needs to be a thread-level
synchronization primitive.
Hence, this modifies rwmutex to synchronize threads instead of
goroutines. This has the consequence of making it write-barrier-free,
which is also important for using it in the scheduler.
The implementation makes three changes: it replaces the "w" semaphore
with a mutex, since this was all it was being used for anyway; it
replaces "writerSem" with a single pending M that parks on its note;
and it replaces "readerSem" with a list of Ms that park on their notes
plus a pass count that together emulate a counting semaphore. I
model-checked the safety and liveness of this implementation through
>1 billion schedules.
For #20738.
Change-Id: I3cf5a18c266a96a3f38165083812803510217787
Reviewed-on: https://go-review.googlesource.com/47071
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When the dedicated mark worker runs, the scheduler won't run on that P
again until GC runs out of mark work. As a result, any goroutines in
that P's local run queue are stranded until another P steals them. In
a normally operating system this may take a long time, and in a 100%
busy system, the scheduler never attempts to steal from another P.
Fix this by draining the local run queue into the global run queue if
the dedicated mark worker has run for long enough. We don't do this
immediately upon scheduling the dedicated mark worker in order to
avoid destroying locality if the mark worker runs for a short time.
Instead, the scheduler delays draining the run queue until the mark
worker gets its first preemption request (and otherwise ignores the
preemption request).
Fixes#20011.
Change-Id: I13067194b2f062b8bdef25cb75e4143b7fb6bb73
Reviewed-on: https://go-review.googlesource.com/46610
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
