Assignments of 2-, 3-, and 4-word values were handled
by individual MOV instructions (and for scalars still are).
But if there are pointers involved, those assignments now
go through the write barrier routine. Before this CL, they
went to writebarrierfat, which calls memmove.
Memmove is too much overhead for these small
amounts of data.
Instead, call writebarrierfat{2,3,4}, which are specialized
for the specific amount of data being copied.
Today the write barrier does not care which words are
pointers, so size alone is enough to distinguish the cases.
If we keep these distinctions in Go 1.5 we will need to
expand them for all the pointer-vs-scalar possibilities,
so the current 3 functions will become 3+7+15 = 25,
still not a large burden (we deleted more morestack
functions than that when we dropped segmented stacks).
BenchmarkBinaryTree17 3250972583 3123910344 -3.91%
BenchmarkFannkuch11 3067605223 2964737839 -3.35%
BenchmarkFmtFprintfEmpty 101 96.0 -4.95%
BenchmarkFmtFprintfString 267 235 -11.99%
BenchmarkFmtFprintfInt 261 253 -3.07%
BenchmarkFmtFprintfIntInt 444 402 -9.46%
BenchmarkFmtFprintfPrefixedInt 374 346 -7.49%
BenchmarkFmtFprintfFloat 472 449 -4.87%
BenchmarkFmtManyArgs 1537 1476 -3.97%
BenchmarkGobDecode 13986528 12432985 -11.11%
BenchmarkGobEncode 13120323 12537420 -4.44%
BenchmarkGzip 451925758 437500578 -3.19%
BenchmarkGunzip 113267612 110053644 -2.84%
BenchmarkHTTPClientServer 103151 77100 -25.26%
BenchmarkJSONEncode 25002733 23435278 -6.27%
BenchmarkJSONDecode 94213717 82568789 -12.36%
BenchmarkMandelbrot200 4804246 4713070 -1.90%
BenchmarkGoParse 4646114 4379456 -5.74%
BenchmarkRegexpMatchEasy0_32 163 158 -3.07%
BenchmarkRegexpMatchEasy0_1K 433 391 -9.70%
BenchmarkRegexpMatchEasy1_32 154 138 -10.39%
BenchmarkRegexpMatchEasy1_1K 1481 1132 -23.57%
BenchmarkRegexpMatchMedium_32 282 270 -4.26%
BenchmarkRegexpMatchMedium_1K 92421 86149 -6.79%
BenchmarkRegexpMatchHard_32 5209 4718 -9.43%
BenchmarkRegexpMatchHard_1K 158141 147921 -6.46%
BenchmarkRevcomp 699818791 642222464 -8.23%
BenchmarkTemplate 132402383 108269713 -18.23%
BenchmarkTimeParse 509 478 -6.09%
BenchmarkTimeFormat 462 456 -1.30%
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/156200043
In [1] the behaviour of encoding/asn1 with respect to marshaling
optional integers was changed. Previously, a zero valued integer would
be omitted when marshaling. After the change, if a default value was
set then the integer would only be omitted if it was the default value.
This changed the behaviour of crypto/x509 because
Certificate.MaxPathLen has a default value of -1 and thus zero valued
MaxPathLens would no longer be omitted when marshaling. This is
arguably a bug-fix -- a value of zero for MaxPathLen is valid and
meaningful and now could be expressed. However it broke users
(including Docker) who were not setting MaxPathLen at all.
This change again causes a zero-valued MaxPathLen to be omitted and
introduces a ZeroMathPathLen member that indicates that, yes, one
really does want a zero. This is ugly, but we value not breaking users.
[1] https://code.google.com/p/go/source/detail?r=4218b3544610e8d9771b89126553177e32687adf
LGTM=rsc
R=rsc
CC=golang-codereviews, golang-dev
https://golang.org/cl/153420045
This error is returned by lib9p when removing a file
without parent. It should fix TestRemoveAllRace
when running on ramfs.
LGTM=bradfitz, aram
R=rsc, bradfitz, aram
CC=golang-codereviews, mischief
https://golang.org/cl/153410044
The code for a generated type is already generating an
unrolled GC bitmask. Rather than unrolling the the source
type bitmasks and copying them, just generate the required
bitmask directly. Don't mark it as an unrolled GC program,
since there is no need to do so.
Fixes#8917.
LGTM=rsc
R=dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/156930044
Our current pe object reader assumes that every symbol starting with
'.' is section. It appeared to be true, until now gcc 4.9.1 generates
some symbols with '.' at the front. Change that logic to check other
symbol fields in addition to checking for '.'. I am not an expert
here, but it seems reasonable to me.
Added test, but it is only good, if tested with gcc 4.9.1. Otherwise
the test PASSes regardless.
Fixes#8811.
Fixes#8856.
LGTM=jfrederich, iant, stephen.gutekanst
R=golang-codereviews, jfrederich, stephen.gutekanst, iant
CC=alex.brainman, golang-codereviews
https://golang.org/cl/152410043
gcc 4.9.1 generates pe sections with names longer then 8 charters.
From IMAGE_SECTION_HEADER definition:
Name
An 8-byte, null-padded UTF-8 string. There is no terminating null character
if the string is exactly eight characters long. For longer names, this
member contains a forward slash (/) followed by an ASCII representation
of a decimal number that is an offset into the string table.
Our current pe object file reader does not read string table when section
names starts with /. Do that, so (issue 8811 example)
c:\go\path\src\isssue8811>go build
# isssue8811
isssue8811/glfw(.text): isssue8811/glfw(/76): not defined
isssue8811/glfw(.text): undefined: isssue8811/glfw(/76)
becomes
c:\go\path\src\isssue8811>go build
# isssue8811
isssue8811/glfw(.text): isssue8811/glfw(.rdata$.refptr._glfwInitialized): not defined
isssue8811/glfw(.text): undefined: isssue8811/glfw(.rdata$.refptr._glfwInitialized)
Small progress to
Update #8811
LGTM=iant, jfrederich
R=golang-codereviews, iant, jfrederich
CC=golang-codereviews
https://golang.org/cl/154210044
I diffed the output of `nm -n gofmt' before and after this change,
and verified that all changes are correct and all corrupted symbol
names are fixed.
Fixes#8906.
LGTM=iant, cookieo9
R=golang-codereviews, iant, cookieo9
CC=golang-codereviews
https://golang.org/cl/159750043
https://golang.org/cl/153770043/ tried to fix the case where a
implicitly tagged Time, that happened to have the same tag as
GENERALIZEDTIME, shouldn't be parsed as a GENERALIZEDTIME.
It did so, mistakenly, by testing whether params.tag != nil. But
explicitly tagged values also have a non-nil tag and there the inner
tag actually does encode the type of the value.
This change instead tests whether the tag class is UNIVERSAL before
assuming that the tag contains type information.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/152380044
That was complete failure - builders are broken,
but original cl worked fine on my system.
I will need access to builders
to test this change properly.
««« original CL description
runtime: handle all windows exception
Fixes#8006.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
»»»
TBR=rsc
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/154180043
In channels, zeroing of gp.waiting is missed on a closed channel panic.
m.morebuf.g is not zeroed.
I don't expect the latter causes any problems, but just in case.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/151610043
TestMakeFuncVariadic only called the variadic function via Call and
CallSlice, not via a direct function call.
I thought these tests would fail under gccgo tip, but they don't. Still seems worth having though.
LGTM=iant
R=golang-codereviews, gobot, iant
CC=golang-codereviews
https://golang.org/cl/152060043
This change prevents confusion in the garbage collector.
The collector wants to make sure that every pointer it finds
isn't junk. Its criteria for junk is (among others) points
to a "free" span.
Because the stack shrinker modifies pointers in the heap,
there is a race condition between the GC scanner and the
shrinker. The GC scanner can see old pointers (pointers to
freed stacks). In particular this happens with SudoG.elem
pointers.
Normally this is not a problem, as pointers into stack spans
are ok. But if the freed stack is the last one in its span,
the span is marked as "free" instead of "contains stacks".
This change makes sure that even if the GC scanner sees
an old pointer, the span into which it points is still
marked as "contains stacks", and thus the GC doesn't
complain about it.
This change will make the GC pause a tiny bit slower, as
the stack freeing now happens in serial with the mark pause.
We could delay the freeing until the mutators start back up,
but this is the simplest change for now.
TBR=dvyukov
CC=golang-codereviews
https://golang.org/cl/158750043
These tests fail when using gccgo. In gccgo using Interface
on the value of a method function is implemented using a
variant of MakeFunc. That approach did not correctly handle
variadic functions.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/151280043
The change contains 3 spot optimizations to scan loop:
1. Don't use byte vars, use uintptr's instead.
This seems to alleviate some codegen issue,
and alone accounts to a half of speedup.
2. Remove bitmap cache. Currently we cache only 1 byte,
so caching is not particularly effective anyway.
Removal of the cache simplifies code and positively affects regalloc.
3. Replace BitsMultiword switch with if and
do debug checks only in Debug mode.
I've benchmarked changes separately and ensured that
each of them provides speedup on top of the previous one.
This change as a whole fixes the unintentional regressions
of scan loop that were introduced during development cycle.
Fixes#8625.
Fixes#8565.
On go.benchmarks/garbage benchmark:
GOMAXPROCS=1
time: -3.13%
cputime: -3.22%
gc-pause-one: -15.71%
gc-pause-total: -15.71%
GOMAXPROCS=32
time: -1.96%
cputime: -4.43%
gc-pause-one: -6.22%
gc-pause-total: -6.22%
LGTM=khr, rsc
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/153990043
Out of stack space due to new 2-word call in freedefer.
Go back to smaller function calls.
TBR=brainman
CC=golang-codereviews
https://golang.org/cl/152340043
It appears to be an opaque bit pattern more than a pointer.
The Go garbage collector has discovered that for m0
it is set to 0x4c.
Should fix Windows build.
TBR=brainman
CC=golang-codereviews
https://golang.org/cl/149640043
Another dangling stack pointer in a cached structure.
Same as SudoG.elem and SudoG.selectdone.
Definitely a fix, and the new test in freedefer makes the
crash reproducible, but probably not a complete fix.
I have seen one dangling pointer in a Defer.panic even
after this fix; I cannot see where it could be coming from.
I think this will fix the solaris build.
I do not think this will fix the occasional failure on the darwin build.
TBR=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/155080043
This avoids a pop-up box on OS X and it avoids
a test failure if something is using 5555.
I apologize for not noticing this during the review.
TBR=r
CC=golang-codereviews
https://golang.org/cl/152320044
It seems reasonable that people might want to look up the
ImportComment with "go list".
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/143600043
I have a CL which at every gc looks through data and bss
sections for nonpointer data (according to gc maps) that
looks like a pointer. These are potential missing roots.
The only thing it finds are begnign, storing stack pointers
into m0.scalararg[1] and never cleaning them up. Let's
clean them up now so the test CL passes all.bash cleanly.
The test CL can't be checked in because we might store
pointer-looking things in nonpointer data by accident.
LGTM=iant
R=golang-codereviews, iant, khr
CC=golang-codereviews
https://golang.org/cl/153210043
We no longer have full type information in the heap, so
we can't dump that any more. Instead we dump ptr/noptr
maps so at least we can compute graph connectivity.
In addition, we still dump Iface/Eface types so together
with dwarf type info we might be able to reconstruct
types of most things in the heap.
LGTM=dvyukov
R=golang-codereviews, dvyukov, rsc, khr
CC=golang-codereviews
https://golang.org/cl/155940043
This will help find bugs during the release freeze.
It's not clear it should be kept for the release itself.
That's issue 8861.
The most likely thing that would trigger this is stale
pointers that previously were ignored or caused memory
leaks. These were allowed due to the use of conservative
collection. Now that everything is precise, we should not
see them anymore.
The small number check reinforces what the stack copier
is already doing, catching the storage of integers in pointers.
It caught issue 8864.
The check is disabled if _cgo_allocate is linked into the binary,
which is to say if the binary is using SWIG to allocate untyped
Go memory. In that case, there are invalid pointers and there's
nothing we can do about it.
LGTM=rlh
R=golang-codereviews, dvyukov, rlh
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/148470043
Depending on flags&KindGCProg,
gc[0] and gc[1] are either pointers or inlined bitmap bits.
That's not compatible with a precise garbage collector:
it needs to be always pointers or never pointers.
Change the inlined bitmap case to store a pointer to an
out-of-line bitmap in gc[0]. The out-of-line bitmaps are
dedup'ed, so that for example all pointer types share the
same out-of-line bitmap.
Fixes#8864.
LGTM=r
R=golang-codereviews, dvyukov, r
CC=golang-codereviews, iant, khr, rlh
https://golang.org/cl/155820043
http.Client calls URL.String() to fill in the Referer header, which may
contain authentication info. This patch removes authentication info from
the Referer header without introducing any API changes.
A new test for net/http is also provided.
This is the polished version of Alberto García Hierro's
https://golang.org/cl/9766046/
It should handle https Referer right.
Fixes#8417
LGTM=bradfitz
R=golang-codereviews, gobot, bradfitz, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/151430043
