Skip to content

G
go
Switch branch/tag
  1. 22 Nov, 2019 8 commits
    • Michael Anthony Knyszek's avatar
      runtime: release worldsema before Gosched in STW GC mode · 05511a5c
      Michael Anthony Knyszek authored 
      After CL 182657 we no longer hold worldsema across the GC, we hold
gcsema instead.

However in STW GC mode we don't release worldsema before calling Gosched
on the user goroutine (note that user goroutines are disabled during STW
GC) so that user goroutine holds onto it. When the GC is done and the
runtime inevitably wants to "stop the world" again (though there isn't
much to stop) it'll sit there waiting for worldsema which won't be
released until the aforementioned goroutine is scheduled, which it won't
be until the GC is done!

So, we have a deadlock.

The fix is easy: just release worldsema before calling Gosched.

Fixes #34736.

Change-Id: Ia50db22ebed3176114e7e60a7edaf82f8535c1b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/208379
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      05511a5c
    • Michael Anthony Knyszek's avatar
      runtime: increase TestPhysicalMemoryUtilization threshold · 1c5bd345
      Michael Anthony Knyszek authored 
      TestPhysicalMemoryUtilization occasionally fails on some platforms by
only a small margin. The reason for this is that it assumes the
scavenger will always be able to scavenge all the memory that's released
by sweeping, but because of the page cache, there could be free and
unscavenged memory held onto by a P which the scavenger simply cannot
get to.

As a result, if the page cache gets filled completely (512 KiB of free
and unscavenged memory) this could skew a test which expects to
scavenge roughly 8 MiB of memory. More specifically, this is 512 KiB of
memory per P, and if a system is more inclined to bounce around
between Ps (even if there's only one goroutine), this memory can get
"stuck".

Through some experimentation, I found that failures correlated highly
with relatively large amounts of memory ending up in some page cache
(like 60 or 64 pages) on at least one P.

This change changes the test's threshold such that it accounts for the
page cache, and scales up with GOMAXPROCS. Because the test constants
themselves don't change, however, the test must now also bound
GOMAXPROCS such that the threshold doesn't get too high (at which point
the test becomes meaningless).

Fixes #35580.

Change-Id: I6bdb70706de991966a9d28347da830be4a19d3a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/208377
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
      1c5bd345
    • Jay Conrod's avatar
      cmd/go: add 'go generate' commands to modfile_flag test · 300f5d5b
      Jay Conrod authored 
      Verify that 'go generate' works with -modfile. Also check that
go commands starts with 'go generate' do not inherit -modfile, but
they should still work if -modfile is set in GOFLAGS.

Updates #34506

Change-Id: I5e1f897b4e38e4fdaccc0fbb7a71b8d0e9fc0660
Reviewed-on: https://go-review.googlesource.com/c/go/+/208236
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      300f5d5b
    • Bryan C. Mills's avatar
      misc/cgo/testcshared: avoid writing to GOROOT in tests · 3922c006
      Bryan C. Mills authored 
      The tests in this package invoked 'go install -i -buildmode=c-shared'
in order to generate an archive as well as multiple C header files.

Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_testcshared_shared, which may not be writable
— for example, if GOROOT is owned by the root user but the test is
being run by a non-root user.

Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.

In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.

Updates #28387
Updates #30316
Updates #35715

Change-Id: I622426a860828020d98f7040636f374e5c766d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/208119
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      3922c006
    • Bryan C. Mills's avatar
      misc/cgo/testcarchive: avoid writing to GOROOT in tests · c02f3b86
      Bryan C. Mills authored 
      Also add a -testwork flag to facilitate debugging the test itself.

Three of the tests of this package invoked 'go install -i
-buildmode=c-archive' in order to generate an archive as well as
multiple C header files.

Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_shared, which may not be writable — for
example, if GOROOT is owned by the root user but the test is being run
by a non-root user.

Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.

In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.

Updates #28387
Updates #30316
Updates #35715

Change-Id: I3d483f84e22058561efe740aa4885fc3f26137b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/208117
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      c02f3b86
    • Hana Kim's avatar
      runtime/pprof: avoid crash due to truncated stack traces · ea18a1c2
      Hana Kim authored 
      The profile proto message builder maintains a location entry cache
that maps a location (possibly involving multiple user frames
that represent inlined function calls) to the location id. We have
been using the first pc of the inlined call sequence as the key of
the cached location entry assuming that, for a given pc, the sequence
of frames representing the inlined call stack is deterministic and
stable. Then, when analyzing the new stack trace, we expected the
exact number of pcs to be present in the captured stack trace upon
the cache hit.

This assumption does not hold, however, in the presence of the stack
trace truncation in the runtime during profiling, and also with the
potential bugs in runtime.

A better fix is to use all the pcs of the inlined call sequece as
the key instead of the first pc. But that is a bigger code change.
This CL avoids the crash assuming the trace was truncated.

Fixes #35538

Change-Id: I8c6bae98bc8b178ee51523c7316f56b1cce6df16
Reviewed-on: https://go-review.googlesource.com/c/go/+/207609
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
      ea18a1c2
    • two's avatar
      reflect: remove obsolete comment about gobitVector · 95be9b75
      two authored 
      Change-Id: Ie3495a51ac2021a55e7c1ee43a66d07a5bf2757a
GitHub-Last-Rev: b6a6bab3ab840b361021b25cac37eb6891c0fe4b
GitHub-Pull-Request: golang/go#35709
Reviewed-on: https://go-review.googlesource.com/c/go/+/207853Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      95be9b75
    • David Chase's avatar
      cmd/compile: try harder to not use an empty src.XPos for a bogus line · 0e02cfb3
      David Chase authored 
      The fix for #35652 did not guarantee that it was using a non-empty
src position to replace an empty one.  The new code checks again
and falls back to a more certain position.  (The input in question
compiles to a single empty infinite loop, and none of the actual instructions
had any source position at all.  That is a bug, but given the pathology
of this input, not one worth dealing with this late in the release cycle,
if ever.)

Literally:

00000 (5) TEXT "".f(SB), ABIInternal
00001 (5) PCDATA $0, $-2
00002 (5) PCDATA $1, $-2
00003 (5) FUNCDATA $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00004 (5) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00005 (5) FUNCDATA $2, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
b2
00006 (?) XCHGL AX, AX
b6
00007 (+1048575) JMP 6
00008 (?) END

TODO: Add runtime.InfiniteLoop(), replace infinite loops with a call to
that, and use an eco-friendly runtime.gopark instead.  (This was Cherry's
excellent idea.)

Updates #35652
Fixes #35695

Change-Id: I4b9a841142ee4df0f6b10863cfa0721a7e13b437
Reviewed-on: https://go-review.googlesource.com/c/go/+/207964
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      0e02cfb3
  3. 21 Nov, 2019 7 commits
  5. 20 Nov, 2019 10 commits
  7. 19 Nov, 2019 15 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7