Skip to content

G
go
Switch branch/tag
  1. 07 Aug, 2015 2 commits
  3. 06 Aug, 2015 8 commits
    • Russ Cox's avatar
      doc: do not call WaitGroup a function · 4b145d42
      Russ Cox authored 
      Fixes #12060.

Change-Id: Ie2fd10bedded1a4f4e0daa0c0c77ecd898480767
Reviewed-on: https://go-review.googlesource.com/13324Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      4b145d42
    • Austin Clements's avatar
      runtime: call goexit1 instead of goexit · ad731887
      Austin Clements authored 
      Currently, runtime.Goexit() calls goexit()—the goroutine exit stub—to
terminate the goroutine. This *mostly* works, but can cause a
"leftover stack barriers" panic if the following happens:

1. Goroutine A has a reasonably large stack.

2. The garbage collector scan phase runs and installs stack barriers
   in A's stack. The top-most stack barrier happens to fall at address X.

3. Goroutine A unwinds the stack far enough to be a candidate for
   stack shrinking, but not past X.

4. Goroutine A calls runtime.Goexit(), which calls goexit(), which
   calls goexit1().

5. The garbage collector enters mark termination.

6. Goroutine A is preempted right at the prologue of goexit1() and
   performs a stack shrink, which calls gentraceback.

gentraceback stops as soon as it sees goexit on the stack, which is
only two frames up at this point, even though there may really be many
frames above it. More to the point, the stack barrier at X is above
the goexit frame, so gentraceback never sees that stack barrier. At
the end of gentraceback, it checks that it saw all of the stack
barriers and panics because it didn't see the one at X.

The fix is simple: call goexit1, which actually implements the process
of exiting a goroutine, rather than goexit, the exit stub.

To make sure this doesn't happen again in the future, we also add an
argument to the stub prototype of goexit so you really, really have to
want to call it in order to call it. We were able to reliably
reproduce the above sequence with a fair amount of awful code inserted
at the right places in the runtime, but chose to change the goexit
prototype to ensure this wouldn't happen again rather than pollute the
runtime with ugly testing code.

Change-Id: Ifb6fb53087e09a252baddadc36eebf954468f2a8
Reviewed-on: https://go-review.googlesource.com/13323Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      ad731887
    • Russ Cox's avatar
      runtime: fix race that dropped GoSysExit events from trace · 26baed6a
      Russ Cox authored 
      This makes TestTraceStressStartStop much less flaky.
Running under stress, it changes the failure rate from
above 1/100 to under 1/50000. That very unlikely
failure happens when an unexpected GoSysExit is
written. Not sure how that happens yet, but it is much
less important.

Fixes #11953.

Change-Id: I034671936334b4f3ab733614ef239aa121d20247
Reviewed-on: https://go-review.googlesource.com/13321Reviewed-by: default avatarDmitry Vyukov <dvyukov@google.com>
      26baed6a
    • Joe Shaw's avatar
      doc: remove duplicate -asmflags mention · 59735588
      Joe Shaw authored 
      Fixes #12053

Change-Id: Icd883b4f1ac944a8ec718c79770a8e3fc6542e3a
Reviewed-on: https://go-review.googlesource.com/13259Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      59735588
    • Russ Cox's avatar
      net/url: allow all valid host chars in RawPath · fced03a5
      Russ Cox authored 
      The old code was only allowing the chars we choose not to escape.
We sometimes prefer to escape chars that do not strictly need it.
Allowing those to be used in RawPath lets people override that
preference, which is in fact the whole point of RawPath (new in Go 1.5).

While we are here, also allow [ ] in RawPath.
This is not strictly spec-compliant, but it is what modern browers
do and what at least some people expect, and the [ ] do not cause
any ambiguity (the usual reason they would be escaped, as they are
part of the RFC gen-delims class).
The argument for allowing them now instead of waiting until Go 1.6
is that this way RawPath has one fixed meaning at the time it is
introduced, that we should not need to change or expand.

Fixes #5684.

Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c
Reviewed-on: https://go-review.googlesource.com/13258Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      fced03a5
    • Russ Cox's avatar
      net/url: do not percent-encode valid host characters · e8be9a17
      Russ Cox authored 
      The code in question was added as part of allowing zone identifiers
in IPv6 literals like http://[ipv6%zone]:port/foo, in golang.org/cl/2431.

The old condition makes no sense. It refers to §3.2.1, which is the wrong section
of the RFC, it excludes all the sub-delims, which §3.2.2 (the right section)
makes clear are valid, and it allows ':', which is not actually valid,
without an explanation as to why (because we keep :port in the Host field
of the URL struct).

