Skip to content

G
golang
Switch branch/tag
  1. 30 Jul, 2015 20 commits
  3. 29 Jul, 2015 19 commits
    • Russ Cox's avatar
      runtime: set invalidptr=1 by default, as documented · d3ffc975
      Russ Cox authored 
      Also make invalidptr control the recently added GC pointer check,
as documented.

Change-Id: Iccfdf49480219d12be8b33b8f03d8312d8ceabed
Reviewed-on: https://go-review.googlesource.com/12857
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: 's avatarRob Pike <r@golang.org>
      d3ffc975
    • Andrew Gerrand's avatar
      doc: remove non-answer from FAQ · e4bd8e04
      Andrew Gerrand authored 
      Change-Id: Ie43986d016e5a9fb17ca1393263932bbb56e81ff
Reviewed-on: https://go-review.googlesource.com/12836Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      e4bd8e04
    • Russ Cox's avatar
      runtime/trace: remove existing Skips · bd5ca222
      Russ Cox authored 
      The skips added in CL 12579, based on incorrect time stamps,
should be sufficient to identify and exclude all the time-related
flakiness on these systems.

If there is other flakiness, we want to find out.

For #10512.

Change-Id: I5b588ac1585b2e9d1d18143520d2d51686b563e3
Reviewed-on: https://go-review.googlesource.com/12746Reviewed-by: 's avatarAustin Clements <austin@google.com>
      bd5ca222
    • Russ Cox's avatar
      runtime/trace: record event sequence numbers explicitly · 80c98fa9
      Russ Cox authored 
      Nearly all the flaky failures we've seen in trace tests have been
due to the use of time stamps to determine relative event ordering.
This is tricky for many reasons, including:
 - different cores might not have exactly synchronized clocks
 - VMs are worse than real hardware
 - non-x86 chips have different timer resolution than x86 chips
 - on fast systems two events can end up with the same time stamp

Stop trying to make time reliable. It's clearly not going to be for Go 1.5.
Instead, record an explicit event sequence number for ordering.
Using our own counter solves all of the above problems.

The trace still contains time stamps, of course. The sequence number
is just used for ordering.

Should alleviate #10554 somewhat. Then tickDiv can be chosen to
be a useful time unit instead of having to be exact for ordering.

Separating ordering and time stamps lets the trace parser diagnose
systems where the time stamp order and actual order do not match
for one reason or another. This CL adds that check to the end of
trace.Parse, after all other sequence order-based checking.
If that error is found, we skip the test instead of failing it.
Putting the check in trace.Parse means that cmd/trace will pick
up the same check, refusing to display a trace where the time stamps
do not match actual ordering.

Using net/http's BenchmarkClientServerParallel4 on various CPU counts,
not tracing vs tracing:

name                      old time/op    new time/op    delta
ClientServerParallel4       50.4µs ± 4%    80.2µs ± 4%  +59.06%        (p=0.000 n=10+10)
ClientServerParallel4-2     33.1µs ± 7%    57.8µs ± 5%  +74.53%        (p=0.000 n=10+10)
ClientServerParallel4-4     18.5µs ± 4%    32.6µs ± 3%  +75.77%        (p=0.000 n=10+10)
ClientServerParallel4-6     12.9µs ± 5%    24.4µs ± 2%  +89.33%        (p=0.000 n=10+10)
ClientServerParallel4-8     11.4µs ± 6%    21.0µs ± 3%  +83.40%        (p=0.000 n=10+10)
ClientServerParallel4-12    14.4µs ± 4%    23.8µs ± 4%  +65.67%        (p=0.000 n=10+10)

Fixes #10512.

Change-Id: I173eecf8191e86feefd728a5aad25bf1bc094b12
Reviewed-on: https://go-review.googlesource.com/12579Reviewed-by: 's avatarAustin Clements <austin@google.com>
      80c98fa9
    • Russ Cox's avatar
      runtime: ignore arguments in cgocallback_gofunc frame · fde39262
      Russ Cox authored 
      Otherwise the GC may see uninitialized memory there,
which might be old pointers that are retained, or it might
trigger the invalid pointer check.

Fixes #11907.

Change-Id: I67e306384a68468eef45da1a8eb5c9df216a77c0
Reviewed-on: https://go-review.googlesource.com/12852Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
Reviewed-by: 's avatarAustin Clements <austin@google.com>
      fde39262
    • Russ Cox's avatar
      runtime: fix darwin/amd64 assembly frame sizes · f6dfe167
      Russ Cox authored 
      Change-Id: I2f0ecdc02ce275feadf07e402b54f988513e9b49
