A future change to gofmt will rewrite
// Doc comment.
//go:foo
to
// Doc comment.
//
//go:foo
Apply that change preemptively to all comments (not necessarily just doc comments).
For #51082.
Change-Id: Iffe0285418d1e79d34526af3520b415a12203ca9
Reviewed-on: https://go-review.googlesource.com/c/go/+/384260
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Now that Go 1.18 has been released, remove the old pacer.
Change-Id: Ie7a7596d67f3fc25d3f375a08fc75eafac2eb834
Reviewed-on: https://go-review.googlesource.com/c/go/+/393396
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
If something goes horribly wrong with the assumptions surrounding a
piController, its internal error state might accumulate in an unbounded
manner. In practice this means unexpected Inf and NaN values.
Avoid this by identifying cases where the error overflows and resetting
controller state.
In the scavenger, this case is much more likely. All that has to happen
is the proportional relationship between sleep time and estimated CPU
usage has to break down. Unfortunately because we're just measuring
monotonic time for all this, there are lots of ways it could happen,
especially in an oversubscribed system. In these cases, just fall back
on a conservative pace for scavenging and try to wait out the issue.
In the pacer I'm pretty sure this is impossible. Because we wire the
output of the controller to the input, the response is very directly
correlated, so it's impossible for the controller's core assumption to
break down.
While we're in the pacer, add more detail about why that controller is
even there, as well as its purpose.
Finally, let's be proactive about other sources of overflow, namely
overflow from a very large input value. This change adds a check after
the first few operations to detect overflow issues from the input,
specifically the multiplication.
No tests for the pacer because I was unable to actually break the
pacer's controller under a fuzzer, and no tests for the scavenger because
it is not really in a testable state.
However:
* This change includes a fuzz test for the piController.
* I broke out the scavenger code locally and fuzz tested it, confirming
that the patch eliminates the original failure mode.
* I tested that on a local heap-spike test, the scavenger continues
operating as expected under normal conditions.
Fixes#51061.
Change-Id: I02a01d2dbf0eb9d2a8a8e7274d4165c2b6a3415a
Reviewed-on: https://go-review.googlesource.com/c/go/+/383954
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There was an off-by-one error in the time histogram buckets calculation
that caused the linear sub-buckets distances to be off by 2x.
The fix was trivial, but in writing tests I realized there was a much
simpler way to express the calculation for the histogram buckets, and
took the opportunity to do that here. The new bucket calculation also
fixes the bug.
Fixes#50732.
Change-Id: Idae89986de1c415ee4e148f778e0e101ca003ade
Reviewed-on: https://go-review.googlesource.com/c/go/+/380094
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TestFutexsleep was originally created in CL 7876043 as a
regression test for buggy division logic in futexsleep. Several months
later CL 11575044 moved this logic to timediv (called by futexsleep).
This test calls runtime.Futexsleep, which temporarily disables
asynchronous preemption. Unfortunately, TestFutexSleep calls this from
multiple goroutines, creating a race condition that may result in
asynchronous preemption remaining disabled for the remainder of the
process lifetime.
We could fix this by moving the async preemption disable to the main
test function, however this test has had a history of flakiness. As an
alternative, this CL replaces the test wholesale with a new test for
timediv, covering the overflow logic without the difficulty of dealing
with futex.
Fixes#50749.
Change-Id: If9e1dac63ef1535adb49f9a9ffcaff99b9135895
Reviewed-on: https://go-review.googlesource.com/c/go/+/380058
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes#49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
In iOS <14, the address space is strictly limited to 8 GiB, or 33 bits.
As a result, the page allocator also assumes all heap memory lives in
this region. This is especially necessary because the page allocator has
a PROT_NONE mapping proportional to the size of the usable address
space, so this keeps that mapping very small.
However starting with iOS 14, this restriction is relaxed, and mmap may
start returning addresses outside of the <14 range. Today this means
that in iOS 14 and later, users experience an error in the page
allocator when a heap arena is mapped outside of the old range.
This change increases the ios/arm64 heapAddrBits to 40 while
simultaneously making ios/arm64 use the 64-bit pagealloc implementation
(with reservations and incremental mapping) to accommodate both iOS
versions <14 and 14+.
Once iOS <14 is deprecated, we can remove these exceptions and treat
ios/arm64 like any other arm64 platform.
This change also makes the BaseChunkIdx expression a little bit easier
to read, while we're here.
Fixes#46860.
Change-Id: I13865f799777739109585f14f1cc49d6d57e096b
Reviewed-on: https://go-review.googlesource.com/c/go/+/344401
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TestTinyAllocIssue37262 assumes that all of its allocations will come
from the same tiny allocator (that is, the same P), and that nothing
else will allocate from that tiny allocator while it's running. It can
fail incorrectly if these assumptions aren't met.
Fix this potential test flakiness by disabling preemption during this
test.
As far as I know, this has never happened on the builders. It was
found by mayMoreStackPreempt.
Change-Id: I59f993e0bdbf46a9add842d0e278415422c3f804
Reviewed-on: https://go-review.googlesource.com/c/go/+/366994
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This change modifies the scavenger to no longer hold the heap lock while
actively scavenging pages. To achieve this, the change also:
* Reverses the locking behavior of the (*pageAlloc).scavenge API, to
only acquire the heap lock when necessary.
* Introduces a new lock on the scavenger-related fields in a pageAlloc
so that access to those fields doesn't require the heap lock. There
are a few places in the scavenge path, notably reservation, that
requires synchronization. The heap lock is far too heavy handed for
this case.
* Changes the scavenger to marks pages that are actively being scavenged
as allocated, and "frees" them back to the page allocator the usual
way.
* Lifts the heap-growth scavenging code out of mheap.grow, where the
heap lock is held, and into allocSpan, just after the lock is
released. Releasing the lock during mheap.grow is not feasible if we
want to ensure that allocation always makes progress (post-growth,
another allocator could come in and take all that space, forcing the
goroutine that just grew the heap to do so again).
This change means that the scavenger now must do more work for each
scavenge, but it is also now much more scalable. Although in theory it's
not great by always taking the locked paths in the page allocator, it
takes advantage of some properties of the allocator:
* Most of the time, the scavenger will be working with one page at a
time. The page allocator's locked path is optimized for this case.
* On the allocation path, it doesn't need to do the find operation at
all; it can go straight to setting bits for the range and updating the
summary structure.
Change-Id: Ie941d5e7c05dcc96476795c63fef74bcafc2a0f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/353974
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change implements the GC pacer redesign outlined in #44167 and the
accompanying design document, behind a GOEXPERIMENT flag that is on by
default.
In addition to adding the new pacer, this CL also includes code to track
and account for stack and globals scan work in the pacer and in the
assist credit system.
The new pacer also deviates slightly from the document in that it
increases the bound on the minimum trigger ratio from 0.6 (scaled by
GOGC) to 0.7. The logic behind this change is that the new pacer much
more consistently hits the goal (good!) leading to slightly less
frequent GC cycles, but _longer_ ones (in this case, bad!). It turns out
that the cost of having the GC on hurts throughput significantly (per
byte of memory used), though tail latencies can improve by up to 10%! To
be conservative, this change moves the value to 0.7 where there is a
small improvement to both throughput and latency, given the memory use.
Because the new pacer accounts for the two most significant sources of
scan work after heap objects, it is now also safer to reduce the minimum
heap size without leading to very poor amortization. This change thus
decreases the minimum heap size to 512 KiB, which corresponds to the
fact that the runtime has around 200 KiB of scannable globals always
there, up-front, providing a baseline.
Benchmark results: https://perf.golang.org/search?q=upload:20211001.6
tile38's KNearest benchmark shows a memory increase, but throughput (and
latency) per byte of memory used is better.
gopher-lua showed an increase in both CPU time and memory usage, but
subsequent attempts to reproduce this behavior are inconsistent.
Sometimes the overall performance is better, sometimes it's worse. This
suggests that the benchmark is fairly noisy in a way not captured by the
benchmarking framework itself.
biogo-igor is the only benchmark to show a significant performance loss.
This benchmark exhibits a very high GC rate, with relatively little work
to do in each cycle. The idle mark workers are quite active. In the new
pacer, mark phases are longer, mark assists are fewer, and some of that
time in mark assists has shifted to idle workers. Linux perf indicates
that the difference in CPU time can be mostly attributed to write-barrier
slow path related calls, which in turn indicates that the write barrier
being on for longer is the primary culprit. This also explains the memory
increase, as a longer mark phase leads to more memory allocated black,
surviving an extra cycle and contributing to the heap goal.
For #44167.
Change-Id: I8ac7cfef7d593e4a642c9b2be43fb3591a8ec9c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/309869
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change creates a formal exported interface for the GC pacer and
creates tests for it that simulate some series of GC cycles. The tests
are completely driven by the real pacer implementation, except for
assists, which are idealized (though revise is called repeatedly).
For #44167.
Change-Id: I0112242b07e7702595ca71001d781ad6c1fddd2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/353354
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
[git-generate]
cd src/runtime
mv export_test.go export.go
GOROOT=$(dirname $(dirname $PWD)) rf '
add mheap.pagesInUse \
// Proportional sweep \
// \
// These parameters represent a linear function from gcController.heapLive \
// to page sweep count. The proportional sweep system works to \
// stay in the black by keeping the current page sweep count \
// above this line at the current gcController.heapLive. \
// \
// The line has slope sweepPagesPerByte and passes through a \
// basis point at (sweepHeapLiveBasis, pagesSweptBasis). At \
// any given time, the system is at (gcController.heapLive, \
// pagesSwept) in this space. \
// \
// It is important that the line pass through a point we \
// control rather than simply starting at a 0,0 origin \
// because that lets us adjust sweep pacing at any time while \
// accounting for current progress. If we could only adjust \
// the slope, it would create a discontinuity in debt if any \
// progress has already been made. \
pagesInUse_ atomic.Uint64 // pages of spans in stats mSpanInUse
ex {
import "runtime/internal/atomic"
var t mheap
var v, w uint64
var d int64
t.pagesInUse -> t.pagesInUse_.Load()
t.pagesInUse = v -> t.pagesInUse_.Store(v)
atomic.Load64(&t.pagesInUse) -> t.pagesInUse_.Load()
atomic.LoadAcq64(&t.pagesInUse) -> t.pagesInUse_.LoadAcquire()
atomic.Store64(&t.pagesInUse, v) -> t.pagesInUse_.Store(v)
atomic.StoreRel64(&t.pagesInUse, v) -> t.pagesInUse_.StoreRelease(v)
atomic.Cas64(&t.pagesInUse, v, w) -> t.pagesInUse_.CompareAndSwap(v, w)
atomic.Xchg64(&t.pagesInUse, v) -> t.pagesInUse_.Swap(v)
atomic.Xadd64(&t.pagesInUse, d) -> t.pagesInUse_.Add(d)
}
rm mheap.pagesInUse
mv mheap.pagesInUse_ mheap.pagesInUse
'
mv export.go export_test.go
Change-Id: I495d188683dba0778518563c46755b5ad43be298
Reviewed-on: https://go-review.googlesource.com/c/go/+/356549
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Change-Id: I1209904326b1563e12d9c7d19a12a10c72d1dbcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/329191
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
In CL 298669 we added defer/go wrapping, and, as it is not
allowed for closures to escape when compiling runtime, we worked
around it by rewriting go'd closures to argumentless
non-capturing closures, so it is not a real closure and so not
needed to escape.
Previous CL removes the restriction. Now we can undo the
workaround.
Updates #40724.
Change-Id: Ic7bf129da4aee7b7fdb7157414eca943a6a27264
Reviewed-on: https://go-review.googlesource.com/c/go/+/325110
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Merge List:
+ 2021-05-25 f22ec51deb doc: add Go 1.17 release note about inlining functions with closures
+ 2021-05-25 8b462d7567 cmd/go: add a -compat flag to 'go mod tidy'
+ 2021-05-24 c89f1224a5 net: verify results from Lookup* are valid domain names
+ 2021-05-24 08a8fa9c47 misc/wasm: ensure correct stack pointer in catch clauses
+ 2021-05-24 32b73ae180 cmd/go: align checks of module path during initialization.
+ 2021-05-24 15d9d4a009 cmd/go: add tests illustrating what happens when Go 1.16 is used in a Go 1.17 main module
+ 2021-05-24 873401df5b cmd/compile: ensure equal functions don't do unaligned loads
+ 2021-05-24 b83610699a cmd/compile: record regabi status in DW_AT_producer
+ 2021-05-24 a22e317220 cmd/compile: always include underlying type for map types
+ 2021-05-24 4356e7e85f runtime: account for spill slots in Windows callback compilation
+ 2021-05-24 52d7033ff6 cmd/go/internal/modload: set the default GoVersion in a single location
+ 2021-05-24 05819bc104 cmd/go/internal/modcmd: factor out a type for flags whose arguments are Go versions
+ 2021-05-22 cca23a7373 cmd/compile: revert CL/316890
+ 2021-05-21 f87194cbd7 doc/go1.17: document changes to net/http package
+ 2021-05-21 217f5dd496 doc: document additional atomic.Value methods
+ 2021-05-21 3c656445f1 cmd/go: in TestScript/mod_replace, download an explicit module path
+ 2021-05-21 76b2d6afed os: document that StartProcess puts files into blocking mode
+ 2021-05-21 e4d7525c3e cmd/dist: display first class port status in json output
+ 2021-05-21 4fb10b2118 cmd/go: in 'go mod download' without args, don't save module zip sums
+ 2021-05-21 4fda54ce3f doc/go1.17: document database/sql changes for Go 1.17
+ 2021-05-21 8876b9bd6a doc/go1.17: document io/fs changes for Go 1.17
+ 2021-05-21 5fee772c87 doc/go1.17: document archive/zip changes for Go 1.17
+ 2021-05-21 3148694f60 cmd/go: remove warning from module deprecation notice printing
+ 2021-05-21 7e63c8b765 runtime: wait for Go runtime to initialize in Windows signal test
+ 2021-05-21 831573cd21 io/fs: added an example for io/fs.WalkDir
+ 2021-05-20 baa934d26d cmd: go get golang.org/x/tools/analysis@49064d23 && go mod vendor
+ 2021-05-20 7c692cc7ea doc/go1.17: document changes to os package
+ 2021-05-20 ce9a3b79d5 crypto/x509: add new FreeBSD 12.2+ trusted certificate folder
+ 2021-05-20 f8be906d74 test: re-enable test on riscv64 now that it supports external linking
+ 2021-05-20 def5360541 doc/go1.17: add release notes for OpenBSD ports
+ 2021-05-20 ef1f52cc38 doc/go1.17: add release note for windows/arm64 port
+ 2021-05-20 bb7495a46d doc/go1.17: document new math constants
+ 2021-05-20 f07e4dae3c syscall: document NewCallback and NewCallbackCDecl limitations
+ 2021-05-20 a8d85918b6 misc/cgo/testplugin: skip TestIssue25756pie on darwin/arm64 builder
+ 2021-05-19 6c1c055d1e cmd/internal/moddeps: use filepath.SkipDir only on directories
+ 2021-05-19 658b5e66ec net: return nil UDPAddr from ReadFromUDP
+ 2021-05-19 15a374d5c1 test: check portable error message on issue46234.go
+ 2021-05-18 eeadce2d87 go/build/constraint: fix parsing of "// +build" (with no args)
+ 2021-05-18 6d2ef2ef2a cmd/compile: don't emit inltree for closure within body of inlined func
+ 2021-05-18 048cb4ceee crypto/x509: remove duplicate import
Change-Id: Ib0442e3555493805f2aa1df26dfd6898df989a37
This is a different fix for #37716.
Should help make the fix for #46283 easier, since we will no longer
need to keep compiler-generated hash functions and the runtime
hash function in sync.
Change-Id: I84cb93144e425dcd03afc552b5fbd0f2d2cc6d39
Reviewed-on: https://go-review.googlesource.com/c/go/+/322150
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
At this point all funcPC references are ABIInternal functions.
Replace with the intrinsics.
Change-Id: I3ba7e485c83017408749b53f92877d3727a75e27
Reviewed-on: https://go-review.googlesource.com/c/go/+/321954
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Top align allocations in tinyalloc buckets when in race mode.
This will make checkptr checks more reliable, because any code
that modifies a pointer past the end of the object will trigger
a checkptr error.
No test, because we need -race for this to actually kick in. We could
add it to the race detector tests, but the race detector tests are all
geared towards race detector reports, not checkptr reports. Mucking
with parsing reports is more than a test is worth.
Fixes#38872
Change-Id: Ie56f0fbd1a9385539f6631fd1ac40c3de5600154
Reviewed-on: https://go-review.googlesource.com/c/go/+/315029
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently tiny allocations are not represented in either MemStats or
runtime/metrics, but they're represented in MemStats (indirectly) via
Mallocs. Add them to runtime/metrics by first merging
memstats.tinyallocs into consistentHeapStats (just for simplicity; it's
monotonic so metrics would still be self-consistent if we just read it
atomically) and then adding /gc/heap/tiny/allocs:objects to the list of
supported metrics.
Change-Id: Ie478006ab942a3e877b4a79065ffa43569722f3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/312909
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change modifies the system that allows Go functions to be set as
callbacks in various Windows systems to support the new register ABI.
For #40724.
Change-Id: Ie067f9e8a76c96d56177d7aa88f89cbe7223e12e
Reviewed-on: https://go-review.googlesource.com/c/go/+/300113
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This CL adds a set of helper functions for testing GC interactions.
These are intended for use in the regabi signature fuzzer, but are
generally useful for GC tests, so we make them generally available to
runtime tests.
These provide:
1. An easy way to force stack movement, for testing stack copying.
2. A simple and robust way to check the reachability of a set of
pointers.
3. A way to check what general category of memory a pointer points to,
mostly so tests can make sure they're testing what they mean to.
For #40724, but generally useful.
Change-Id: I15d33ccb3f5a792c0472a19c2cc9a8b4a9356a66
Reviewed-on: https://go-review.googlesource.com/c/go/+/305330
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Adds code to the compiler's "order" phase to rewrite go and defer
statements to always be argument-less. E.g.
defer f(x,y) => x1, y1 := x, y
defer func() { f(x1, y1) }
This transformation is not beneficial on its own, but it helps
simplify runtime defer handling for the new register ABI (when
invoking deferred functions on the panic path, the runtime doesn't
need to manage the complexity of determining which args to pass in
register vs memory).
This feature is currently enabled by default if GOEXPERIMENT=regabi or
GOEXPERIMENT=regabidefer is in effect.
Included in this CL are some workarounds in the runtime to insure that
"go" statement targets in the runtime are argument-less already (since
wrapping them can potentially introduce heap-allocated closures, which
are currently not allowed). The expectation is that these workarounds
will be temporary, and can go away once we either A) change the rules
about heap-allocated closures, or B) implement some other scheme for
handling go statements.
Change-Id: I01060d79a6b140c6f0838d6e6813f807ccdca319
Reviewed-on: https://go-review.googlesource.com/c/go/+/298669
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
This change moves the call of sysMap from (*mheap).sysAlloc into
(*mheap).grow, so we only sysMap what we're going to use in the near
future (thanks to the curArena mechanism). The purpose of this change is
to better support systems with strict overcommit rules which generally
accept reserved memory but not prepared memory (see malloc.go for exact
descriptions of these states).
This move requires changing linearAlloc to only optionally map memory.
In one case, with mheap.heapArenaAlloc, we do want it to map memory. But
now in the other case, with mheap.arena, we don't, because we want grow
to take care of it.
The risk with this change is we may make more syscalls than before on
systems with 64 MiB arenas, but because heap growth is relatively rare
this is unlikely to be a noticable issue. We also bound the amount of
syscalls made by only extending curArena (and thus mapping) by
pallocChunkPages*pageSize which is 4 MiB.
Fixes#42612.
Change-Id: I736df696afe78ddb1a747a896caa0db8726027e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/270537
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change modifies runfinq to properly pass arguments to finalizers in
registers via reflectcall.
For #40724.
Change-Id: I414c0eff466ef315a0eb10507994e598dd29ccb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/300112
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
To ease readability we typically keep the partial order lists sorted by
rank. This isn't required for correctness, it just makes it (slightly)
easier to read the lists.
Currently we must notice out-of-order entries during code review, which
is an error-prone process.
Add a test to enforce ordering, and fix the errors that have crept in.
Most of the existing errors were misordered lockRankHchan or
lockRankPollDesc.
While we're here, I've moved the correctness check that the partial
ordering satisfies the total ordering from init to a test case. This
will allow us to catch these errors without even running
staticlockranking.
Change-Id: I9c11abe49ea26c556439822bb6a3183129600c3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/300171
Trust: Michael Pratt <mpratt@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Document what the values in internal/sys mean.
Remove various special cases for arm64 in the code using StackAlign.
Delete Uintreg - it was for GOARCH=amd64p32,
which was specific to GOOS=nacl and has been retired.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I40e8fa07b4e192298b6536b98a72a751951a4383
Reviewed-on: https://go-review.googlesource.com/c/go/+/288795
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Today, timeHistogram, when copied, has the wrong set of counts for the
bucket that should represent (-inf, 0), when in fact it contains [0, 1).
In essence, the buckets are all shifted over by one from where they're
supposed to be.
But this also means that the existence of the overflow bucket is wrong:
the top bucket is supposed to extend to infinity, and what we're really
missing is an underflow bucket to represent the range (-inf, 0).
We could just always zero this bucket and continue ignoring negative
durations, but that likely isn't prudent.
timeHistogram is intended to be used with differences in nanotime, but
depending on how a platform is implemented (or due to a bug in that
platform) it's possible to get a negative duration without having done
anything wrong. We should just be resilient to that and be able to
detect it.
So this change removes the overflow bucket and replaces it with an
underflow bucket, and timeHistogram no longer panics when faced with a
negative duration.
Fixes#43328.
Fixes#43329.
Change-Id: If336425d7d080fd37bf071e18746800e22d38108
Reviewed-on: https://go-review.googlesource.com/c/go/+/279468
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Some functions that required holding the heap lock _or_ world stop have
been simplified to simply requiring the heap lock. This is conceptually
simpler and taking the heap lock during world stop is guaranteed to not
contend. This was only done on functions already called on the
systemstack to avoid too many extra systemstack calls in GC.
Updates #40677
Change-Id: I15aa1dadcdd1a81aac3d2a9ecad6e7d0377befdc
Reviewed-on: https://go-review.googlesource.com/c/go/+/250262
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Michael Pratt <mpratt@google.com>
This change adds a concurrent HDR time histogram to the runtime with
tests. It also adds a function to generate boundaries for use by the
metrics package.
For #37112.
Change-Id: Ifbef8ddce8e3a965a0dcd58ccd4915c282ae2098
Reviewed-on: https://go-review.googlesource.com/c/go/+/247046
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change adds support for a variety of runtime memory metrics and
contains the base implementation of Read for the runtime/metrics
package, which lives in the runtime.
It also adds testing infrastructure for the metrics package, and a bunch
of format and documentation tests.
For #37112.
Change-Id: I16a2c4781eeeb2de0abcb045c15105f1210e2d8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/247041
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
This change moves the mcache-local malloc stats into the
consistentHeapStats structure so the malloc stats can be managed
consistently with the memory stats. The one exception here is
tinyAllocs for which moving that into the global stats would incur
several atomic writes on the fast path. Microbenchmarks for just one CPU
core have shown a 50% loss in throughput. Since tiny allocation counnt
isn't exposed anyway and is always blindly added to both allocs and
frees, let that stay inconsistent and flush the tiny allocation count
every so often.
Change-Id: I2a4b75f209c0e659b9c0db081a3287bf227c10ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/247039
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change modifies the type of several mstats fields to be a new type:
sysMemStat. This type has the same structure as the fields used to have.
The purpose of this change is to make it very clear which stats may be
used in various functions for accounting (usually the platform-specific
sys* functions, but there are others). Currently there's an implicit
understanding that the *uint64 value passed to these functions is some
kind of statistic whose value is atomically managed. This understanding
isn't inherently problematic, but we're about to change how some stats
(which currently use mSysStatInc and mSysStatDec) work, so we want to
make it very clear what the various requirements are around "sysStat".
This change also removes mSysStatInc and mSysStatDec in favor of a
method on sysMemStat. Note that those two functions were originally
written the way they were because atomic 64-bit adds required a valid G
on ARM, but this hasn't been the case for a very long time (since
golang.org/cl/14204, but even before then it wasn't clear if mutexes
required a valid G anymore). Today we implement 64-bit adds on ARM with
a spinlock table.
Change-Id: I4e9b37cf14afc2ae20cf736e874eb0064af086d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/246971
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change renames a bunch of malloc statistics stored in the mcache
that are all named with the "local_" prefix. It also renames largeAlloc
to allocLarge to prevent a naming conflict, and next_sample because it
would be the last mcache field with the old C naming style.
Change-Id: I29695cb83b397a435ede7e9ad5c3c9be72767ea3
Reviewed-on: https://go-review.googlesource.com/c/go/+/246969
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change makes local_tinyallocs work like the rest of the malloc
stats and doesn't flush local_tinyallocs, instead making that the
source-of-truth.
Change-Id: I3e6cb5f1b3d086e432ce7d456895511a48e3617a
Reviewed-on: https://go-review.googlesource.com/c/go/+/246967
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change makes it so that various local malloc stats (excluding
heap_scan and local_tinyallocs) are no longer written first to mheap
fields but are instead accessed directly from each mcache.
This change is part of a move toward having stats be distributed, and
cleaning up some old code related to the stats.
Note that because there's no central source-of-truth, when an mcache
dies, it must donate its stats to another mcache. It's always safe to
donate to the mcache for the 0th P, so do that.
Change-Id: I2556093dbc27357cb9621c9b97671f3c00aa1173
Reviewed-on: https://go-review.googlesource.com/c/go/+/246964
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change adds a test suite for addrRanges.findSucc so we can change
the implementation more safely.
For #40191.
Change-Id: I14a834b6d54836cbc676eb0edb292ba6176705cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/242678
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Currently the AddrRange used for testing is defined separately from
addrRange in the runtime, making it difficult to test it as well as
addrRanges. Redefine AddrRange in terms of addrRange instead.
For #40191.
Change-Id: I3aa5b8df3e4c9a3c494b46ab802dd574b2488141
Reviewed-on: https://go-review.googlesource.com/c/go/+/242677
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
I just submitted CL 255297 which mostly fixed this problem, but totally
forgot to actually acquire/release the heap lock. Oops.
Updates #41391.
Change-Id: I45b42f20a9fc765c4de52476db3654d4bfe9feb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/255298
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
CL 249917 made the mspan in MSpanCountAlloc no longer stack-allocated
(for good reason), but then allocated an mspan on each call and did not
free it, resulting in a leak. That allocation was also not protected by
the heap lock, which could lead to data corruption of mheap fields and
the spanalloc.
To fix this, export some functions to allocate/free dummy mspans from
spanalloc (with proper locking) and allocate just one up-front for the
benchmark, freeing it at the end. Then, update MSpanCountAlloc to accept
a dummy mspan.
Note that we need to allocate the dummy mspan up-front otherwise we
measure things like heap locking and fixalloc performance instead of
what we actually want to measure: how fast we can do a popcount on the
mark bits.
Fixes#41391.
Change-Id: If6629a6ec1ece639c7fb78532045837a8c872c04
Reviewed-on: https://go-review.googlesource.com/c/go/+/255297
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Both ReadMemStatsSlow and CheckScavengedBits iterate over the page
allocator's chunks but don't actually check if they exist. During the
development process the chunks index became sparse, so now this was a
possibility. If the runtime tests' heap is sparse we might end up
segfaulting in either one of these functions, though this will generally
be very rare.
The pattern here to return nil for a nonexistent chunk is also useful
elsewhere, so this change introduces tryChunkOf which won't throw, but
might return nil. It also updates the documentation of chunkOf.
Fixes#41296.
Change-Id: Id5ae0ca3234480de1724fdf2e3677eeedcf76fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/253777
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Right now we just prevent such types from being on the heap. This CL
makes it so they cannot appear on the stack either. The distinction
between heap and stack is pretty vague at the language level (e.g. it
is affected by -N), and we don't need the flexibility anyway.
Once go:notinheap types cannot be in either place, we don't need to
consider pointers to such types to be pointers, at least according to
the garbage collector and stack copying. (This is the big win of this
CL, in my opinion.)
The distinction between HasPointers and HasHeapPointer no longer
exists. There is only HasPointers.
This CL is cleanup before possible use of go:notinheap to fix#40954.
Update #13386
Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a
Reviewed-on: https://go-review.googlesource.com/c/go/+/249917
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Go 1.14 included a (rather awful) workaround for a Linux kernel bug
that corrupted vector registers on x86 CPUs during signal delivery
(https://bugzilla.kernel.org/show_bug.cgi?id=205663). This bug was
introduced in Linux 5.2 and fixed in 5.3.15, 5.4.2 and all 5.5 and
later kernels. The fix was also back-ported by major distros. This
workaround was necessary, but had unfortunate downsides, including
causing Go programs to exceed the mlock ulimit in many configurations
(#37436).
We're reasonably confident that by the Go 1.16 release, the number of
systems running affected kernels will be vanishingly small. Hence,
this CL removes this workaround.
This effectively reverts CLs 209597 (version parser), 209899 (mlock
top of signal stack), 210299 (better failure message), 223121 (soft
mlock failure handling), and 244059 (special-case patched Ubuntu
kernels). The one thing we keep is the osArchInit function. It's empty
everywhere now, but is a reasonable hook to have.
Updates #35326, #35777 (the original register corruption bugs).
Updates #40184 (request to revert in 1.15).
Fixes#35979.
Change-Id: Ie213270837095576f1f3ef46bf3de187dc486c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/246200
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently maxOffAddr is defined in terms of the whole 64-bit address
space, assuming that it's all supported, by using ^uintptr(0) as the
maximal address in the offset space. In reality, the maximal address in
the offset space is (1<<heapAddrBits)-1 because we don't have more than
that actually available to us on a given platform.
On most platforms this is fine, because arenaBaseOffset is just
connecting two segments of address space, but on AIX we use it as an
actual offset for the starting address of the available address space,
which is limited. This means using ^uintptr(0) as the maximal address in
the offset address space causes wrap-around, especially when we just
want to represent a range approximately like [addr, infinity), which
today we do by using maxOffAddr.
To fix this, we define maxOffAddr more appropriately, in terms of
(1<<heapAddrBits)-1.
This change also redefines arenaBaseOffset to not be the negation of the
virtual address corresponding to address zero in the virtual address
space, but instead directly as the virtual address corresponding to
zero. This matches the existing documentation more closely and makes the
logic around arenaBaseOffset decidedly simpler, especially when trying
to reason about its use on AIX.
Fixes#38966.
Change-Id: I1336e5036a39de846f64cc2d253e8536dee57611
Reviewed-on: https://go-review.googlesource.com/c/go/+/233497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
