The check of MADD&MSUB was added to the function IsMIPSMUL in
a previous commit, and the comments should also be updated.
Change-Id: I2d3da055d55b459b908714c542dff99ab5c6cf99
Reviewed-on: https://go-review.googlesource.com/c/go/+/171102
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 170955 set tlsg to the Android Q free TLS slot offset in the linker
data (16 on amd64, 8 on 386), offsetting all TLS relative access.
We need the 0'th slot (TLS_SLOT_SELF) at initialization, so
compensate with a corresponding negative offset.
Fixes the android/386 and android/amd64 builders broken by CL 170955.
Change-Id: I9882088c0c8bc6a777d2aabc9404cb76f02b6cea
Reviewed-on: https://go-review.googlesource.com/c/go/+/170956
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
An instruction that references TLS, e.g.
MOVQ 0(TLS), AX
on some platforms (e.g. Android), or in shared mode, may be
translated to (assuming TLS offset already loaded to CX)
MOVQ 0(CX)(TLS*1), AX
which in turns translates to
movq %fs:(%rcx), %rax
We have rejected non-zero offset for TLS reference, like 16(TLS).
Actually, the instruction can take offset, i.e. it is a valid
instruction for, e.g.,
movq %fs:16(%rcx),%rcx
So, allow offset in TLS reference.
Change-Id: Iaf1996bad7fe874e0c298ea441af5acb136a4028
Reviewed-on: https://go-review.googlesource.com/c/go/+/171151
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A goroutine should be preempted if it runs for 10ms without blocking.
We found that this doesn't work for goroutines which call short system calls.
For example, the next program can stuck for seconds without this fix:
$ cat main.go
package main
import (
"runtime"
"syscall"
)
func main() {
runtime.GOMAXPROCS(1)
c := make(chan int)
go func() {
c <- 1
for {
t := syscall.Timespec{
Nsec: 300,
}
if true {
syscall.Nanosleep(&t, nil)
}
}
}()
<-c
}
$ time go run main.go
real 0m8.796s
user 0m0.367s
sys 0m0.893s
Updates #10958
Change-Id: Id3be54d3779cc28bfc8b33fe578f13778f1ae2a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/170138
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change renames the temporary directory prefix for testing to
go-testcover from gotestcover. It looks like other packages have the
"go-" prefix for temporary directories, such as go-build, go-tool-dist
and go-nettest.
Change-Id: I91ab570d33c4c1bb48e6e01451a811272f6f8b77
Reviewed-on: https://go-review.googlesource.com/c/go/+/171100
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change is mostly cosmetic.
OINDREGSP was used only for reading the results of a function call.
In recognition of that fact, rename it to ORESULT.
Along the way, trim down our handling of it to the bare minimum,
and rely on the increased clarity of ORESULT to inline nodarg.
Passes toolstash-check.
Change-Id: I25b177df4ea54a8e94b1698d044c297b7e453c64
Reviewed-on: https://go-review.googlesource.com/c/go/+/170705
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When buildmode=pie, external linking is forced, and our toolchain build id
will be included in the external build id, resulting in the building of
a toolchain tool will never reach a fixed point id.
More importantly, this change will make make.bash converge on self-hosted
Android builds (Android refuses to run non-PIE executables).
Fixes#31320
Updates #18968
Change-Id: Icb5db9f4b1b688afe37f4dafe261ffda580fa4e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/170942
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Android Q frees a static TLS slot for us to use. Use the offset of
that slot as the default for our TLS offset.
As a result, runtime/cgo is no more a requirement for Android Q and
newer.
Updates #31343
Updates #29674
Change-Id: I759049b2e2865bd3d4fdc05a8cfc6db8b0da1f5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/170955
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Android refuses to run non-PIE binaries, a restriction already
encoded in the cmd/go tool's buildModeInit function. This CL adds
the necessary flags to cmd/dist to make ./make.bash run on an
Android device.
Change-Id: I162084f573befaa41dcb47a2b78448bce5b83d35
Reviewed-on: https://go-review.googlesource.com/c/go/+/170943
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
If no other instruction mentions an inline mark, we can get rid of it.
This normally happens when the inlined function is empty, or when all
of its code is folded into other instructions.
Also use consistent statement-ness for inline mark positions, so that
more of them can be removed in favor of existing instructions.
Update #29571Fixes#31172
Change-Id: I71f84d355101f37a27960d9e8528f42f92767496
Reviewed-on: https://go-review.googlesource.com/c/go/+/170445
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Right now the mTreap structure exposes the treapNode structure through
only one interface: find. There's no reason (performance or otherwise)
for exposing this, and we get a cleaner abstraction through the
iterators this way. This change also makes it easier to make changes to
the mTreap implementation without violating its interface.
Change-Id: I5ef86b8ac81a47d05d8404df65af9ec5f419dc40
Reviewed-on: https://go-review.googlesource.com/c/go/+/164098
Reviewed-by: Austin Clements <austin@google.com>
This change just makes the code in scavengeLargest easier to reason
about by reducing the number of exit points to the method. It should
still be correct either way because the condition checked at the end
(released > nbytes) will always be false if we return, but this just
makes the code a little easier to maintain.
Change-Id: If60da7696aca3fab3b5ddfc795d600d87c988238
Reviewed-on: https://go-review.googlesource.com/c/go/+/160617
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
CL 56470 unindented bytes.Fields, but not strings.Fields. Do so now to
make it easier to diff the two functions for potential differences.
Change-Id: Ifef81f50cee64e8277e91efa5ec5521d8d21d3bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/170951
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The Modified field allows representation of extended timestamps, which provide more accuracy than the legacy MS-DOS timestamps.
The FileInfo method provides an implementation of the os.FileInfo interface for files inside archives.
With this change, we make FileInfo use the Modified field, if present, to return more detailed timestamps from its ModTime method.
Fixes#28350
Change-Id: Ia31b5b871a3e61df38a3a1325787ae23ea0b8088
GitHub-Last-Rev: 13e94be3f8
GitHub-Pull-Request: golang/go#28352
Reviewed-on: https://go-review.googlesource.com/c/go/+/144382
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Fixes the TestSplice test on Android where the default
TMPDIR (/data/local/tmp) might not be available.
Change-Id: I4f104d11254ba855b1bd2dfa0547d69b7bce4878
Reviewed-on: https://go-review.googlesource.com/c/go/+/170947
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The underlying system call tested by TestCredentialNoSetGroups
is blocked on Android.
Discovered while running all.bash from an Android device; the syscall
is only blocked in an app context.
Change-Id: I16fd2e64636a0958b0ec86820723c0577b8f8f24
Reviewed-on: https://go-review.googlesource.com/c/go/+/170945
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's not supported in an app context:
$ go test -short os
--- FAIL: TestChdirAndGetwd (0.00s)
os_test.go:1213: Open /: open /: permission denied
Change-Id: I56b951f925a50fd67715ee2f1de64951ee867e91
Reviewed-on: https://go-review.googlesource.com/c/go/+/170946
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Without this change, building an Android toolchain fails:
$ CGO_ENABLED=1 GOARCH=arm64 GOOS=android ./bootstrap.bash
...
rmdir: failed to remove 'bin/go_android_arm64_exec': Not a directory
Change-Id: Ibc3b1e2fd24b73a63bd3020ce1e813f2b4496125
Reviewed-on: https://go-review.googlesource.com/c/go/+/170941
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
They were added a very long time ago, as a convenience before Go had
struct literals. Today, it is better to use the zero-valued literal. For
example, the compiler cannot prove that ZP or ZR have not been modified.
Change-Id: I7469f1c751e91bf76fe1eab07b5772eccb5d6405
Reviewed-on: https://go-review.googlesource.com/c/go/+/171097
Reviewed-by: Nigel Tao <nigeltao@golang.org>
This opcode was only used to mark unreachable code for plive to use.
plive now uses the SSA representation, so it knows locations are
unreachable because they are ends of Exit blocks. It doesn't need
these opcodes any more.
These opcodes actually used space in the binary, 2 bytes per undef
on x86 and more for other archs.
Makes the amd64 go binary 0.2% smaller.
Change-Id: I64c84c35db7c7949617a3a5830f09c8e5fcd2620
Reviewed-on: https://go-review.googlesource.com/c/go/+/171058
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was the only benchmark missing the SetBytes call, as spotted
earlier by Bryan.
It's not required to make the benchmark useful, but it can still be a
good way to see how its speed is affected by the reduced allocations:
name time/op
CodeUnmarshal-8 12.1ms ± 1%
CodeUnmarshalReuse-8 11.4ms ± 1%
name speed
CodeUnmarshal-8 161MB/s ± 1%
CodeUnmarshalReuse-8 171MB/s ± 1%
name alloc/op
CodeUnmarshal-8 3.28MB ± 0%
CodeUnmarshalReuse-8 1.94MB ± 0%
name allocs/op
CodeUnmarshal-8 92.7k ± 0%
CodeUnmarshalReuse-8 77.6k ± 0%
While at it, remove some unnecessary empty lines.
Change-Id: Ib2bd92d5b3237b8f3092e8c6f863dab548fee2f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/170938
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, shrinkstack will free the stack if the goroutine is dead.
There are only two places that call shrinkstack: scanstack, which will
never call it if the goroutine is dead; and markrootFreeGStacks, which
only calls it on dead goroutines.
Clean this up by separating stack freeing out of shrinkstack.
Change-Id: I7d7891e620550c32a2220833923a025704986681
Reviewed-on: https://go-review.googlesource.com/c/go/+/170890
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
We've copy-pasted the pattern of releasem in many places. This CL
replaces almost everywhere that manipulates g.m.locks and g.preempt
with calls to acquirem/releasem. There are a few where we do something
more complicated, like where exitsyscall has to restore the stack
bound differently depending on the preempt flag, which this CL leaves
alone.
Change-Id: Ia7a46c261daea6e7802b80e7eb9227499f460433
Reviewed-on: https://go-review.googlesource.com/c/go/+/170064
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 3660 replaced m.gcing with m.preemptoff, but unintentionally
reversed the sense of part of a sanity check in notetsleep.
Originally, notetsleep required that it be called from g0 or with
preemption disabled (specifically from within the garbage collector).
CL 3660 made it require that it be called from g0 or that preemption
be *enabled*.
I'm not sure why it had the original exception for being called from a
user g within the garbage collector, but the current garbage collector
certainly doesn't need that, and the new condition is completely wrong.
Make the sanity check just require that it's called on g0.
Change-Id: I6980d44f5a4461935e10b1b33a981e32b1b7b0c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/170063
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, every Pool is cleared completely at the start of each GC.
This is a problem for heavy users of Pool because it causes an
allocation spike immediately after Pools are clear, which impacts both
throughput and latency.
This CL fixes this by introducing a victim cache mechanism. Instead of
clearing Pools, the victim cache is dropped and the primary cache is
moved to the victim cache. As a result, in steady-state, there are
(roughly) no new allocations, but if Pool usage drops, objects will
still be collected within two GCs (as opposed to one).
This victim cache approach also improves Pool's impact on GC dynamics.
The current approach causes all objects in Pools to be short lived.
However, if an application is in steady state and is just going to
repopulate its Pools, then these objects impact the live heap size *as
if* they were long lived. Since Pooled objects count as short lived
when computing the GC trigger and goal, but act as long lived objects
in the live heap, this causes GC to trigger too frequently. If Pooled
objects are a non-trivial portion of an application's heap, this
increases the CPU overhead of GC. The victim cache lets Pooled objects
affect the GC trigger and goal as long-lived objects.
This has no impact on Get/Put performance, but substantially reduces
the impact to the Pool user when a GC happens. PoolExpensiveNew
demonstrates this in the substantially reduction in the rate at which
the "New" function is called.
name old time/op new time/op delta
Pool-12 2.21ns ±36% 2.00ns ± 0% ~ (p=0.070 n=19+16)
PoolOverflow-12 587ns ± 1% 583ns ± 1% -0.77% (p=0.000 n=18+18)
PoolSTW-12 5.57µs ± 3% 4.52µs ± 4% -18.82% (p=0.000 n=20+19)
PoolExpensiveNew-12 3.69ms ± 7% 1.25ms ± 5% -66.25% (p=0.000 n=20+19)
name old p50-ns/STW new p50-ns/STW delta
PoolSTW-12 5.48k ± 2% 4.53k ± 2% -17.32% (p=0.000 n=20+20)
name old p95-ns/STW new p95-ns/STW delta
PoolSTW-12 6.69k ± 4% 5.13k ± 3% -23.31% (p=0.000 n=19+18)
name old GCs/op new GCs/op delta
PoolExpensiveNew-12 0.39 ± 1% 0.32 ± 2% -17.95% (p=0.000 n=18+20)
name old New/op new New/op delta
PoolExpensiveNew-12 40.0 ± 6% 12.4 ± 6% -68.91% (p=0.000 n=20+19)
(https://perf.golang.org/search?q=upload:20190311.2)
Fixes#22950.
Change-Id: If2e183d948c650417283076aacc20739682cdd70
Reviewed-on: https://go-review.googlesource.com/c/go/+/166961
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Currently, Pool stores each per-P shard's overflow in a slice
protected by a Mutex. In order to store to the overflow or steal from
another shard, a P must lock that shard's Mutex. This allows for
simple synchronization between Put and Get, but has unfortunate
consequences for clearing pools.
Pools are cleared during STW sweep termination, and hence rely on
pinning a goroutine to its P to synchronize between Get/Put and
clearing. This makes the Get/Put fast path extremely fast because it
can rely on quiescence-style coordination, which doesn't even require
atomic writes, much less locking.
The catch is that a goroutine cannot acquire a Mutex while pinned to
its P (as this could deadlock). Hence, it must drop the pin on the
slow path. But this means the slow path is not synchronized with
clearing. As a result,
1) It's difficult to reason about races between clearing and the slow
path. Furthermore, this reasoning often depends on unspecified nuances
of where preemption points can occur.
2) Clearing must zero out the pointer to every object in every Pool to
prevent a concurrent slow path from causing all objects to be
retained. Since this happens during STW, this has an O(# objects in
Pools) effect on STW time.
3) We can't implement a victim cache without making clearing even
slower.
This CL solves these problems by replacing the locked overflow slice
with a lock-free structure. This allows Gets and Puts to be pinned the
whole time they're manipulating the shards slice (Pool.local), which
eliminates the races between Get/Put and clearing. This, in turn,
eliminates the need to zero all object pointers, reducing clearing to
O(# of Pools) during STW.
In addition to significantly reducing STW impact, this also happens to
speed up the Get/Put fast-path and the slow path. It somewhat
increases the cost of PoolExpensiveNew, but we'll fix that in the next
CL.
name old time/op new time/op delta
Pool-12 3.00ns ± 0% 2.21ns ±36% -26.32% (p=0.000 n=18+19)
PoolOverflow-12 600ns ± 1% 587ns ± 1% -2.21% (p=0.000 n=16+18)
PoolSTW-12 71.0µs ± 2% 5.6µs ± 3% -92.15% (p=0.000 n=20+20)
PoolExpensiveNew-12 3.14ms ± 5% 3.69ms ± 7% +17.67% (p=0.000 n=19+20)
name old p50-ns/STW new p50-ns/STW delta
PoolSTW-12 70.7k ± 1% 5.5k ± 2% -92.25% (p=0.000 n=20+20)
name old p95-ns/STW new p95-ns/STW delta
PoolSTW-12 73.1k ± 2% 6.7k ± 4% -90.86% (p=0.000 n=18+19)
name old GCs/op new GCs/op delta
PoolExpensiveNew-12 0.38 ± 1% 0.39 ± 1% +2.07% (p=0.000 n=20+18)
name old New/op new New/op delta
PoolExpensiveNew-12 33.9 ± 6% 40.0 ± 6% +17.97% (p=0.000 n=19+20)
(https://perf.golang.org/search?q=upload:20190311.1)
Fixes#22331.
For #22950.
Change-Id: Ic5cd826e25e218f3f8256dbc4d22835c1fecb391
Reviewed-on: https://go-review.googlesource.com/c/go/+/166960
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This adds two benchmarks that will highlight two problems in Pool that
we're about to address.
The first benchmark measures the impact of large Pools on GC STW time.
Currently, STW time is O(# of items in Pools), and this benchmark
demonstrates 70µs STW times.
The second benchmark measures the impact of fully clearing all Pools
on each GC. Typically this is a problem in heavily-loaded systems
because it causes a spike in allocation. This benchmark stresses this
by simulating an expensive "New" function, so the cost of creating new
objects is reflected in the ns/op of the benchmark.
For #22950, #22331.
Change-Id: I0c8853190d23144026fa11837b6bf42adc461722
Reviewed-on: https://go-review.googlesource.com/c/go/+/166959
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This adds a dynamically sized, lock-free, single-producer,
multi-consumer queue that will be used in the new Pool stealing
implementation. It's built on top of the fixed-size queue added in the
previous CL.
For #22950, #22331.
Change-Id: Ifc0ca3895bec7e7f9289ba9fb7dd0332bf96ba5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/166958
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is the first step toward fixing multiple issues with sync.Pool.
This adds a fixed size, lock-free, single-producer, multi-consumer
queue that will be used in the new Pool stealing implementation.
For #22950, #22331.
Change-Id: I50e85e3cb83a2ee71f611ada88e7f55996504bb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/166957
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
fdopendir takes ownership of its file descriptor argument.
Getdirentries shouldn't do that, so dup the file descriptor
before passing to fdopendir.
Fixes#31269
Change-Id: Ie36be8fd6c59eb339dcc9f40228d4191fc1e5850
Reviewed-on: https://go-review.googlesource.com/c/go/+/170698
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
