If there are many goroutines contending for two different locks
and both locks hash to the same semaRoot, the scans to find the
goroutines for a particular lock can end up being O(n), making
n lock acquisitions quadratic.
As long as only one actively-used lock hashes to each semaRoot
there's no problem, since the list operations in that case are O(1).
But when the second actively-used lock hits the same semaRoot,
then scans for entries with for a given lock have to scan over the
entries for the other lock.
Fix this problem by changing the semaRoot to hold only one sudog
per unique address. In the running example, this drops the length of
that list from O(n) to 2. Then attach other goroutines waiting on the
same address to a separate list headed by the sudog in the semaRoot list.
Those "same address list" operations are still O(1), so now the
example from above works much better.
There is still an assumption here that in real programs you don't have
many many goroutines queueing up on many many distinct addresses.
If we end up with that problem, we can replace the top-level list with
a treap.
Fixes#17953.
Change-Id: I78c5b1a5053845275ab31686038aa4f6db5720b2
Reviewed-on: https://go-review.googlesource.com/36792
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change allows greatly reducing memory allocations with a slightly
performance improvement as well.
Instances of (*png).Encoder can have a optional BufferPool attached to
them. This allows reusing temporary buffers used when encoding a new
image. This buffers include instances to zlib.Writer and bufio.Writer.
Also, buffers for current and previous rows are saved in the encoder
instance and reused as long as their cap() is enough to fit the current
image row.
A new benchmark was added to demonstrate the performance improvement
when setting a BufferPool to an Encoder instance:
$ go test -bench BenchmarkEncodeGray -benchmem
BenchmarkEncodeGray-4 1000 2349584 ns/op 130.75 MB/s 852230 B/op 32 allocs/op
BenchmarkEncodeGrayWithBufferPool-4 1000 2241650 ns/op 137.04 MB/s 900 B/op 3 allocs/op
Change-Id: I4488201ae53cb2ad010c68c1e0118ee12beae14e
Reviewed-on: https://go-review.googlesource.com/34150
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make the comments a bit clearer and more accurate,
in anticipation of updating the code.
Change-Id: I1111e6c3405a8688fcd29b809a48a762ff41edaa
Reviewed-on: https://go-review.googlesource.com/36833
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add temporaries to reorder the assignment for OAS2XXX nodes.
This makes orderstmt(), rewrite
a, b, c = ...
as
tmp1, tmp2, tmp3 = ...
a, b, c = tmp1, tmp2, tmp3
and
a, ok = ...
as
t1, t2 = ...
a = t1
ok = t2
Fixes#13433.
Change-Id: Id0f5956e3a254d0a6f4b89b5f7b0e055b1f0e21f
Reviewed-on: https://go-review.googlesource.com/34713
Run-TryBot: Dhananjay Nakrani <dhananjayn@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Encourage people towards the various help forums as a first port of
call. Better sign-posting will reduce the incidence or questions being
asked in the issue tracker that should otherwise be handled elsewhere,
thereby keeping the issue tracker email traffic more focussed.
Change-Id: I13b2e498d88be010fca421067ae6fb579a46d6b7
Reviewed-on: https://go-review.googlesource.com/34250
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Using 'sep' as parameter name for strings functions that take a
separator argument is fine, but for functions like Index or Count that
look for a substring it's better to use 'substr' (like Contains
already does).
Fixes#19039
Change-Id: Idd557409c8fea64ce830ab0e3fec37d3d56a79f0
Reviewed-on: https://go-review.googlesource.com/36874
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The environment variables used in those tests override the default
OS ones. However, one of them (SystemRoot) seems to be required on
some Windows systems for invoking cmd.exe properly.
This fixes#4930 and #6568.
Change-Id: I23dfb67c1de86020711a3b59513f6adcbba12561
Reviewed-on: https://go-review.googlesource.com/36873
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: Ia2852666ef44e7ef0bba2360e92caccc83fd0e5c
Reviewed-on: https://go-review.googlesource.com/36796
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When running benchmarks with -cpuprofile,
the entire process gets profiled,
and ReadMemStats is surprisingly expensive.
Running the sort benchmarks right now with
-cpuprofile shows almost half of all execution
time in ReadMemStats.
Since ReadMemStats is not required if the benchmark
does not need allocation stats, simply skip it.
This will make cpu profiles nicer to read
and significantly speed up the process of running benchmarks.
It might also make sense to toggle cpu profiling
on/off as we begin/end individual benchmarks,
but that wouldn't get us the time savings of
skipping ReadMemStats, so this CL is useful in itself.
Change-Id: I425197b1ee11be4bc91d22b929e2caf648ebd7c5
Reviewed-on: https://go-review.googlesource.com/36791
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The 0x10101 magic constant is a little more principled than 0x10100, as
the rounding adjustment now spans the complete range [0, 0xffff] instead
of [0, 0xff00].
Consider this round-tripping code:
y, cb, cr := color.RGBToYCbCr(r0, g0, b0)
r1, g1, b1 := color.YCbCrToRGB(y, cb, cr)
Due to rounding errors both ways, we often but not always get a perfect
round trip (where r0 == r1 && g0 == g1 && b0 == b1). This is true both
before and after this commit. In some cases we got luckier, in others we
got unluckier.
For example, before this commit, (180, 135, 164) doesn't round trip
perfectly (it's off by 1) but (180, 135, 165) does. After this commit,
both cases are reversed: the former does and the latter doesn't (again
off by 1). Over all possible (r, g, b) triples, there doesn't seem to be
a big change for better or worse.
There is some history in these CLs:
image/color: tweak the YCbCr to RGBA conversion formula.
https://go-review.googlesource.com/#/c/12220/2/src/image/color/ycbcr.go
image/color: have YCbCr.RGBA work in 16-bit color, per the Color
interface.
https://go-review.googlesource.com/#/c/8073/2/src/image/color/ycbcr.go
Change-Id: Ib25ba7039f49feab2a9d1a4141b86db17db7b3e1
Reviewed-on: https://go-review.googlesource.com/36732
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Instead we can just call needwritebarrier when constructing the SSA
representation.
Change-Id: I6fefaad49daada9cdb3050f112889e49dca0047b
Reviewed-on: https://go-review.googlesource.com/34566
Reviewed-by: Cherry Zhang <cherryyz@google.com>
So it could be inlined.
Using bit-tricks it could be implemented without condition
(improved trick version by Minux Ma).
Simple benchmark shows it is faster on i386 and x86_64, though
I don't know will it be faster on other architectures?
benchmark old ns/op new ns/op delta
BenchmarkFastrand-3 2.79 1.48 -46.95%
BenchmarkFastrandHashiter-3 25.9 24.9 -3.86%
Change-Id: Ie2eb6d0f598c0bb5fac7f6ad0f8b5e3eddaa361b
Reviewed-on: https://go-review.googlesource.com/34782
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the user is calling SetGCPercent(-1), they intend to disable GC.
They probably don't intend to run one. If they do, they can call
runtime.GC themselves.
Change-Id: I40ef40dfc7e15193df9ff26159cd30e56b666f73
Reviewed-on: https://go-review.googlesource.com/34013
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
During the mark phase of garbage collection, goroutines that allocate
may be recruited to assist. This change creates trace events for mark
assists and displays them similarly to sweep assists in the trace
viewer.
Mark assists are different than sweeps in that they can be preempted, so
displaying them in the trace viewer is a little tricky -- we may need to
synthesize multiple slices for one mark assist. This could have been
done in the parser instead, but I thought it might be preferable to keep
the parser as true to the event stream as possible.
Change-Id: I381dcb1027a187a354b1858537851fa68a620ea7
Reviewed-on: https://go-review.googlesource.com/36015
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
The other expvar tests are already parallelized, and this will help to
measure the impact of potential implementations for #18177.
updates #18177
Change-Id: I0f4f1a16a0285556cbcc8339855b6459af412675
Reviewed-on: https://go-review.googlesource.com/36717
Reviewed-by: Russ Cox <rsc@golang.org>
Otherwise, calling PtrTo on the result will fail.
Fixes#19003
Change-Id: I8d7d1981a5d0417d5aee52740469d71e90734963
Reviewed-on: https://go-review.googlesource.com/36731
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The code previously tested only whether DNS-name SANs were present in a
certificate which is only approximately correct. In fact, /any/ SAN
extension, including one with no DNS names, should cause the CN to be
ignored.
Change-Id: I3d9824918975be6d4817e7cbb48ed1b0c5a2fc8b
Reviewed-on: https://go-review.googlesource.com/36696
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The s390x port was based on the ppc64 port and, because of the way the
port was done, inherited some instructions from it. ppc64 supports
3-operand (4-operand for FMADD etc.) floating point instructions
but s390x doesn't (the destination register is always an input) and
so these were emulated.
There is a bug in the emulation of FMADD whereby if the destination
register is also a source for the multiplication it will be
clobbered. This doesn't break any assembly code in the std lib but
could affect future work.
To fix this I have gone through the floating point instructions and
removed all unnecessary 3-/4-operand emulation. The compiler doesn't
need it and assembly writers don't need it, it's just a source of
bugs.
I've also deleted the FNMADD family of emulated instructions. They
aren't used anywhere.
Change-Id: Ic07cedcf141a6a3b43a0c84895460f6cfbf56c04
Reviewed-on: https://go-review.googlesource.com/33350
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Unlike the pure go implementation used by every other architecture,
the amd64 asm implementation of Exp does not fail early if the
argument is known to overflow. Make it fail early.
Cost of the check is < 1ns (on an old Sandy Bridge machine):
name old time/op new time/op delta
Exp-4 18.3ns ± 1% 18.7ns ± 1% +2.08% (p=0.000 n=18+20)
Fixes#14932Fixes#18912
Change-Id: I04b3f9b4ee853822cbdc97feade726fbe2907289
Reviewed-on: https://go-review.googlesource.com/36271
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously if a connection was requested but timed out during the
request and when acquiring the db.Lock the connection request
is fulfilled and the request is unable to be returned to the
connection pool, then then driver connection would not be closed.
No tests were added or modified because I was unable to determine
how to trigger this situation without something invasive.
Change-Id: I9d4dc680e3fdcf63d79d212174a5b8b313f363f1
Reviewed-on: https://go-review.googlesource.com/36641
Reviewed-by: Russ Cox <rsc@golang.org>
These are very tightly coupled, and internal/protopprof is small.
There's no point to having a separate package.
Change-Id: I2c8aa49c9e18a7128657bf2b05323860151b5606
Reviewed-on: https://go-review.googlesource.com/36711
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a re-roll of a previous commit,
a855da29db, which was rolled back in
14347ee480.
It was rolled back because it broke a unit test in image/gif. The
image/gif code was fixed by 9ef65dbe06
"image/gif: fix frame-inside-image bounds checking".
The original commit message:
image: fix the overlap check in Rectangle.Intersect.
The doc comment for Rectangle.Intersect clearly states, "If the two
rectangles do not overlap then the zero rectangle will be returned."
Prior to this fix, calling Intersect on adjacent but non-overlapping
rectangles would return an empty but non-zero rectangle.
The fix essentially changes
if r.Min.X > r.Max.X || r.Min.Y > r.Max.Y { etc }
to
if r.Min.X >= r.Max.X || r.Min.Y >= r.Max.Y { etc }
(note that the > signs have become >= signs), but changing that line to:
if r.Empty() { etc }
seems clearer (and equivalent).
Change-Id: I2e3af1f1686064a573b2e513b39246fe60c03631
Reviewed-on: https://go-review.googlesource.com/36734
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 35554 taught order.go to use static variables
for constants that needed to be addressable for runtime routines.
However, there is one class of runtime routines that
do not actually need an addressable value: fast map access routines.
This CL teaches order.go to avoid using static variables
for addressability in those cases.
Instead, it avoids introducing a temp at all,
which the backend would just have to optimize away.
Fixes#19015.
Change-Id: I5ef780c604fac3fb48dabb23a344435e283cb832
Reviewed-on: https://go-review.googlesource.com/36693
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The semantics of the Go image.Rectangle type is that the In and
Intersects methods treat empty rectangles specially. There are multiple
valid representations of an empty image.Rectangle. One of them is the
zero image.Rectangle but there are others. They're obviously not all
equal in the == sense, so we shouldn't use != to check GIF's semantics.
This change will allow us to re-roll
a855da29db "image: fix the overlap check
in Rectangle.Intersect" which was rolled back in
14347ee480.
Change-Id: Ie1a0d092510a7bb6170e61adbf334b21361ff9e6
Reviewed-on: https://go-review.googlesource.com/36639
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The current implementation fails to produce an "IsNotExist" error on some
platforms (unix) for certain situations where it would be expected. This causes
downstream consumers, like FileServer, to emit 500 errors instead of a 404 for
some non-existant paths on certain platforms but not others.
As an example, os.Open("/index.html/foo") on a unix-type system will return
syscall.ENOTDIR, which os.IsNotExist cannot return true for (because the
error code is ambiguous without context). On windows, this same example
would result in os.IsNotExist returning true -- since the returned error is
specific.
This change alters Dir.Open to look up the tree for an "IsPermission" or
"IsNotExist" error to return, or a non-directory, returning os.ErrNotExist in
the last case. For all other error scenarios, the original error is returned.
This ensures that downstream code, like FileServer, receive errors that behave
the same across all platforms.
Fixes#18984
Change-Id: Id7d16591c24cd96afddb6d8ae135ac78da42ed37
Reviewed-on: https://go-review.googlesource.com/36635
Reviewed-by: Yasuhiro MATSUMOTO <mattn.jp@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The gcCompat mode was introduced to match the new parser's node position
setup exactly with the positions used by the original parser. Some of the
gcCompat adjustments were required to satisfy syntax error test cases,
and the rest were required to make toolstash cmp pass.
This change removes the former gcCompat adjustments and instead adjusts
the respective test cases as necessary. In some cases this makes the error
lines consistent with the ones reported by gccgo.
Where it has changed, the position associated with a given syntactic construct
is the position (line/col number) of the left-most token belonging to the
construct.
Change-Id: I5b60c00c5999a895c4d6d6e9b383c6405ccf725c
Reviewed-on: https://go-review.googlesource.com/36695
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This change contains a very minor tidy-up to a test.
Change-Id: I3a8c0168bcdcbf90cacbbac2566c8423c92129f8
Reviewed-on: https://go-review.googlesource.com/33726
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
go:systemstack works by tweaking the stack check prologue to check
against a different bound, while go:nosplit removes the stack check
prologue entirely. Hence, they can't be used together. Make the build
fail if they are.
Change-Id: I2d180c4b1d31ff49ec193291ecdd42921d253359
Reviewed-on: https://go-review.googlesource.com/36710
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Recently, a commit (85ecc51c) changed the instruction from VORL to VOR.
Fixes#19014
Change-Id: I9a7e0b5771842b1abb5afc73dc41d5e7960cf390
Reviewed-on: https://go-review.googlesource.com/36625
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestRPC writes to newServer and newServerAddr guarded with a
sync.Once.
TestAcceptExitAfterListenerClose was overwriting those variables,
which caused the second invocation of TestRPC within a single process
to fail.
A second invocation can occur as a result of running the test with
multiple values for the -cpu flag.
fixes#19001.
Change-Id: I291bacf44aefb49c2264ca0290a28248c026f80e
Reviewed-on: https://go-review.googlesource.com/36624
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Add asm implementation for AES in order to make use of VMX cryptographic
acceleration instructions for POWER8. There is a speed boost of over 10
times using those instructions:
Fixes#18076
old ns/op new ns/op delta
BenchmarkEncrypt-20 337 30.3 -91.00%
BenchmarkDecrypt-20 347 30.5a -91.21%
BenchmarkExpand-20 1180 130 -88.98%
old MB/s new MB/s speedup
BenchmarkEncrypt-20 47.38 527.68 11.13x
BenchmarkDecrypt-20 46.05 524.45 11.38x
Change-Id: Ifa4d1b508f4803cc72dcaad97acc8495d651b019
Reviewed-on: https://go-review.googlesource.com/33587
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
There's no need to hold the handshake lock across this call and it can
lead to deadlocks if the net.Conn calls back into the tls.Conn.
Fixes#18426.
Change-Id: Ib1b2813cce385949d970f8ad2e52cfbd1390e624
Reviewed-on: https://go-review.googlesource.com/36561
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently there are cases where an XOR with -1 followed by an AND
is generanted when it could be done with just an ANDN instruction.
Changes to PPC64.rules and required files allows this change
in generated code. Examples of this occur in sha3 among others.
Fixes: #18918
Change-Id: I647cb9b4a4aaeebb27db85f8bf75487d78f720c9
Reviewed-on: https://go-review.googlesource.com/36218
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
This ensures that SIGPROF is handled correctly when using
runtime/pprof in a c-archive or c-shared library.
Separate profiler handling into pre-process changes and per-thread
changes. Simplify the Windows code slightly accordingly.
Fixes#18220.
Change-Id: I5060f7084c91ef0bbe797848978bdc527c312777
Reviewed-on: https://go-review.googlesource.com/34018
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
The AuthorityKeyId value from the template was used by
CreateCertificate, but that wasn't documented. Also, CreateCertificate
would stash a value in the template if it needed to override it, which
was wrong: template should be read-only.
Fixes#18962.
Change-Id: Ida15c54c341e5bbf553756e8aa65021d8085f453
Reviewed-on: https://go-review.googlesource.com/36556
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
