Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/race: broken on windows after 128 GB memory change #4379

Closed
rsc opened this issue Nov 13, 2012 · 12 comments
Closed

runtime/race: broken on windows after 128 GB memory change #4379

rsc opened this issue Nov 13, 2012 · 12 comments
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Nov 13, 2012

My CL that expanded the maximum runtime memory to 128 GB appears to have broken race
detection on the Windows build machine. I will disable the test for now so that the
build machine stays useful.
@dvyukov
Copy link
Member

dvyukov commented Nov 14, 2012

Comment 1:

The problem seems to be with bigger span lookup table:
    // span lookup
    MSpan *map[1<<MHeapMap_Bits];
It now takes 256MB, so tsan wants to map 1GB of memory.
I can simply not map shadow memory for mheap, it should not be accessed from
instrumented code.
However, if the machine rejects to map 1GB of memory, it means that it will reject to
run any 4 Go binaries simultaneously. It may also be confusing for some people/scripts
to see 256MB of memory mapped memory for a tiny executable.
Another option is to map the map lazily at known position in memory (e.g. 0xf000000000).
Then it will be invisible to the race detector.

@dvyukov
Copy link
Member

dvyukov commented Nov 16, 2012

Comment 2:

This issue was closed by revision 8f82bff.

Status changed to Fixed.

@dvyukov
Copy link
Member

dvyukov commented Nov 16, 2012

Comment 3:

Indeed, it seems that now it's just broken unrelated to race detector:
ok      net/textproto   0.024s
ok      net/url 0.029s
ok      old/netchan 0.049s
ok      os  0.141s
--- FAIL: TestEcho (0.00 seconds)
exec_test.go:34:    echo: fork/exec
C:\Users\ADMINI~1\AppData\Local\Temp\2\go-build804797863\os\exec\_test\exec.test.exe:
The paging file is too small for this operation to complete.
exec_test.go:37:    echo: want "foo bar baz\n", got ""
FAIL
FAIL    os/exec 0.141s
--- FAIL: TestCtrlBreak (0.00 seconds)
signal_windows_test.go:73:  Failed to compile: fork/exec
c:\gobuilder\windows-amd64-f440e65f93fe\go\bin\go.exe: The paging file is too small for
this operation to complete.
FAIL
FAIL    os/signal   0.029s
ok      os/user 22.647s
ok      path    0.031s
ok      path/filepath   2.820s
ok      reflect 0.051s
http://build.golang.org/log/62e782a1093e025342a4a333b1c8dc48af1eb999

Status changed to Accepted.

@dvyukov
Copy link
Member

dvyukov commented Nov 16, 2012

Comment 4:

We can't pre-allocate 1GB for each Go process on Windows. We need to lazily allocate
mheap. WDYT?

@dvyukov
Copy link
Member

dvyukov commented Nov 16, 2012

Comment 5:

Sorry, 256MB.

@minux
Copy link
Member

minux commented Nov 16, 2012

Comment 6:

a similar problem exists for netbsd/amd64.
related discussions:
https://golang.org/cl/6846061/
https://groups.google.com/group/golang-dev/browse_thread/thread/48598af2cc4d599e/189563b6166b7c9b
i guess you also support the dynamic allocation of the mheap struct solution?

@alexbrainman
Copy link
Member

Comment 7:

Our builder is running in a VM. I am not certain, but as far as I remember, it only has
1gb of ram. I do not know how big the page file is. It has been a while since I have
fiddle with windows memory, but perhaps it is just running out of memory. I will
investigate more when I have a chance.
Alex

@alexbrainman
Copy link
Member

Comment 8:

Build succeeds on my computer, so it is something on our builder. I will investigate
when I have time.
Alex

@alexbrainman
Copy link
Member

Comment 9:

Builders page file size was "system managed", whatever that means. I have changed it to
start with 2G and grow up to 4G max. I had to restart the system to do that. Lets see
what difference it makes. I am bit concerned that we need to fiddle with the system to
run our builder. I hope it is new tests, not our build tools or new standard lib changes
that are the reason. Perhaps, I worry too much.
Alex

@alexbrainman
Copy link
Member

Comment 10:

dbf92f38bae0 build succeeded. I think we are done here. Dmitry, I will leave it for you
to close, unless you think there is any improvements can be done.
Alex

@dvyukov
Copy link
Member

dvyukov commented Nov 17, 2012

Comment 11:

I think there is still a serious problem if Go program do not run by default on Windows
and NetBSD.
>i guess you also support the dynamic allocation of the mheap struct solution?
I don't know what exactly do you mean by "dynamic allocation", but we must be careful
here because it's used in the inner loop of GC. Most likely it's unacceptable to just
make it a pointer (unless we are sure than GC loop caches it in a register and it does
not affect performance).

@minux
Copy link
Member

minux commented Nov 26, 2012

Comment 12:

Status changed to Duplicate.

Merged into issue #4447.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants