In a C thread, it's necessary to acquire an extra M by using needm while invoking a Go function from C. But, needm and dropm are heavy costs due to the signal-related syscalls.
So, we change to not dropm while returning back to C, which means binding the extra M to the C thread until it exits, to avoid needm and dropm on each C to Go call.
Instead, we only dropm while the C thread exits, so the extra M won't leak.
When invoking a Go function from C:
Allocate a pthread variable using pthread_key_create, only once per shared object, and register a thread-exit-time destructor.
And store the g0 of the current m into the thread-specified value of the pthread key, only once per C thread, so that the destructor will put the extra M back onto the extra M list while the C thread exits.
When returning back to C:
Skip dropm in cgocallback, when the pthread variable has been created, so that the extra M will be reused the next time invoke a Go function from C.
This is purely a performance optimization. The old version, in which needm & dropm happen on each cgo call, is still correct too, and we have to keep the old version on systems with cgo but without pthreads, like Windows.
This optimization is significant, and the specific value depends on the OS system and CPU, but in general, it can be considered as 10x faster, for a simple Go function call from a C thread.
For the newly added BenchmarkCGoInCThread, some benchmark results:
1. it's 28x faster, from 3395 ns/op to 121 ns/op, in darwin OS & Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
2. it's 6.5x faster, from 1495 ns/op to 230 ns/op, in Linux OS & Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz
Fixes#51676
Change-Id: I380702fe2f9b6b401b2d6f04b0aba990f4b9ee6c
GitHub-Last-Rev: 93dc64ad98
GitHub-Pull-Request: golang/go#51679
Reviewed-on: https://go-review.googlesource.com/c/go/+/392854
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: thepudds <thepudds1460@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The runtime.etext symbol is a marker symbol that marks the end of
(Go's) text section. Currently it has 0 size on some platforms.
Especially in external linking mode, this may cause the next
symbol (e.g. a C function) to have the same address as
runtime.etext, which may confuse some symbolizer. Add some padding
bytes to avoid address collision.
Change-Id: Ic450bab72e4ac79a3b6b891729831d4148b89234
Reviewed-on: https://go-review.googlesource.com/c/go/+/479075
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Nicolas Hillegeer <aktau@google.com>
Run-TryBot: Nicolas Hillegeer <aktau@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This new type will be used in the following commits.
The goal is to have a tristate, an enum with three values:
- system default (0)
- enabled
- disabled
The system default value is linked to defaultMPTCPEnabled: disabled by
default for the moment. Users will be able to force enabling/disabling
MPTCP or use the default behaviour.
This work has been co-developped by Gregory Detal
<gregory.detal@tessares.net>.
Updates #56539
Change-Id: I8fa0cad7a18ca967508799fc828ef060b27683d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/477735
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
This check is already done by PeekASN1Tag.
Change-Id: Ieba0e35548f7f99bce689d29adaea6b8e471cc70
GitHub-Last-Rev: b4ef3dcc23
GitHub-Pull-Request: golang/go#59197
Reviewed-on: https://go-review.googlesource.com/c/go/+/478835
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: I84533d2df1a20f6337c43b1ca00d8022909a0018
GitHub-Last-Rev: 7dcc4e7296
GitHub-Pull-Request: golang/go#59195
Reviewed-on: https://go-review.googlesource.com/c/go/+/478816
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Under the right conditions we can optimize cmp comparisons to cmn
comparisons, such as:
func foo(a, b int) int {
var c int
if a + b < 0 {
c = 1
}
return c
}
Previously it's compiled as:
ADD R1, R0, R1
CMP $0, R1
CSET LT, R0
With this CL it's compiled as:
CMN R1, R0
CSET MI, R0
Here we need to pay attention to the overflow situation of a+b, the MI
flag means N==1, which doesn't honor the overflow flag V, its value
depends only on the sign of the result. So it has the same semantic of
the Go code, so it's correct.
Similarly, this CL also optimizes the case of >= comparison
using the PL conditional flag.
Change-Id: I47179faba5b30cca84ea69bafa2ad5241bf6dfba
Reviewed-on: https://go-review.googlesource.com/c/go/+/476116
Run-TryBot: Eric Fang <eric.fang@arm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL combines some rules with the same structure.
In order to avoid extremely long rules, this CL does not merge some
rules. In addition, this CL aligned the components of some rules for
better reading.
Change-Id: I4ba1493251ace00b10591e3c8eef4b6277a4b226
Reviewed-on: https://go-review.googlesource.com/c/go/+/476115
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Currently, net/http replaces the Referer header with the URL of the
previous request, regardless of its status. This CL changes this
behavior, respecting the Referer header for secure connections, if it is
set.
Fixes#44160
Change-Id: I2d7fe37dd681549136329e832188294691584870
Reviewed-on: https://go-review.googlesource.com/c/go/+/291636
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nick Craig-Wood <nickcw@gmail.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Most of these are one-off mistakes. Only one file was all spaces.
Change-Id: I277c3ce4a4811aa4248c90676f66bc775ae8d062
Reviewed-on: https://go-review.googlesource.com/c/go/+/478976
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The -test.v=test2json flag causes the testing package to inject extra
control characters in the output to allow the JSON parser to more
gracefully handle extraneous writes to os.Stdout and/or os.Stderr in
the package under test (see CL 443596). However, it doesn't filter out
those control characters because almost no real-world tests will
output them.
It turns out that testing.TestFlag is one of the rare tests that does
output those control characters, because it tests the
-test.v=test2json flag itself.
Fixes#59181.
Change-Id: I35ca6748afcd3d4333563028817caac946f5e86a
Reviewed-on: https://go-review.googlesource.com/c/go/+/479035
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Fixes#59172
Change-Id: I54d5e724f10117a40ec5dd58c810f6bbb2475933
GitHub-Last-Rev: d1a986698c
GitHub-Pull-Request: golang/go#59173
Reviewed-on: https://go-review.googlesource.com/c/go/+/478215
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Fetch usernames and groups via systemd userdb if available.
Otherwise fall back to parsing /etc/passwd, etc.
Fixes#38810
Co-authored-by: Michael Stapelberg <stapelberg@google.com>
Change-Id: Iff6ffc54feec6b6cec241b89e362c2285c8c0454
GitHub-Last-Rev: 1a627cc9a1
GitHub-Pull-Request: golang/go#57458
Reviewed-on: https://go-review.googlesource.com/c/go/+/459455
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
The vendor_import test lists packages that are known bad (e.g.
bad.go, invalid.go). Pass -e to permit error.
The mod_vendor_auto test includes a package that imports a main
package, which should be an error. Pass -e to permit error.
Updates #59186.
Change-Id: I3b63025c3935f55feda1a95151d4c688d0394644
Reviewed-on: https://go-review.googlesource.com/c/go/+/477838
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Currently it uses "go list ...", which includes all packages in
the known universe, and may include unresolved dependencies. The
test for issue #8181 is specifically for that the test dependency
of package b is downloaded. Test that specifically.
Change-Id: Icfbd7e197698b10ae4bc7c8aa3b0f2c477ca6b8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/477837
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
- Log the actual addresses reported, in case that information is relevant.
- Keep going after the first error, so that we report more information
about the idle connections after they have been used. (Was the first
connection dropped completely, or did it later show up as idle?)
- Remove the third request at the end of the test. It had been
assuming that the address for a new connection would always be
different from the address for the just-closed connection; however,
that assumption does not hold in general.
Removing the third request addresses one of the two failure modes seen
in #55195. It may help in investigating the other failure mode, but I
do not expect it to fix the failures entirely. (I suspect that the
other failure mode is a synchronization bug in returning the idle
connection from the first request.)
For #55195.
Change-Id: If9604ea68db0697268288ce9812dd57633e83fbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/478515
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
This is intended to fix the failure mode observed in
https://build.golang.org/log/f153e06ed547517fb2cddb0fa817fea40a6146f7,
but I haven't been able to reproduce that failure mode locally so I'm
not sure whether it actually does.
Change-Id: Ib14378f1299a76b54013419bdc715a9dbdd94667
Reviewed-on: https://go-review.googlesource.com/c/go/+/478235
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In #59155, we observed that the IdleConnStrsForTesting_h2 helper
function sometimes reported extra connections after a
"client conn not usable" failure and retry. It turns out that that
state corresponds exactly to the
http2clientConnIdleState.canTakeNewRequest field, so (with a bit of
extra nethttpomithttp2 plumbing) we can use that field in the helper
to filter out the unusable connections.
Fixes#59155.
Change-Id: Ief6283c9c8c5ec47dd9f378beb0ddf720832484e
Reviewed-on: https://go-review.googlesource.com/c/go/+/477856
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Also add code to replace the vendor directory in the prefix-map in
vendored modules. We weren't doing that before because in vendored
modules, the module's Dir field was set to empty, so nothing was being
replaced. Instead when Dir is not set, so we are in vendor mode,
replace the entire vendor directory's path.
Change-Id: I910499c74237699fd36d18049909a72e2b6705d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/478455
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
During the context switch of goroutine scheduling, the value of the
fcc0 register needs to be saved on the stack.
Fixs #59000.
Change-Id: Ie80dbae738f60df6c11a3fe31fc57de817d76afc
Reviewed-on: https://go-review.googlesource.com/c/go/+/475577
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: WANG Xuerui <git@xen0n.name>
For #59169
Change-Id: Id72ad9fe8b6e1d7cf64f972520ae8858f70c025a
Reviewed-on: https://go-review.googlesource.com/c/go/+/478217
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
These are ISA 3.0 power9 instructions which are helpful when reducing
a vector compare result into a GPR.
They are used in a future patch to improve the bytes.IndexByte asm
routine.
Change-Id: I424e2628e577167b9b7c0fcbd82099daf568ea35
Reviewed-on: https://go-review.googlesource.com/c/go/+/478115
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
While at it, also use concrete type for non-interface type test.
Change-Id: Ie468c30ee31ba99ef8f9a810d3be851fd37b9b43
Reviewed-on: https://go-review.googlesource.com/c/go/+/478356
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Delete the set of bytes that need quoting in TextHandler, because it
is almost identical to the set for JSON. Use JSONHandler's safeSet
with a few exceptions.
Updates #56345.
Change-Id: Iff6d309c067affef2e5ecfcebd6e1bb8f00f95b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/478198
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bench log file is created non-portably, only works on system where
"/tmp" existed and "/" is path separator.
Fixing this by using portable methods from std lib.
Updates #56345
Change-Id: I1f6b6b97b913ca56a6053beca7025652618ecbf3
Reviewed-on: https://go-review.googlesource.com/c/go/+/478355
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Truncate() a non existent file on Windows currently creates a new blank
file. This behavior is not consistent with other OSes where a file not
found error would instead be returned. This change makes Truncate on
Windows return a file-not-found error when the specified file doesn't
exist, bringing the behavior consistent.
New test cases have been added to prevent a regression.
Fixes#58977
Change-Id: Iaf7b41fc4ea86a2b2ccc59f8be81be42ed211b5c
GitHub-Last-Rev: 636b6c37c1
GitHub-Pull-Request: golang/go#59085
Reviewed-on: https://go-review.googlesource.com/c/go/+/477215
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
For G[T] that was seen and compiled in imported package, it is not added
to typecheck.Target.Decls, prevent wasting compile time re-creating
DUPOKS symbols. However, the linker do not support a type symbol
referencing a method symbol across DSO boundary. That causes unreachable
sym error when building under -linkshared mode.
To fix it, always re-compile generic methods in linkshared mode.
Fixes#58966
Change-Id: I894b417cfe8234ae1fe809cc975889345df22cef
Reviewed-on: https://go-review.googlesource.com/c/go/+/477375
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
- Remove the norace_test.go files, moving their contents elsewhere.
- Rename the internal/testutil package to internal/slogtest.
- Remove value_unsafe.go, moving its contents to value.go.
Updates golang/go#56345.
Change-Id: I2a24ace5aea47f7a3067cd671f606c4fb279d744
Reviewed-on: https://go-review.googlesource.com/c/go/+/478197
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For ++/-- statements, we know that syntax.AssignStmt.Lhs is a
single expression. Avoid unpacking (and allocating a slice) in
that case. Minor optimization.
Change-Id: I6615fd12277b1cd7d4f8b86e0b9d39f27708c13e
Reviewed-on: https://go-review.googlesource.com/c/go/+/477915
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
We can just use Checker.use, as long as we take care of blank (_)
identifiers that may appear of the LHS of assignments. It's ok to
"use" non-blank variables in case of an error, even on the LHS.
This makes this code match the types2 implementation.
Change-Id: Ied9b9802ecb63912631bbde1dc6993ae855a691b
Reviewed-on: https://go-review.googlesource.com/c/go/+/477895
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Step towards disentangling assignment checking functionality.
In preparation for reverse inference of function type arguments,
but independently helpful in better separating concerns in the code.
Change-Id: I9bac9d8005090c00d9ae6c5cfa13765aacce6b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/477855
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
The changes to exprList (in call.go), made in CL 282193, didn't
get faithfully ported to types2: in the case of operand mode
commaerr, unpacking didn't correctly set the type of the 2nd
value to error. This shouldn't matter for the compiler, but
the code differs from the go/types version. Make them the same.
Change-Id: I6f69575f9ad4f43169b851dffeed85c19588a261
Reviewed-on: https://go-review.googlesource.com/c/go/+/478255
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
The runtime/trace package proved useful for investigating go command
performance, and it makes sense (to me) to make this available for
development behind an undocumented flag, at the cost of ~25KB of binary
size. We could of course futher hide this functionality behind an
experiment or build tag, if necessary.
Updates #59157
Change-Id: I612320920ca935f1ee10bb6a803b7952f36c939b
Reviewed-on: https://go-review.googlesource.com/c/go/+/477896
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
On Linux, for a shared object, at least with the Gold linker, the
output file path is recorded in the .gnu.version_d section. When
the output file path is in a temporary directory, it causes
nondeterministic build.
This is similar to #58557, but for Linux with the Gold linker.
Apply the same workaround as in CL 477296.
Should fix the linux-arm64-longtest builder.
Change-Id: Ic703bff32c1bcc40054b89be696e04280855e876
Reviewed-on: https://go-review.googlesource.com/c/go/+/478196
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Make it clear that "qualified by the given name" doesn't imply
any particular implementation, but depends on how the Handler
treats groups.
Updates golang/go#56345.
Change-Id: Idf52553108bc1a7d2091a06cc4fc4cc0fc52cc14
Reviewed-on: https://go-review.googlesource.com/c/go/+/478056
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Use the new functions in 1.20 (unsafe.StringData, etc.) instead
of StringHeader and StructHeader from the reflect package.
Updates golang/go#56345.
Change-Id: I84d0db7b203aeffe45ce8b06beb7b4ee17e19949
Reviewed-on: https://go-review.googlesource.com/c/go/+/478055
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The slog structured logging package.
This code was copied from the slog directory of the x/exp repo
at commit 642cacee5cc05231f45555a333d07f1005ffc287, with the
following changes:
- Change import paths.
- Delete unused files list.go, list_test.go.
- Rename example_depth_test.go to example_wrap_test.go and
adjust example output.
- Change the tag safe_values to safe_slog_values.
- Make captureHandler goroutine-safe to fix a race condition
in benchmarks.
- Other small changes as suggested in review comments.
Also, add dependencies to go/build/deps_test.go.
Also, add new API for the API checker.
Updates golang/go#56345.
Change-Id: Id8d720967571ced5c5f32c84a8dd9584943cd7df
Reviewed-on: https://go-review.googlesource.com/c/go/+/477295
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Currently the pacer is designed to pace against the edge. Specifically,
it tries to find the sweet spot at which there are zero assists, but
simultaneously finishes each GC perfectly on time.
This pretty much works, despite the noisiness of the measurement of the
cons/mark ratio, which is central to the pacer's function. (And this
noise is basically a given; the cons/mark ratio is used as a prediction
under a steady-state assumption.) Typically, this means that the GC
might assist a little bit more because it started the GC late, or it
might execute more GC cycles because it started early. In many cases the
magnitude of this variation is small.
However, we can't possibly control for all sources of noise, especially
since some noise can come from the underlying system. Furthermore, there
are inputs to the measurement that have effectively no restrictions on
how they vary, and the pacer needs to assume that they're essentially
static when they might not be in some applications (i.e. goroutine
stacks).
The result of high noise is that the variation in when a GC starts is
much higher, leading to a significant amount of assists in some GC
cycles. While the GC cycle frequency basically averages out in the
steady-state in the face of this variation, starting a GC late has the
significant drawback of reducing application latencies.
This CL thus biases the pacer toward avoiding assists by picking a
cons/mark smoothing function that takes the maximum measured cons/mark
over 5 cycles total. I picked 5 cycles because empirically this was the
best trade-off between window size and smoothness for a uniformly
distributed jitter in the cons/mark signal. The cost here is that if
there's a significant phase change in the application that makes it less
active with the GC, then we'll be using a stale cons/mark measurement
for 5 cycles. I suspect this is fine precisely because this only happens
when the application becomes less active, i.e. when latency matters
less.
Another good reason for this particular bias is that even though the GC
might start earlier and end earlier on average, resulting in more
frequent GC cycles and potentially worse throughput, it also means that
it uses less memory used on average. As a result, there's a reasonable
workaround in just turning GOGC up slightly to reduce GC cycle
frequency and bringing memory (and hopefully throughput) levels back to
the same baseline. Meanwhile, there should still be fewer assists than
before which is just a clear improvement to latency.
Lastly, this CL updates the GC pacer tests to capture this bias against
assists and toward GC cycles starting earlier in the face of noise.
Sweet benchmarks didn't show any meaningful difference, but real
production applications showed a reduction in tail latencies of up
to 45%.
Updates #56966.
Change-Id: I8f03d793f9a1c6e7ef3524d18294dbc0d7de6122
Reviewed-on: https://go-review.googlesource.com/c/go/+/467875
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This is relatively easy using the new traceback iterator.
Ancestor tracebacks are now limited to 50 frames. We could keep that
at 100, but the fact that it used 100 before seemed arbitrary and
unnecessary.
Fixes#7181
Updates #54466
Change-Id: If693045881d84848f17e568df275a5105b6f1cb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/475960
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Correct an error message to missing section, not unreachable
symbol.
Also, under -v >= 2, dump symbol info on error for debugging.
Updates #58966.
Change-Id: I0f832c517d64f4b672b313a8b9be2d028744f945
Reviewed-on: https://go-review.googlesource.com/c/go/+/476735
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
