It's called from exitsyscall, which is nosplit.
Change-Id: I3f5f92e044497a88a72c1870beb2bdd15c4263c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/496517
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Retrying the original CL with a small modification. The original CL
did not handle the case of reading an itab out of a dictionary
correctly. When we read an itab out of a dictionary, we must treat
the type inside that itab as maybe being put in an interface.
Original CL: 486895
Revert CL: 490156
Change-Id: Id2dc1699d184cd8c63dac83986a70b60b4e6cbd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/491495
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently the trace clock is cputicks() with comments sprinkled in
different places as to which clock to use. Since the execution tracer
redesign will use a different clock, it seems like a good time to clean
that up.
Also, rename the start/end timestamps to be more readable (i.e.
startTime vs. timeStart).
Change-Id: If43533eddd0e5f68885bb75cdbadb38da42e7584
Reviewed-on: https://go-review.googlesource.com/c/go/+/494775
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The new cmp package provides types and functions related to
comparing ordered values.
For #59488
Change-Id: I43f4b2e6036f63b87c2152672d2b6fa18235cbeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/496356
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Eli Bendersky <eliben@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
The implementation of SetDeadline in Plan 9 begins by calculating
d = the offset of the requested deadline from time.Now(). If d > 0,
a timer is set to interrupt future I/O. If d < 0, the channel is
flagged to prevent future I/O and any current I/O is cancelled.
But if d = 0, nothing happens and the deadline isn't set.
The d = 0 case should be handled the same as d < 0.
Fixes#60282Fixes#52896
Change-Id: Id8167db3604db1c129d99376fa78a3da75417d20
Reviewed-on: https://go-review.googlesource.com/c/go/+/496137
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Currently STW events are only emitted for GC STWs. There's little reason
why the trace can't contain events for every STW: they're rare so don't
take up much space in the trace, yet being able to see when the world
was stopped is often critical to debugging certain latency issues,
especially when they stem from user-level APIs.
This change adds new "kinds" to the EvGCSTWStart event, renames the
GCSTW events to just "STW," and lets the parser deal with unknown STW
kinds for future backwards compatibility.
But, this change must break trace compatibility, so it bumps the trace
version to Go 1.21.
This change also includes a small cleanup in the trace command, which
previously checked for STW events when deciding whether user tasks
overlapped with a GC. Looking at the source, I don't see a way for STW
events to ever enter the stream that that code looks at, so that
condition has been deleted.
Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494495
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This was an oversight, which might cause accounted-for idle time to be
lost. Noticed this while working on #60276.
Change-Id: Ic743785d6dc82555e660f2c9b6aaa9dedef56ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/496117
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
The new Pinner API's implementation imposes some partial-orders that are
safe but previously did not exist between a mspanSpecial, mheapSpecial,
and mheap. Fix that up in the lock ranking.
For #46787.
Change-Id: I51cc8f7f069240caeb44d749bed43515634f4814
Reviewed-on: https://go-review.googlesource.com/c/go/+/496193
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Some C APIs require the use or structures that contain pointers to
buffers (iovec, io_uring, ...). The pointer passing rules would
require that these buffers are allocated in C memory and to process
this data with Go libraries it would need to be copied.
In order to provide a zero-copy way to use these C APIs, this CL
implements a Pinner API that allows to pin Go objects, which
guarantees that the garbage collector does not move these objects
while pinned. This allows to relax the pointer passing rules so that
pinned pointers can be stored in C allocated memory or can be
contained in Go memory that is passed to C functions.
The Pin() method accepts pointers to objects of any type and
unsafe.Pointer. Slices and arrays can be pinned by calling Pin()
with the pointer to the first element. Pinning of maps is not
supported.
If the GC collects unreachable Pinner holding pinned objects it
panics. If Pin() is called with the other non-pointer types it
panics as well.
Performance considerations: This change has no impact on execution
time on existing code, because checks are only done in code paths,
that would panic otherwise. The memory footprint on existing code is
one pointer per memory span.
Fixes: #46787
Signed-off-by: Sven Anderson <sven@anderson.de>
Change-Id: I110031fe789b92277ae45a9455624687bd1c54f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/367296
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Currently if GOGC=off and GOMEMLIMIT is set, then the synchronous
scavenger is likely to work fairly often to maintain the limit, since
the heap goal goes right up to the edge of the memory limit (minus a
fixed 1 MiB of headroom).
If the application's allocation rate is high, and page-level
fragmentation is high, then most allocations will scavenge.
This change mitigates this problem by adding a proportional component
to constant headroom added to the memory-limit-based heap goal. This
means the runtime will have much more headroom before fragmentation
forces memory to be eagerly scavenged.
The proportional headroom in this case is 3%, or ~30 MiB for a 1 GiB
heap. This technically will increase GC frequency in the GOGC=off case
by a tiny amount, but will likely have a positive impact on both
allocation throughput and latency that outweighs this difference.
I wrote a small program to reproduce this issue and confirmed that the
issue is resolved by this patch:
https://github.com/golang/go/issues/57069#issuecomment-1551746565
This value of 3% is chosen as it seems to be a inflection point in this
particular small program. 2% still resulted in quite a bit of eager
scavenging work. I confirmed this results in a GC frequency increase of
about 3%.
This choice is still somewhat arbitrary because the program is
arbitrary, so perhaps worth revisiting in the future. Still, it should
help a good number of programs.
Fixes#57069.
Change-Id: Icb9829db0dfefb4fe42a0cabc5aa8d35970dd7d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460375
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
runtime.fcntl returns the error value as a negative value, so it needs
to be inverted before being converted to syscall.Errno.
Change-Id: I43cd0b035150424ac59e623b17a9396c7d62c186
Reviewed-on: https://go-review.googlesource.com/c/go/+/495675
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
There is no harm in continuing type-checking a built-in even if there
is a version error.
Change-Id: I161abd904a26075694c26639e247a17126947fcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/496415
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Also, clean up atomics on released-per-cycle while we're here.
For #57069.
Change-Id: I14026e8281f01dea1e8c8de6aa8944712b7b24d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/495916
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
I implemented this in order to debug connection failures on a
new-to-me VM development environment that uses Cloud NAT. It doesn't
directly fix the bug, but perhaps folks will find it useful to
diagnose port-exhaustion-related flakiness in other environments.
For #52545.
Change-Id: Icd3f13dcf62e718560c4f4a965a4df7c1bd785ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/473277
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
CL 495918 enabled testcarchive much more widely and added many dynamic
test skips. CL 495855 added TestDeepStack before these dynamic skips
were in. Unfortunately, the two CLs don't logically commute, so when
CL 495918 landed, it broke at least nocgo builders and platforms that
don't support c-archive builds. Fix this by adding the necessary skips
to TestDeepStack.
Change-Id: I3d352f731fe67a01c7b96871fde772db8eb21b5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/496376
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This CL permanently enables the new behavior for -lang=go1.21 and
newer, and keeps the existing behavior if -lang=go1.20 or older.
To be submitted once #58671 is accepted.
For #58671.
Change-Id: I83a1d393f0ce7871be8f38ec35742d393946c55f
Reviewed-on: https://go-review.googlesource.com/c/go/+/496035
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The test driver for testso and testsovar are literally identical, and
only the testdata code is different between the two test packages.
Merge them into a single test package with two tests that share a
driver.
Change-Id: I3f107a6aba345c0dd58606c10e3ac8eee33b33c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/496315
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Currently, this test only enabled on non-Darwin UNIX platforms because
it uses the non-standard _thread attribute for thread-local storage.
C11 introduced a standard way to declare something thread-local, so
this CL takes advantage of that to generalize the test to Darwin and
Windows.
Change-Id: Iba31b6216721df6eb8e978d7487cd3a787cae588
Reviewed-on: https://go-review.googlesource.com/c/go/+/496295
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Currently, dist registers cmd/cgo/internal{test,testtls,testnocgo}
specially, so they're opted out of "go test std cmd". It has to
register these test packages to run in various non-default build
configurations, but at this point they can also run with the default
build configuration (and for test and testtls, we intentionally want
to test them in the default configuration; this is pointless but
harmless for testnocgo). Hence, this CL drops the special registration
of their default build configurations from registerCgoTests and lets
them be registered as part of registerStdTests.
Change-Id: Id283f3cdcdb202955a854648c0ed1e3c4aa554d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/496179
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This test is actually intended to test that we can build in -static
mode even without any cgo. That means it's quite harmless to run in
the default build configuration (in addition to running with various
other build configurations).
Change-Id: Ic6cb5c0eaab83f9bd5718aae57d0fdc69afcb8b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/496178
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This makes testtls build and run on all platforms in the default build
configuration (though it will Skip on some).
Change-Id: I6aba96a82d618c9798a0d4418b40b2644cfceec9
Reviewed-on: https://go-review.googlesource.com/c/go/+/496177
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This is the default value of this flag, so passing it clutters up
debugging output. This also makes it clearer which tests are running
with a default configuration.
Change-Id: If793934829c79f087c7a6e3fa8f64dc33959c213
Reviewed-on: https://go-review.googlesource.com/c/go/+/496176
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Several cgo tests no longer have any special conditions, so they can
just be normal cmd tests. This brings dist's "go_test:.*" tests much
closer to what "go test std cmd" runs.
Change-Id: I4d09f60628a41081e97e6b6e7dd0d93df47a65bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/495919
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This CL moves many cgo test conditions out of dist and into the tests
themselves, now that they can use the testenv.Must* helpers.
This refines a lot of the conditions, which happens to have the effect
of enabling many tests on Android and iOS that are disabled by
too-coarse GOOS checks in dist today.
Fixes#15919.
Change-Id: I2947526b08928d2f7f89f107b5b2403b32092ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/495918
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Currently, we have several tests disabled if GO_GCFLAGS is non-empty.
Long ago, this was critical because many of these tests use "go
install" with no -gcflags and would thus overwrite std packages in
GOROOT built with -gcflags=$GO_GCFLAGS. Now these packages all live in
the build cache, so this is no longer a concern.
The other reason for this (the reason given in the code comment), is
that these tests will rebuild significant portions of std without
flags. While this is still theoretically true, there are many tests
that run "go build" with no -gcflags, so these tests don't contribute
much overall.
Empirically, on my linux/amd64 host, running these tests at all grows
the Go build cache by 14%, from 1.899 GB to 2.165 GB. When building
with GO_GCFLAGS="-N -l" (the only use case on the builders), enabling
them grows the Go build cache by 18%, from 1.424 GB to 1.684 GB. This
is only a 4 percentage point difference, and still results in a build
cache that's smaller than the default build
Given all this, there's little reason to carry the complexity of
disabling these tests when GO_GCFLAGS != "". Removing this condition
is a step toward running these as regular cmd tests.
Change-Id: I2c41be721927c40a742e01476cd9a0f7650d38e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/495917
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
It is incredibly slow, taking half as long as the regular cmd/api checks
and over 5 minutes on plan9-arm. Leave it for the longtest builders.
Change-Id: Ic8bd420f174268d0b6a13d84e7bd364f6c13cf41
Reviewed-on: https://go-review.googlesource.com/c/go/+/496375
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Change-Id: Ibddf36431abb799d8f9288d6e17159ce1538d62e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495879
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Generated with relnote. I did my best to put things where they made
sense, but some weren't obvious, like the Unicode upgrade and backward
compatibility stuff.
For #58645.
Change-Id: Ic3cd9ef32cec7591ace4d2df1da40e4afd97d083
Reviewed-on: https://go-review.googlesource.com/c/go/+/496316
Auto-Submit: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
These are some follow-up tweaks to CL 494958. This CL drops a stale
and unnecessary check and passes through trailing non-JSON output.
Updates #37486.
Change-Id: I7cdb73a103f9cd49767d5491812d5ad011ee5c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/496297
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
m.Path is never empty for a module in the build list; this fixes a
typo from CL 334932.
Change-Id: I5328081ba3bcf5eeac9a1b21a03969ba82ab20ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/489076
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Previously, we were only logging the top-level names of leaked
directories, which doesn't provide much information for debugging.
For #55260.
Change-Id: I845d158135d67b5d7fdeb16ab7031a061535e643
Reviewed-on: https://go-review.googlesource.com/c/go/+/479055
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
- Build the fake go1.999testpath binary from Go source instead of
special-casing a fake command on Windows.
- Skip the part of the test that uses shell scripts served from the
test GOPROXY if /bin/sh is not present.
This makes the test more expensive, but also more realistic: notably,
it does not require test hooks to determine whether to run a real or
fake binary.
Change-Id: If14fec52186631d7833eba653c91ec5198dede58
Reviewed-on: https://go-review.googlesource.com/c/go/+/486400
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fix a case where x.mode == invalid was returned despite builtin
returning true.
Change-Id: Iae9c18aac16bcbadc3530d341b380e05c8743fcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/495299
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Check all arguments for validity once, in the beginning.
Conservatively replace arg(x, i) calls with *x = args[i].
Use y (2nd arguments) directly, w/o copying.
Remove unnecessary copies and slice creations in append.
Change-Id: I1e2891cba9658f5b3cdf897e81db2f690a99b16b
Reviewed-on: https://go-review.googlesource.com/c/go/+/495515
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
This reverts CL 495596.
Reason for revert: duplicate symbol failures in x/tools and random PPC crashes.
Change-Id: I57305f8e72ee1567dc5a6a829c2d70fb5719028a
Reviewed-on: https://go-review.googlesource.com/c/go/+/496185
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This test was introduced in CL 18882, but only recently enabled as of
CL 493603. It's intended to check that we don't move executing C code
between threads when it re-enters Go, but it has always contained a
flake. Go *can* preempt between the Go call to gettid and the C call
to gettid and move the goroutine to another thread because there's no
C code on the stack during the Go call to gettid. This will cause the
test to fail.
Fix this by making both gettid calls in C, with a re-entry to Go
between them.
Fixes#60265
Change-Id: I546621a541ce52b996d68b17d3bed709d2b5b1f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/496182
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