Reviewed-on: https://go-review.googlesource.com/12855Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      f6dfe167
    • Russ Cox's avatar
      cmd/internal/obj/arm64: fix build · c1ccbab0
      Russ Cox authored 
      Change-Id: I3088e17aff72096e3ec2ced49c70564627c982a6
Reviewed-on: https://go-review.googlesource.com/12854Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      c1ccbab0
    • Russ Cox's avatar
      runtime: reenable bad pointer check in GC · 4addec3a
      Russ Cox authored 
      The last time we tried this, linux/arm64 broke.
The series of CLs leading to this one fixes that problem.
Let's try again.

Fixes #9880.

Change-Id: I67bc1d959175ec972d4dcbe4aa6f153790f74251
Reviewed-on: https://go-review.googlesource.com/12849Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarAustin Clements <austin@google.com>
      4addec3a
    • Russ Cox's avatar
      cmd/internal/obj/arm64: reject misaligned stack frames, except empty frames · 034a10d4
      Russ Cox authored 
      The layout code has to date insisted on stack frames that are 16-aligned
including the saved LR, and it ensured this by growing the frame itself.
This breaks code that refers to values near the top of the frame by positive
offset from SP, and in general it's too magical: if you see TEXT xxx, $N,
you expect that the frame size is actually N, not sometimes N and sometimes N+8.

This led to a serious bug in the compiler where ambiguously live values
were not being zeroed correctly, which in turn triggered an assertion
in the GC about finding only valid pointers. The compiler has been
fixed to always emit aligned frames, and the hand-written assembly
has also been fixed.

Now that everything is aligned, make unaligned an error instead of
something to "fix" silently.

For #9880.

Change-Id: I05f01a9df174d64b37fa19b36a6b6c5f18d5ba2d
Reviewed-on: https://go-review.googlesource.com/12848Reviewed-by: 's avatarAustin Clements <austin@google.com>
      034a10d4
    • Russ Cox's avatar
      cmd/link: fix nosplit stack overflow checks · 767e0658
      Russ Cox authored 
      The nosplit stack overflow checks were confused about morestack.
The comment about not having correct SP information at the call
to morestack was true, but that was a real bug, not something to
work around. I fixed that problem in CL 12144. With that fixed,
no need to special-case morestack in the way done here.

This cleanup and simplification of the code was the first step
to fixing a bug that happened when I started working on the
arm64 frame size adjustments, but the cleanup was sufficient
to make the bug go away.

For #9880.

Change-Id: I16b69a5c16b6b8cb4090295d3029c42d606e3b9b
Reviewed-on: https://go-review.googlesource.com/12846Reviewed-by: 's avatarAustin Clements <austin@google.com>
      767e0658
    • Russ Cox's avatar
      runtime, reflect: use correctly aligned stack frame sizes on arm64 · 42122057
      Russ Cox authored 
      arm64 requires either no stack frame or a frame with a size that is 8 mod 16
(adding the saved LR will make it 16-aligned).

The cmd/internal/obj/arm64 has been silently aligning frames, but it led to
a terrible bug when the compiler and obj disagreed on the frame size,
and it's just generally confusing, so we're going to make misaligned frames
an error instead of something that is silently changed.

This CL prepares by updating assembly files.
Note that the changes in this CL are already being done silently by
cmd/internal/obj/arm64, so there is no semantic effect here,
just a clarity effect.

For #9880.

Change-Id: Ibd6928dc5fdcd896c2bacd0291bf26b364591e28
Reviewed-on: https://go-review.googlesource.com/12845Reviewed-by: 's avatarAustin Clements <austin@google.com>
      42122057
    • Russ Cox's avatar
      cmd/compile: align arm64 stack frames correctly · 3952057c
      Russ Cox authored 
      If the compiler doesn't do it, cmd/internal/obj/arm64 will,
and that will break the zeroing of ambiguously live values
done in zerorange, which in turn produces uninitialized
pointer cells that the GC trips over.

For #9880.

Change-Id: Ice97c30bc8b36d06b7b88d778d87fab8e1827fdc
Reviewed-on: https://go-review.googlesource.com/12847Reviewed-by: 's avatarAustin Clements <austin@google.com>
      3952057c
    • Austin Clements's avatar
      runtime: report GC CPU utilization in MemStats · 23e4744c
      Austin Clements authored 
      This adds a GCCPUFraction field to MemStats that reports the
cumulative fraction of the program's execution time spent in the
garbage collector. This is equivalent to the utilization percent shown
in the gctrace output and makes this available programmatically.

