Fixes#66836
Change-Id: I603faca2acd2bcffabbcaca8b8670d46387d2a5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/578995
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Currently the directory is created only if -objdir is omited.
Creating the directory here is useful to avoid doing this in each build system.
And also this is consistent with similar flags of other tools like `-o`.
Change-Id: Ic39d6eb3e003bc4884089f80f790e30df4a54b01
Reviewed-on: https://go-review.googlesource.com/c/go/+/576815
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Before, all methods of File (including Close) were
safe for concurrent use (I checked), except the three
variants of ReadDir.
This change makes the ReadDir operations
atomic too, and documents explicitly that all methods
of File have this property, which was already implied
by the package documentation.
Fixes#66498
Change-Id: I05c88b4e60b44c702062e99ed8f4a32e7945927a
Reviewed-on: https://go-review.googlesource.com/c/go/+/578322
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL changes the interpretation of the unset value
of gotypesalias to equal "1". The actual deletion of
all the transitional logic will happen in a follow-up.
Note that the compiler still interprets unset as "0".
More work appears to be required within the compiler
before it is safe to flip its default.
Change-Id: I854ab1fd856c7c361a757676b0670e2f23402816
Reviewed-on: https://go-review.googlesource.com/c/go/+/577715
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL ports to types2 the (passing) test from CL 576975,
which fixed a bug in go/types.
Updates #66704
Updates #65294
Change-Id: Icdf77e39ed177d9f9ecc435d5125f02f2ee4dd0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/579015
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change simplifies the construction of the Huffman tree in the
bzip2 package by replacing custom sort logic with the more concise and
idiomatic use of "slices" and "cmp" packages.
Change-Id: I2a8aef146b54b9433038b133d2cc8856ba077c72
GitHub-Last-Rev: c031bb5663
GitHub-Pull-Request: golang/go#66817
Reviewed-on: https://go-review.googlesource.com/c/go/+/578438
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
After the alignment of the loop header is performed, the offset of the checked
conditional branch instruction may overflow, so it needs to be checked again.
When checking whether the offset of the branch jump instruction overflows, it
can be classified and processed according to the range of the immediate field
of the specific instruction, which can reduce the introduction of unnecessary
jump instructions.
Fixes#61819
Change-Id: I772a5b5b8b8de21c78d7566be30be8ff65fdbce8
Reviewed-on: https://go-review.googlesource.com/c/go/+/519915
Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Meidan Li <limeidan@loongson.cn>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: qiu laidongfeng2 <2645477756@qq.com>
Reviewed-by: WANG Xuerui <git@xen0n.name>
The "rt" seems to be caused after copy-pasting the previous "wt" block
which make sense as WriterTo, but for ReaderFrom it makes more sense
thinking of rf instead of rt.
Change-Id: I873699c27211bea6cdba3e199f36eb3c38188d70
GitHub-Last-Rev: 1795600a9b
GitHub-Pull-Request: golang/go#66811
Reviewed-on: https://go-review.googlesource.com/c/go/+/578635
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This change removes the ExecTracer2 experiment flag. This flag was
created as part of the tracer overhaul described in #60773.
Updates #66703
For #60773
Change-Id: Ib95b9dd8bb68b48e7543c4d28d853c6a518438f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/576258
Auto-Submit: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This change renames the v2 execution tracer files created as part of
Updates #66703
For #60773
Change-Id: I91bfdc08fec4ec68ff3a6e8b5c86f6f8bcae6e6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/576257
Auto-Submit: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This change makes the new execution tracer described in #60773, the
default tracer. This change attempts to make the smallest amount of
changes for a single CL.
Updates #66703
For #60773
Change-Id: I3742f3419c54f07d7c020ae5e1c18d29d8bcae6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/576256
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I5bccef3c46072e388bfe9985e70745853f673a42
GitHub-Last-Rev: adee7b9f7f
GitHub-Pull-Request: golang/go#66829
Reviewed-on: https://go-review.googlesource.com/c/go/+/578875
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I7507e6cff00d027fd7840e0661499efc63353f6e
GitHub-Last-Rev: 81348ed39d
GitHub-Pull-Request: golang/go#66820
Reviewed-on: https://go-review.googlesource.com/c/go/+/578441
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Also simulate TCP_KEEPIDLE, TCP_KEEPINTVL, and TCP_KEEPCNT with
TCP_KEEPALIVE_THRESHOLD + TCP_KEEPALIVE_ABORT_THRESHOLD for
Solaris prior to 11.4
Fixes#9614Fixes#64251
Change-Id: Ia0777076a7952630bc52761cddd0b06b0d81c6a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/577195
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Change-Id: I3eb41562bf6478b288d9f41915fd7d027399a6ba
GitHub-Last-Rev: 218b9cb067
GitHub-Pull-Request: golang/go#66551
Reviewed-on: https://go-review.googlesource.com/c/go/+/574575
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes#63116
Change-Id: Iab8c415555ab85097be6d2d133b3349c5219a23b
GitHub-Last-Rev: 8a8177b9af
GitHub-Pull-Request: golang/go#63348
Reviewed-on: https://go-review.googlesource.com/c/go/+/532217
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Along with the removal of the slow path from Linux and *BSD.
For #59359
Change-Id: I6c79594252e5e5f1c1c57c11e09458fcae3793d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/577175
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Fixes#65812
Change-Id: I63facb32eeddbe9b6e0279f1c039779ba2e2ab7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/575015
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes#66760
Change-Id: I6ba5bc5b00506b66cb8dc3984a61f32a6358d9bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/577895
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This is a practical use of CL 577915, follow-up to CL 577835.
Change-Id: Ibe7e2fa11b444afa1898dc6f6aba1512fe98f1fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/578195
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
It wasn't DocLink compatible in a number of ways.
Change-Id: Ib7ab9a908ef47561ac70cdc0c157d49dcfd03a02
Reviewed-on: https://go-review.googlesource.com/c/go/+/577375
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes#66784
Change-Id: Ifd17e0830e04e7028d8a876c6c12c496f5167887
Reviewed-on: https://go-review.googlesource.com/c/go/+/578395
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I449fcb008fa4c1f51de0bef9f016907d8b99da67
GitHub-Last-Rev: b2e84f1a8a
GitHub-Pull-Request: golang/go#66772
Reviewed-on: https://go-review.googlesource.com/c/go/+/578135
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In both the v1 and v2 cmd/trace, pprofMatchingGoroutines will generate
no output at all if the filter name passed to it is the empty string.
This is rather pointless because there are at least two places where we
don't pass a name to filter. Modify pprofMatchingGoroutines to include
*all* goroutines in the trace if the name to filter by is not specified.
For #66782.
Change-Id: I6b72298d676bc93892b075a7426e6e56bc6656c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/578356
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Turns out we ported all the profile generation, but forgot to actually
support the command line flags for them! This change fixes the issue by
handling the different kinds of profiles and writing them out to stdout.
Fixes#66782.
Change-Id: I7756fb4636ce8daaf11ed471be79c86ce3d463cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/578318
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This will avoid a potential endless loop for a corrupt DNS packet.
For #66754
Change-Id: I46591b3f7695bcc88d2312833e45955f8c129d2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/578375
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Currently TestGCStress' main loop allocates a randomly-sized byte slice
in a loop. On the windows-386 builder, it looks like the following is
happening.
In such heavily-allocating scenarios, the test seems to be able to
outrun the GC. This is a known issue on all platforms, but it looks to
me like there may be a real issue with mark termination. (Fixing that is
outside the scope of this change, but relevant here.)
Furthermore, while the test is ramping up, the pacer is taking time to
acclimate to the high allocation rate. This is probably made worse due
to the coarse time granularity on Windows, since the pacer relies on
accurate time measurements.
Because the pacer is ramping up, it isn't starting early enough, causing
a lot of memory to get allocated black and inflate the live heap size.
This happens for more than one cycle.
Last but not least, because the core allocating loop of this test
allocates randomly-sized byte slices, we could just get unlucky and
inflate the live heap by much more sometimes. Furthermore, the
randomness creates chaos for the pacer that is totally unnecessary for
this test.
Although I couldn't reproduce the issue we're seeing on the trybots in a
gomote, I *could* reproduce memory spikes in general. These memory
spikes always occurred before the pacer had a chance to "warm up," in
the first two cycles after the heavy allocating begins.
I believe the flakiness we're seeing is all of these factors lining up,
because if I just make the size of the allocated byte slices smaller and
non-random, I can no longer reproduce the memory spikes. This change
implements this as a fix in the hope that it'll resolve the flakiness.
Fixes#66624.
Change-Id: I478d45e7c600e5aee4b21dbe831e1f287284f5e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/578319
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
There is no hyphen between the organization and the number.
For example, https://standards.ieee.org/ieee/754/6210/
shows the string "IEEE 754-2019" and not "IEEE-754-2019".
This assists in searching for "IEEE 754" in documentation
and not missing those using "IEEE-754".
Change-Id: I9a50ede807984ff1e2f17390bc1039f6a5d162e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/575438
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For Go 1.23, we decided to no longer support the old CLI interface
exposed by wasmtime. This removes the extra logic included to support
both the new and the old CLI interface. Now only versions of wasmtime
14 and newer are supported.
Fixes#63718
Change-Id: Iea31388dc41bc8d73caa923c7e4acae2228bf515
Reviewed-on: https://go-review.googlesource.com/c/go/+/577135
Reviewed-by: Randy Reddig <randy.reddig@fastly.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Use "slices.Equal" instead of "reflect.DeepEqual".
Replace unnecessary helper type "byIndex" with "slices.SortFunc".
No effect on benchmarks.
Change-Id: I1fb2768ea6d9db7f487408fa109343be3f1741d5
GitHub-Last-Rev: 8429bc1452
GitHub-Pull-Request: golang/go#66646
Reviewed-on: https://go-review.googlesource.com/c/go/+/575715
Reviewed-by: qiu laidongfeng2 <2645477756@qq.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Like on other BSDs, use faccessat(AT_FDCWD, path, mode, AT_EACCESS)
Change-Id: I80f8d327dd152576165b9206e32dfb749b41d187
Reviewed-on: https://go-review.googlesource.com/c/go/+/538836
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
CL 570555 replaced a loop which added empty
color.RGBA elements with a call to clear.
color.Palette is a slice of interfaces, so using
clear results in a slice of nil elements, rather
than what we previously had which was empty
color.RGBA elements. This could cause a panic when
attempting to re-encode a GIF which had an
extended color palette because of the weird
transparency hack.
This was discovered by OSS-Fuzz. I've added a test
case using their reproducer in order to prevent
future regressions.
Change-Id: I00a89257d90b6cca68672173eecdaa0a24f18d9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/577555
Reviewed-by: Nigel Tao <nigeltao@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
For #61060
Change-Id: I13cd73b4062cb7bd248d2a4afae06dfa29ac0203
Reviewed-on: https://go-review.googlesource.com/c/go/+/577955
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Depending on the query, a RawBytes can contain memory owned by the
driver or by database/sql:
If the driver provides the column as a []byte,
RawBytes aliases that []byte.
If the driver provides the column as any other type,
RawBytes contains memory allocated by database/sql.
Prior to this CL, Rows.Scan will reuse existing capacity in a
RawBytes to permit a single allocation to be reused across rows.
When a RawBytes is reused across queries, this can result
in database/sql writing to driver-owned memory.
Add a buffer to Rows to store RawBytes data, and reuse this
buffer across calls to Rows.Scan.
Fixes#65201
Change-Id: Iac640174c7afa97eeb39496f47dec202501b2483
Reviewed-on: https://go-review.googlesource.com/c/go/+/557917
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Document that links to symbols in the standard library can be
written as "[foo]", without the actual link.
For #64169.
Change-Id: I9d8a33e85df70037320a169d55a2bb4a8a981ebf
Reviewed-on: https://go-review.googlesource.com/c/go/+/577915
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This test has been OOMing on 32-bit platforms for a bit. I suspect the
very high allocation rate is causing the program to outrun the GC in
some corner-case scenarios, especially on 32-bit Windows.
I don't have a strong grasp of what's going on yet, but lowering the
memory footprint should help with the flakiness. This shouldn't
represent a loss in test coverage, since we're still allocating and
assisting plenty (tracing the latter is a strong reason this test
exists).
For #66624.
Change-Id: Idd832cfc5cde04701386919df4490f201c71130a
Reviewed-on: https://go-review.googlesource.com/c/go/+/577475
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
The existing implementation of traceMap is a hash map with a fixed
bucket table size which scales poorly with the number of elements added
to the map. After a few thousands elements are in the map, it tends to
fall over.
Furthermore, cleaning up the trace map is currently non-preemptible,
without very good reason.
This change replaces the traceMap implementation with a simple
append-only concurrent hash-trie. The data structure is incredibly
simple and does not suffer at all from the same scaling issues.
Because the traceMap no longer has a lock, and the traceRegionAlloc it
embeds is not thread-safe, we have to push that lock down. While we're
here, this change also makes the fast path for the traceRegionAlloc
lock-free. This may not be inherently faster due to contention on the
atomic add, but it creates an easy path to sharding the main allocation
buffer to reduce contention in the future. (We might want to also
consider a fully thread-local allocator that covers both string and
stack tables. The only reason a thread-local allocator isn't feasible
right now is because each of these has their own region, but we could
certainly group all them together.)
Change-Id: I8c06d42825c326061a1b8569e322afc4bc2a513a
Reviewed-on: https://go-review.googlesource.com/c/go/+/570035
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
Currently lots of functions require systemstack because the trace buffer
might get flushed, but that will already switch to the systemstack for
the most critical bits (grabbing trace.lock). That means a lot of this
code is non-preemptible when it doesn't need to be. We've seen this
cause problems at scale, when dumping very large numbers of stacks at
once, for example.
This is a re-land of CL 572095 which was reverted in CL 577376. This
re-land includes a fix of the test that broke on the longtest builders.
Change-Id: Ia8d7cbe3aaa8398cf4a1818bac66c3415a399348
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/577377
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Change-Id: I28baf375502ccf3b0dfcc8250650e914ac7bae78
Reviewed-on: https://go-review.googlesource.com/c/go/+/577835
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Jes Cok <xigua67damn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This is covered by the existing tests under gotypesalias=1.
Change-Id: Ia17f35fe580b745fa4bdaf4689dfe9c2ed6ebc5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/577735
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
This change reenables the legacy three-phase resolution
(non-alias typenames, aliases, the rest) even when
GODEBUG=gotypesalias=1. Unfortunately the existing test case
for #50259 causes the simpler logic to fail.
Updates #50259
Updates #65294
Change-Id: Ibfaf8146e46760718673a916a9b220a9d678409a
Reviewed-on: https://go-review.googlesource.com/c/go/+/577616
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Flag flip to enable stack slot merging by default when optimizing.
Please see the earlier CL for details on what this is doing.
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: I8e30d553e74ace43d418f883199721f05320d3d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/576681
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
It is possible to have situations where a given ir.Name is
non-address-taken at the source level, but whose address is
materialized in order to accommodate the needs of arch-dependent
memory ops. The issue here is that the SymAddr op will show up as
touching a variable of interest, but the subsequent memory op will
not. This is generally not an issue for computing whether something is
live across a call, but it is problematic for collecting the more
fine-grained live interval info that drives stack slot merging.
As an example, consider this Go code:
package p
type T struct {
x [10]int
f float64
}
func ABC(i, j int) int {
var t T
t.x[i&3] = j
return t.x[j&3]
}
On amd64 the code sequences we'll see for accesses to "t" might look like
v10 = VarDef <mem> {t} v1
v5 = MOVOstoreconst <mem> {t} [val=0,off=0] v2 v10
v23 = LEAQ <*T> {t} [8] v2 : DI
v12 = DUFFZERO <mem> [80] v23 v5
v14 = ANDQconst <int> [3] v7 : AX
v19 = MOVQstoreidx8 <mem> {t} v2 v14 v8 v12
v22 = ANDQconst <int> [3] v8 : BX
v24 = MOVQloadidx8 <int> {t} v2 v22 v19 : AX
v25 = MakeResult <int,mem> v24 v19 : <>
Note that the the loads and stores (ex: v19, v24) all refer directly
to "t", which means that regular live analysis will work fine for
identifying variable lifetimes. The DUFFZERO is (in effect) an
indirect write, but since there are accesses immediately after it we
wind up with the same live intervals.
Now the same code with GOARCH=ppc64:
v10 = VarDef <mem> {t} v1
v20 = MOVDaddr <*T> {t} v2 : R20
v12 = LoweredZero <mem> [88] v20 v10
v3 = CLRLSLDI <int> [212543] v7 : R5
v15 = MOVDaddr <*T> {t} v2 : R6
v19 = MOVDstoreidx <mem> v15 v3 v8 v12
v29 = CLRLSLDI <int> [212543] v8 : R4
v24 = MOVDloadidx <int> v15 v29 v19 : R3
v25 = MakeResult <int,mem> v24 v19 : <>
Here instead of memory ops that refer directly to the symbol, we take
the address of "t" (ex: v15) and then pass the address to memory ops
(where the ops themselves no longer refer to the symbol).
This patch enhances the stack slot merging liveness analysis to handle
cases like the PPC64 one above. We add a new phase in candidate
selection that collects more precise use information for merge
candidates, and screens out candidates that are too difficult to
analyze. The phase make a forward pass over each basic block looking
for instructions of the form vK := SymAddr(N) where N is a raw
candidate. It then creates an entry in a map with key vK and value
holding name and the vK use count. As the walk continues, we check for
uses of of vK: when we see one, record it in a side table as an
upwards exposed use of N. At each vK use we also decrement the use
count in the map entry, and if we hit zero, remove the map entry. If
we hit the end of the basic block and we still have map entries, this
implies that the address in question "escapes" the block -- at that
point to be conservative we just evict the name in question from the
candidate set.
Although this CL fixes the issues that forced a revert of the original
merging CL, this CL doesn't enable stack slot merging by default; a
subsequent CL will do that.
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: Id41d359a677767a8e7ac1e962ae23f7becb4031f
Reviewed-on: https://go-review.googlesource.com/c/go/+/576735
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For -gcflags=-d=mergelocalstrace=1 (which reports estimated savings
from stack slot merging), emit separate values for pointerful vs
non-pointerful variables, for a bit more detail.
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: I9dd27d2a254036448c85c13d189d1ed36157c9d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/576680
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
[This is a partial roll-forward of CL 553055, the main change here
is that the stack slot overlap operation is flagged off by default
(can be enabled by hand with -gcflags=-d=mergelocals=1) ]
Preliminary compiler support for merging/overlapping stack slots of
local variables whose access patterns are disjoint.
This patch includes changes in AllocFrame to do the actual
merging/overlapping based on information returned from a new
liveness.MergeLocals helper. The MergeLocals helper identifies
candidates by looking for sets of AUTO variables that either A) have
the same size and GC shape (if types contain pointers), or B) have the
same size (but potentially different types as long as those types have
no pointers). Variables must be greater than (3*types.PtrSize) in size
to be considered for merging.
After forming candidates, MergeLocals collects variables into "can be
overlapped" equivalence classes or partitions; this process is driven
by an additional liveness analysis pass. Ideally it would be nice to
move the existing stackmap liveness pass up before AllocFrame
and "widen" it to include merge candidates so that we can do just a
single liveness as opposed to two passes, however this may be difficult
given that the merge-locals liveness has to take into account
writes corresponding to dead stores.
This patch also required a change to the way ssa.OpVarDef pseudo-ops
are generated; prior to this point they would only be created for
variables whose type included pointers; if stack slot merging is
enabled then the ssagen code creates OpVarDef ops for all auto vars
that are merge candidates.
Note that some temporaries created late in the compilation process
(e.g. during ssa backend) are difficult to reason about, especially in
cases where we take the address of a temp and pass it to the runtime.
For the time being we mark most of the vars created post-ssagen as
"not a merge candidate".
Stack slot merging for locals/autos is enabled by default if "-N" is
not in effect, and can be disabled via "-gcflags=-d=mergelocals=0".
Fixmes/todos/restrictions:
- try lowering size restrictions
- re-evaluate the various skips that happen in SSA-created autotmps
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: Ifda26bc48cde5667de245c8a9671b3f0a30bb45d
Reviewed-on: https://go-review.googlesource.com/c/go/+/575415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This change makes it possible for the runtime to preempt the zeroing of
large objects that contain pointers. It turns out this is fairly
straightforward with allocation headers, since we can just temporarily
tell the GC that there's nothing to scan for a large object with a
single pointer write (as opposed to trying to zero a whole bunch of
bits, as we would've had to do once upon a time).
Fixes#31222.
Change-Id: I10d0dcfa3938c383282a3eb485a6f00070d07bd2
Reviewed-on: https://go-review.googlesource.com/c/go/+/577495
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