The new condition allows all the sub-delims, as specified in RFC 3986,
plus the additional characters [ ] : seen in IP address literals and :port suffixes,
which we also keep in the Host field.

This allows mysql://a,b,c/path to continue to parse, as it did in Go 1.4 and earlier.

This CL does not break any existing tests, suggesting the over-conservative
behavior was not intended and perhaps not realized.

It is especially important not to over-escape the host field, because
Go does not unescape the host field during parsing: it rejects any
host field containing % characters.

Fixes #12036.

Change-Id: Iccbe4985957b3dc58b6dfb5dcb5b63a51a6feefb
Reviewed-on: https://go-review.googlesource.com/13254Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
Reviewed-by: default avatarMikio Hara <mikioh.mikioh@gmail.com>
      e8be9a17
    • Mikio Hara's avatar
      doc/go1.5.html: fix typo · 91e3b351
      Mikio Hara authored 
      Change-Id: Ic61fd38e7d2e0821c6adcaa210199a7dae8849a7
Reviewed-on: https://go-review.googlesource.com/13281Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      91e3b351
    • Russ Cox's avatar
      net/url: restrict :port checking to [ipv6]:port form · fc22331c
      Russ Cox authored 
      Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.

The CL during Go 1.5 that broke this syntax was
trying to fix #11208 and was probably too aggressive.
I added a test case for #11208 to make sure that stays
fixed.

Relaxing the check did not re-break #11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.

Fixes #12023.

Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
Reviewed-by: default avatarMikio Hara <mikioh.mikioh@gmail.com>
      fc22331c
  5. 05 Aug, 2015 22 commits
  7. 04 Aug, 2015 8 commits
    • Robert Griesemer's avatar
      reflect: fix doc string · 3cfc34a5
      Robert Griesemer authored 
      Fixes #12017.

Change-Id: I3dfcf9d0b62cae02eca1973383f0aad286a6ef4d
Reviewed-on: https://go-review.googlesource.com/13136Reviewed-by: default avatarKeith Randall <khr@golang.org>
      3cfc34a5
    • Austin Clements's avatar
      runtime: fix typos in comments · be39a429
      Austin Clements authored 
      Change-Id: I66f7937b22bb6e05c3f2f0f2a057151020ad9699
Reviewed-on: https://go-review.googlesource.com/13049Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      be39a429
    • Austin Clements's avatar
      runtime: fix assist utilization computation · e3870aa6
      Austin Clements authored 
      When commit 510fd135 enabled assists during the scan phase, it failed
to also update the code in the GC controller that computed the assist
CPU utilization and adjusted the trigger based on it. Fix that code so
it uses the start of the scan phase as the wall-clock time when
assists were enabled rather than the start of the mark phase.

Change-Id: I05013734b4448c3e2c730dc7b0b5ee28c86ed8cf
Reviewed-on: https://go-review.googlesource.com/13048Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      e3870aa6
    • Austin Clements's avatar
      runtime: revise assist ratio aggressively · 1fb01a88
      Austin Clements authored 
      At the start of a GC cycle, the garbage collector computes the assist
