runtime: sweep increased allocation count #21297
Comments
heschi
added
the
NeedsInvestigation
|
CC @aclements |
|
Current analysis suggests a bug in https://golang.org/cl/38091. That adds a fast path for map assignment, and uses it for pointers among other values. When a pointer is stored in the map, no write barrier is used. |
|
Change https://golang.org/cl/53413 mentions this issue: |
|
Change https://golang.org/cl/53414 mentions this issue: |
|
I have another fix which just uses typedmemmove to write the keys. It's just a 2-line change. |
|
FWIW, I've encountered a similar crash with https://golang.org/cl/38091 seems to have been committed in March, so after go 1.8 was released. |
|
@andreimatei If you are seeing a problem with Go 1.8, then it is a different problem. Please open a new issue with reproduction instructions. Thanks. |
|
Change https://golang.org/cl/59110 mentions this issue: |
Prior to this change, we use typedmemmove to write the key value to its new location in mapassign_fast32 and mapassign_fast64. (The use of typedmemmove was a last-minute fix in the 1.9 cycle; see #21297 and CL 53414.) This is significantly less inefficient than direct assignment or calling writebarrierptr directly. Fortunately, there aren't many cases to consider. On systems with 32 bit pointers: * A 32 bit AMEM value either is a single pointer or has no pointers. * A 64 bit AMEM value may contain a pointer at the beginning, a pointer at 32 bits, or two pointers. On systems with 64 bit pointers: * A 32 bit AMEM value contains no pointers. * A 64 bit AMEM value either is a single pointer or has no pointers. All combinations except the 32 bit pointers / 64 bit AMEM value are cheap and easy to handle, and the problematic case is likely rare. The most popular map keys appear to be ints and pointers. So we handle them exhaustively. The sys.PtrSize checks are constant branches and are eliminated by the compiler. An alternative fix would be to return a pointer to the key, and have the calling code do the assignment, at which point the compiler would have full type information. Initial tests suggest that the performance difference between these strategies is negligible, and this fix is considerably simpler, and has much less impact on binary size. Fixes #21321 Change-Id: Ib03200e89e2324dd3c76d041131447df66f22bfe Reviewed-on: https://go-review.googlesource.com/59110 Run-TryBot: Josh Bleecher Snyder <[email protected]> Reviewed-by: Austin Clements <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
We have reports inside Google of crashes like:
I've reproduced the problem and I have some confidence that it's a runtime or compiler bug, so I wanted to make sure that we had an external issue to represent the problem and track it as a release blocker. I'll update this as soon as we have detail that can be shared publicly.
The text was updated successfully, but these errors were encountered: