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/pprof: disable profiling test on Windows too? #13871

Closed
bradfitz opened this issue Jan 8, 2016 · 5 comments
Closed

runtime/pprof: disable profiling test on Windows too? #13871

bradfitz opened this issue Jan 8, 2016 · 5 comments

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jan 8, 2016

It appears that Windows also can't do profiles reliably?

https://storage.googleapis.com/go-build-log/5b3db773/windows-amd64-gce_15b35da7.log

--- FAIL: TestStackBarrierProfiling (0.83s)
    pprof_test.go:363: subprocess failed with exit status 1:
        --- FAIL: TestStackBarrierProfiling (0.45s)
            pprof_test.go:97: profile too short: [0x0 0x3 0x0 0x2710 0x0 0x0 0x1 0x0]
        FAIL
FAIL
FAIL    runtime/pprof   2.519s
ok      runtime/trace   6.002s

(part of #13841)

@bradfitz bradfitz added this to the Go1.6 milestone Jan 8, 2016
@aclements
Copy link
Member

Windows is known to have reduced timer resolution, though it can't have the common Unix problem of only delivering profiling signals after a full time slice because the Windows APIs couldn't express a bug like that.

I don't see any Windows profiling failures on the dashboard. Could this be related to whatever CL produced that failure?

@bradfitz
Copy link
Contributor Author

bradfitz commented Jan 8, 2016

If you paste the git rev at the top of that failure log into Gerrit, Gerrit redirects you to the CL it was part of: https://go-review.googlesource.com/#/c/18374/ (an HTTP header validation change)

So, no... it was a flake.

@alexbrainman
Copy link
Member

Go profiler is not using signal on Windows. It is running on a separate dedicated OS thread. The thread is running with THREAD_PRIORITY_HIGHEST to make it as reliable as possible, but ... Also the thread is only started on demand - maybe there is some latency to that when CPU is busy.

Alex

@alexbrainman
Copy link
Member

Also maybe there is merit to https://go-review.googlesource.com/#/c/11478. @dvyukov said it is not needed, so I abandoned the change.

Alex

@rsc rsc modified the milestones: Go1.6Maybe, Go1.6 Jan 14, 2016
@gopherbot
Copy link

CL https://golang.org/cl/18683 mentions this issue.

@golang golang locked and limited conversation to collaborators Feb 3, 2017
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