ratio based on the total scannable heap size. This was intended to be
conservative; after all, this assumes the entire heap may be reachable
and hence needs to be scanned. But it only assumes that the *current*
entire heap may be reachable. It fails to account for heap allocated
during the GC cycle. If the trigger ratio is very low (near zero), and
most of the heap is reachable when GC starts (which is likely if the
trigger ratio is near zero), then it's possible for the mutator to
create new, reachable heap fast enough that the assists won't keep up
based on the assist ratio computed at the beginning of the cycle. As a
result, the heap can grow beyond the heap goal (by hundreds of megs in
stress tests like in issue #11911).

We already have some vestigial logic for dealing with situations like
this; it just doesn't run often enough. Currently, every 10 ms during
the GC cycle, the GC revises the assist ratio. This was put in before
we switched to a conservative assist ratio (when we really were using
estimates of scannable heap), and it turns out to be exactly what we
need now. However, every 10 ms is far too infrequent for a rapidly
allocating mutator.

This commit reuses this logic, but replaces the 10 ms timer with
revising the assist ratio every time the heap is locked, which
coincides precisely with when the statistics used to compute the
assist ratio are updated.

Fixes #11911.

Change-Id: I377b231ab064946228378fa10422a46d1b50f4c5
Reviewed-on: https://go-review.googlesource.com/13047Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      1fb01a88
    • Austin Clements's avatar
      runtime: when gcpacertrace > 0, print information about assist ratio · f9dc3382
      Austin Clements authored 
      This was useful in debugging the mutator assist behavior for #11911,
and it fits with the other gcpacertrace output.

Change-Id: I1e25590bb4098223a160de796578bd11086309c7
Reviewed-on: https://go-review.googlesource.com/13046Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      f9dc3382
    • Austin Clements's avatar
      runtime: make sweep proportional to spans bytes allocated · fc9ca85f
      Austin Clements authored 
      Proportional concurrent sweep is currently based on a ratio of spans
to be swept per bytes of object allocation. However, proportional
sweeping is performed during span allocation, not object allocation,
in order to minimize contention and overhead. Since objects are
allocated from spans after those spans are allocated, the system tends
to operate in debt, which means when the next GC cycle starts, there
is often sweep debt remaining, so GC has to finish the sweep, which
delays the start of the cycle and delays enabling mutator assists.

For example, it's quite likely that many Ps will simultaneously refill
their span caches immediately after a GC cycle (because GC flushes the
span caches), but at this point, there has been very little object
allocation since the end of GC, so very little sweeping is done. The
Ps then allocate objects from these cached spans, which drives up the
bytes of object allocation, but since these allocations are coming
from cached spans, nothing considers whether more sweeping has to
happen. If the sweep ratio is high enough (which can happen if the
next GC trigger is very close to the retained heap size), this can
easily represent a sweep debt of thousands of pages.

Fix this by making proportional sweep proportional to the number of
bytes of spans allocated, rather than the number of bytes of objects
allocated. Prior to allocating a span, both the small object path and
the large object path ensure credit for allocating that span, so the
system operates in the black, rather than in the red.

Combined with the previous commit, this should eliminate all sweeping
from GC start up. On the stress test in issue #11911, this reduces the
time spent sweeping during GC (and delaying start up) by several
orders of magnitude:

                mean    99%ile     max
    pre fix      1 ms    11 ms   144 ms
    post fix   270 ns   735 ns   916 ns

Updates #11911.

Change-Id: I89223712883954c9d6ec2a7a51ecb97172097df3
Reviewed-on: https://go-review.googlesource.com/13044Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      fc9ca85f
    • Austin Clements's avatar
      runtime: always give concurrent sweep some heap distance · e30c6d64
      Austin Clements authored 
      Currently it's possible for the next_gc heap size trigger computed for
the next GC cycle to be less than the current allocated heap size.
This means the next cycle will start immediately, which means there's
no time to perform the concurrent sweep between GC cycles. This places
responsibility for finishing the sweep on GC itself, which delays GC
start-up and hence delays mutator assist.

Fix this by ensuring that next_gc is always at least a little higher
than the allocated heap size, so we won't trigger the next cycle
instantly.

Updates #11911.

Change-Id: I74f0b887bf187518d5fedffc7989817cbcf30592
Reviewed-on: https://go-review.googlesource.com/13043Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      e30c6d64
    • Austin Clements's avatar
      runtime: assist the GC during GC startup and shutdown · fb5230af
      Austin Clements authored 
      Currently there are two sensitive periods during which a mutator can
allocate past the heap goal but mutator assists can't be enabled: 1)
at the beginning of GC between when the heap first passes the heap
trigger and sweep termination and 2) at the end of GC between mark
termination and when the background GC goroutine parks. During these
periods there's no back-pressure or safety net, so a rapidly
allocating mutator can allocate past the heap goal. This is
exacerbated if there are many goroutines because the GC coordinator is
scheduled as any other goroutine, so if it gets preempted during one
of these periods, it may stay preempted for a long period (10s or 100s
of milliseconds).

Normally the mutator does scan work to create back-pressure against
allocation, but there is no scan work during these periods. Hence, as
a fall back, if a mutator would assist but can't yet, simply yield the
CPU. This delays the mutator somewhat, but more importantly gives more
CPU time to the GC coordinator for it to complete the transition.

This is obviously a workaround. Issue #11970 suggests a far better but
far more invasive way to fix this.

Updates #11911. (This very nearly fixes the issue, but about once
every 15 minutes I get a GC cycle where the assists are enabled but
don't do enough work.)

Change-Id: I9768b79e3778abd3e06d306596c3bd77f65bf3f1
Reviewed-on: https://go-review.googlesource.com/13026Reviewed-by: default avatarRuss Cox <rsc@golang.org>
Reviewed-by: default avatarRick Hudson <rlh@golang.org>
      fb5230af
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7