- 09 Apr, 2019 6 commits
-
-
Tobias Klauser authored
As noted by Brad in CL 170954 for package bytes. Change-Id: I2772a356299e54ba5b7884d537e6649039adb9be Reviewed-on: https://go-review.googlesource.com/c/go/+/171198 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Andrei Vagin authored
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/+/170138Reviewed-by: Dmitry Vyukov <dvyukov@google.com> Run-TryBot: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Tobias Klauser authored
Follow what CL 68370 and CL 76470 did for the respective functions in package strings. Also adjust godoc strings to match the respective strings functions and mention the special case for ASCII-only byte slices which don't need conversion. name old time/op new time/op delta ToUpper/#00-8 9.35ns ± 3% 6.08ns ± 2% -35.04% (p=0.000 n=9+9) ToUpper/ONLYUPPER-8 77.7ns ± 1% 16.9ns ± 2% -78.22% (p=0.000 n=10+10) ToUpper/abc-8 36.5ns ± 1% 22.1ns ± 1% -39.43% (p=0.000 n=10+8) ToUpper/AbC123-8 56.9ns ± 2% 28.2ns ± 2% -50.54% (p=0.000 n=8+10) ToUpper/azAZ09_-8 62.3ns ± 1% 26.9ns ± 1% -56.82% (p=0.000 n=9+10) ToUpper/longStrinGwitHmixofsmaLLandcAps-8 219ns ± 2% 63ns ± 2% -71.17% (p=0.000 n=10+10) ToUpper/longɐstringɐwithɐnonasciiⱯchars-8 367ns ± 2% 374ns ± 3% +2.05% (p=0.000 n=9+10) ToUpper/ɐɐɐɐɐ-8 200ns ± 1% 206ns ± 1% +2.49% (p=0.000 n=10+10) ToUpper/a\u0080\U0010ffff-8 90.4ns ± 1% 93.8ns ± 0% +3.82% (p=0.000 n=10+7) ToLower/#00-8 9.59ns ± 1% 6.13ns ± 2% -36.08% (p=0.000 n=10+10) ToLower/abc-8 36.4ns ± 1% 10.4ns ± 1% -71.50% (p=0.000 n=10+10) ToLower/AbC123-8 55.8ns ± 1% 27.5ns ± 1% -50.61% (p=0.000 n=10+10) ToLower/azAZ09_-8 61.7ns ± 1% 30.2ns ± 1% -50.98% (p=0.000 n=8+10) ToLower/longStrinGwitHmixofsmaLLandcAps-8 226ns ± 1% 64ns ± 1% -71.53% (p=0.000 n=10+9) ToLower/LONGⱯSTRINGⱯWITHⱯNONASCIIⱯCHARS-8 354ns ± 0% 361ns ± 0% +2.18% (p=0.000 n=10+10) ToLower/ⱭⱭⱭⱭⱭ-8 180ns ± 1% 186ns ± 0% +3.45% (p=0.000 n=10+9) ToLower/A\u0080\U0010ffff-8 91.7ns ± 0% 94.5ns ± 0% +2.99% (p=0.000 n=10+10) Change-Id: Ifdb8ae328ff9feacd1c170db8eebbf98c399e204 Reviewed-on: https://go-review.googlesource.com/c/go/+/170954 Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Mikio Hara authored
Change-Id: Ib46f1a528e16cd0c2617defbf4dcd1f1b582cdc2 Reviewed-on: https://go-review.googlesource.com/c/go/+/171101 Run-TryBot: Mikio Hara <mikioh.public.networking@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Mikio Hara authored
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/+/171100Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Neven Sajko authored
Assembly files with "/vendor/" or "testdata" in their paths were ignored. Change-Id: I3882ff07eb4426abb9f8ee96f82dff73c81cd61f GitHub-Last-Rev: 51ae8c324d72a12a059272fcf8568e670bfaf21b GitHub-Pull-Request: golang/go#31166 Reviewed-on: https://go-review.googlesource.com/c/go/+/170197Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
- 08 Apr, 2019 17 commits
-
-
Josh Bleecher Snyder authored
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>
-
Elias Naur authored
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>
-
Elias Naur authored
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>
-
Andrew Bonventre authored
Change-Id: I6dd60ea7b8a8756be97503e163c2386af16e4e12 Reviewed-on: https://go-review.googlesource.com/c/go/+/171144Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Andrew Bonventre authored
Change-Id: I09e0c2720ec0a51dc73c24b4550a749448656025 Reviewed-on: https://go-review.googlesource.com/c/go/+/171143Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Andrew Bonventre authored
Change-Id: Ia06f7005f466e55a22c76bf2b47d74ee8eb77dd1 Reviewed-on: https://go-review.googlesource.com/c/go/+/171139Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Andrew Bonventre authored
Change-Id: I84c9a8ddbd3101dd478e4a8a4e191863e68c6af6 Reviewed-on: https://go-review.googlesource.com/c/go/+/171140Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Jingnan Si authored
Windows requires space for four pointers on the stack. Change-Id: I9f7ba3e09b6c660f86d15139bb51954fffc8ccb1 GitHub-Last-Rev: 76d21bcc2b07edfde6daa45000093d070e2337bc GitHub-Pull-Request: golang/go#30944 Reviewed-on: https://go-review.googlesource.com/c/go/+/168351Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Elias Naur authored
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>
-
Elias Naur authored
Adb exec-out is like adb shell except non-flaky in non-interactive settings. Don't ask why. Change-Id: I7ac3c72912883d80bc787c1d0fc101db6bae9c52 Reviewed-on: https://go-review.googlesource.com/c/go/+/170952 Run-TryBot: Elias Naur <mail@eliasnaur.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Daniel Theophanes authored
Fixes #31296 Change-Id: Ib8850fe22749ca0bf268614ba045ffe3fc68f5cc Reviewed-on: https://go-review.googlesource.com/c/go/+/171057 Run-TryBot: Daniel Theophanes <kardianos@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
-
Keith Randall authored
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 #29571 Fixes #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>
-
Michael Anthony Knyszek authored
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/+/164098Reviewed-by: Austin Clements <austin@google.com>
-
Michael Anthony Knyszek authored
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>
-
Tobias Klauser authored
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>
-
Jannis Andrija Schnitzer authored
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: 13e94be3f8ba58717911354146670fc2bc594692 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>
-
Elias Naur authored
The setgid syscall is blocked on Android in app context. Change-Id: I1ff25840bbc25d472ad4e29eb1b51f183a6c4392 Reviewed-on: https://go-review.googlesource.com/c/go/+/170949 Run-TryBot: Elias Naur <mail@eliasnaur.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
- 07 Apr, 2019 6 commits
-
-
Elias Naur authored
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>
-
Elias Naur authored
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>
-
Elias Naur authored
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>
-
Elias Naur authored
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>
-
Nigel Tao authored
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/+/171097Reviewed-by: Nigel Tao <nigeltao@golang.org>
-
Keith Randall authored
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>
-
- 05 Apr, 2019 11 commits
-
-
Daniel Martí authored
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>
-
Keith Randall authored
Update #31269 Change-Id: I0e7184420055b8dfd23688dab9f9d8cba1fa2485 Reviewed-on: https://go-review.googlesource.com/c/go/+/170892 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Austin Clements authored
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>
-
Andrew Bonventre authored
Change-Id: Iec5e69b3ea163f42234d3b73696427a7aa8732e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/170884Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-