Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(239)

Issue 65820044: code review 65820044: cmd/gc, runtime: enable precisestack by default (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 4 months ago by rsc
Modified:
11 years, 4 months ago
Reviewers:
r, gobot, dave
CC:
r, golang-codereviews
Visibility:
Public.

Description

cmd/gc, runtime: enable precisestack by default [Repeat of CL 64100044, after 32-bit fix in CL 66170043.] Precisestack makes stack collection completely precise, in the sense that there are no "used and not set" errors in the collection of stack frames, no times where the collector reads a pointer from a stack word that has not actually been initialized with a pointer (possibly a nil pointer) in that function. The most important part is interfaces: precisestack means that if reading an interface value, the interface value is guaranteed to be initialized, meaning that the type word can be relied upon to be either nil or a valid interface type word describing the data word. This requires additional zeroing of certain values on the stack on entry, which right now costs about 5% overall execution time in all.bash. That cost will come down before Go 1.3 (issue 7345). There are at least two known garbage collector bugs right now, issues 7343 and 7344. The first happens even without precisestack. The second I have only seen with precisestack, but that does not mean that precisestack is what causes it. In fact it is very difficult to explain by what precisestack does directly. Precisestack may be exacerbating an existing problem. Both of those issues are marked for Go 1.3 as well. The reasons for enabling precisestack now are to give it more time to soak and because the copying stack work depends on it.

Patch Set 1 #

Patch Set 2 : diff -r 19355fdfb5aa https://code.google.com/p/go/ #

Patch Set 3 : diff -r 56b477f94d42 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -3 lines) Patch
M src/cmd/gc/lex.c View 1 2 chunks +4 lines, -2 lines 0 comments Download
M src/pkg/runtime/proc.c View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5
rsc
Hello r (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
11 years, 4 months ago (2014-02-19 20:57:42 UTC) #1
r
LGTM
11 years, 4 months ago (2014-02-19 21:36:52 UTC) #2
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=168fe6a1dda1 *** cmd/gc, runtime: enable precisestack by default [Repeat of CL 64100044, ...
11 years, 4 months ago (2014-02-19 22:09:11 UTC) #3
gobot
This CL appears to have broken the linux-arm-panda builder.
11 years, 4 months ago (2014-02-19 23:02:29 UTC) #4
dave_cheney.net
11 years, 4 months ago (2014-02-19 23:40:59 UTC) #5
What the ...

--- FAIL: TestCgoCrashHandler-4 (0.51 seconds)
	crash_test.go:80: output:
		load cmd/cgo: package cmd/cgo: case-insensitive import collision:
"bytes" and "bytes"



On Thu, Feb 20, 2014 at 10:02 AM, <gobot@golang.org> wrote:

> This CL appears to have broken the linux-arm-panda builder.
>
>
> https://codereview.appspot.com/65820044/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b