This does make one small effect on the gctrace output: we now report
the duration of mark termination up to just before the final
start-the-world, rather than up to just after. However, unlike
stop-the-world, I don't believe there's any way that start-the-world
can block, so it should take negligible time.

While there are many statistics one might want to expose via MemStats,
this is one of the few that will undoubtedly remain meaningful
regardless of future changes to the memory system.

The diff for this change is larger than the actual change. Mostly it
lifts the code for computing the GC CPU utilization out of the
debug.gctrace path.

Updates #10323.

Change-Id: I0f7dc3fdcafe95e8d1233ceb79de606b48acd989
Reviewed-on: https://go-review.googlesource.com/12844Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      23e4744c
    • Austin Clements's avatar
      runtime: always capture GC phase transition times · 4b71660c
      Austin Clements authored 
      Currently we only capture GC phase transition times if
debug.gctrace>0, but we're about to compute GC CPU utilization
regardless of whether debug.gctrace is set, so we need these
regardless of debug.gctrace.

Change-Id: If3acf16505a43d416e9a99753206f03287180660
Reviewed-on: https://go-review.googlesource.com/12843Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      4b71660c
    • Austin Clements's avatar
      runtime: avoid race between SIGPROF traceback and stack barriers · 87f97c73
      Austin Clements authored 
      The following sequence of events can lead to the runtime attempting an
out-of-bounds access on a stack barrier slice:

1. A SIGPROF comes in on a thread while the G on that thread is in
   _Gsyscall. The sigprof handler calls gentraceback, which saves a
   local copy of the G's stkbar slice. Currently the G has no stack
   barriers, so this slice is empty.

2. On another thread, the GC concurrently scans the stack of the
   goroutine being profiled (it considers it stopped because it's in
   _Gsyscall) and installs stack barriers.

3. Back on the sigprof thread, gentraceback comes across a stack
   barrier in the stack and attempts to look it up in its (zero
   length) copy of G's old stkbar slice, which causes an out-of-bounds
   access.

This commit fixes this by adding a simple cas spin to synchronize the
SIGPROF handler with stack barrier insertion.

In general I would prefer that this synchronization be done through
the G status, since that's how stack scans are otherwise synchronized,
but adding a new lock is a much smaller change and G statuses are full
of subtlety.

Fixes #11863.

Change-Id: Ie89614a6238bb9c6a5b1190499b0b48ec759eaf7
Reviewed-on: https://go-review.googlesource.com/12748Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      87f97c73
    • Rick Hudson's avatar
      runtime: force mutator to give work buffer to GC · e95bc5fe
      Rick Hudson authored 
      The scheduler, work buffer's dispose, and write barriers
can conspire to hide the a pointer from the GC's concurent
mark phase. If this pointer is the only path to a large
amount of marking the STW mark termination phase may take
a lot of time.

Consider the following:
1) dispose places a work buffer on the partial queue
2) the GC is busy so it does not immediately remove and
   process the work buffer
3) the scheduler runs a mutator whose write barrier dequeues the
   work buffer from the partial queue so the GC won't see it
This repeats until the GC reaches the mark termination
phase where the GC finally discovers the pointer along
with a lot of work to do.

This CL fixes the problem by having the mutator
dispose of the buffer to the full queue instead of
the partial queue. Since the write buffer never asks for full
buffers the conspiracy described above is not possible.

Updates #11694.

Change-Id: I2ce832f9657a7570f800e8ce4459cd9e304ef43b
Reviewed-on: https://go-review.googlesource.com/12840Reviewed-by: 's avatarAustin Clements <austin@google.com>
      e95bc5fe
    • Rob Pike's avatar
      doc: add json tokenizer to go1.5.html · ff991940
      Rob Pike authored 
      Change-Id: I45d92fed757fa1866d5b80e53ed1af6712fa6741
Reviewed-on: https://go-review.googlesource.com/12782Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      ff991940
    • Mikio Hara's avatar
      cmd/internal/asm: delete · 861af804
      Mikio Hara authored 
      Updates #10510.

Change-Id: Ib4d39943969d18517b373292b83d87650d5df12a
Reviewed-on: https://go-review.googlesource.com/12787
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      861af804
    • Rob Pike's avatar
      cmd: delete old[5689]a · ba0c142d
      Rob Pike authored 
      These are the old assemblers written in C, and now they are
not needed.

Fixes #10510.

Change-Id: Id9337ffc8eccfd93c84b2e23f427fb1a576b543d
Reviewed-on: https://go-review.googlesource.com/12784Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      ba0c142d
  5. 28 Jul, 2015 1 commit