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

math/rand: rand tests take a long time to run on arm5 #9732

Closed
davecheney opened this issue Jan 29, 2015 · 11 comments
Closed

math/rand: rand tests take a long time to run on arm5 #9732

davecheney opened this issue Jan 29, 2015 · 11 comments
Assignees
Milestone

Comments

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jan 29, 2015

The math/rand tests take a very long time to run on platforms without an FPU. We've fixed the run time of this package a few times, however this package may have regressed while the arm5 builder was offline

linux-arm-arm5:

ok      math/rand   183.211s
@davecheney davecheney added this to the Go1.5 milestone Jan 29, 2015
@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Jan 30, 2015

Actually it's very slow on all arm platforms
linux-arm-panda:

ok      math/rand       331.167s
@josharian
Copy link
Contributor

@josharian josharian commented Feb 5, 2015

The regression is pretty extreme, actually: ~30x slower. Numbers from an RPi.

1.4:

=== RUN TestStandardNormalValues
--- PASS: TestStandardNormalValues (0.09s)
=== RUN TestNonStandardNormalValues
--- PASS: TestNonStandardNormalValues (11.36s)
=== RUN TestStandardExponentialValues
--- PASS: TestStandardExponentialValues (0.08s)
=== RUN TestNonStandardExponentialValues
--- PASS: TestNonStandardExponentialValues (0.71s)
=== RUN TestNormTables
--- PASS: TestNormTables (0.00s)
=== RUN TestExpTables
--- PASS: TestExpTables (0.00s)
=== RUN TestFloat32
--- PASS: TestFloat32 (12.31s)

tip

=== RUN TestStandardNormalValues
--- PASS: TestStandardNormalValues (2.51s)
=== RUN TestNonStandardNormalValues
--- PASS: TestNonStandardNormalValues (319.04s)
=== RUN TestStandardExponentialValues
--- PASS: TestStandardExponentialValues (2.55s)
=== RUN TestNonStandardExponentialValues
--- PASS: TestNonStandardExponentialValues (20.45s)
=== RUN TestNormTables
--- PASS: TestNormTables (0.09s)
=== RUN TestExpTables
--- PASS: TestExpTables (0.16s)
=== RUN TestFloat32
--- PASS: TestFloat32 (524.50s)

Running a quick CPU profile shows lots of the new time being spent in runtime.stepflt, called from runtime._System. The hottest address outside floating point routines is in runtime·systemstack.

Based on that, I wonder whether someone (GC?) is doing lots of floating point work where it wasn't before, and it's contending for the system stack in order to run floating point routines.

I'll start doing a bisection to find the relevant commit. It'll take a while. :)

@minux
Copy link
Member

@minux minux commented Feb 5, 2015

@minux
Copy link
Member

@minux minux commented Feb 5, 2015

@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Feb 5, 2015

GOARM=6 and GOARM=7 are also horribly slow.

On Fri, Feb 6, 2015 at 8:34 AM, Minux Ma notifications@github.com wrote:

oh, this issue is about the GOARM=5 case.

Always running software FP on system stack is definitely one of the reasons
for the performance regression.


Reply to this email directly or view it on GitHub
#9732 (comment).

@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Feb 5, 2015

I think that in the switch to cmd/dist written in Go, GOARM now always
ends up being set to 5, so all arm implementations are using softfloat

On Fri, Feb 6, 2015 at 8:34 AM, Dave Cheney dave@cheney.net wrote:

GOARM=6 and GOARM=7 are also horribly slow.

On Fri, Feb 6, 2015 at 8:34 AM, Minux Ma notifications@github.com wrote:

oh, this issue is about the GOARM=5 case.

Always running software FP on system stack is definitely one of the
reasons
for the performance regression.


Reply to this email directly or view it on GitHub
#9732 (comment).

@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Feb 5, 2015

Yes, xgetgoarm defaults to 5, and will not attempt vfp detection because

func xtryexecfunc(f func()) bool {
        // TODO(rsc): Implement.
        // The C cmd/dist used this to test whether certain assembly
        // sequences could be executed properly. It used signals and
        // timers and sigsetjmp, which is basically not possible in Go.
        // We probably have to invoke ourselves as a subprocess instead,
        // to contain the fault/timeout.
        return false
}
@davecheney davecheney self-assigned this Feb 5, 2015
@davecheney davecheney closed this Feb 5, 2015
@davecheney davecheney reopened this Feb 5, 2015
@josharian
Copy link
Contributor

@josharian josharian commented Feb 6, 2015

@davecheney yep. My bisection finally finished and pointed the finger at one of these commits:

20a10e7
ad6ee36
d369f97

Do you think that there is a separate math/rand regression, or should we just convert this issue to track fixing that TODO?

@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Feb 6, 2015

@josharian i'll fix the vfp detection in cmd/dist/util.go (it turns out all the twiddling we did on the .s files was pointless, they aren't being called).

With GOARM detected properly, there is not issue with math/rand

panda(~/go/src) % go test -short math/rand
ok      math/rand       2.446s
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 10, 2015

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

minux added a commit that referenced this issue Sep 10, 2015
cmd/dist will re-exec itself to detect VFP support at run-time.

Fixes #9732, #12548.

Change-Id: I9ad0c5c7fa3e97bd79a32da372e1a962565bb3af
Reviewed-on: https://go-review.googlesource.com/3973
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Sep 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.