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.

Sign up for GitHub

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

Jump to bottom

x/exp/rand: devirtualize interfaces #34614

Closed
josharian opened this issue Sep 30, 2019 · 3 comments
Closed

x/exp/rand: devirtualize interfaces #34614

josharian opened this issue Sep 30, 2019 · 3 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Unreleased

Comments

@josharian
Copy link
Contributor

This is a reminder/tracking issue to apply the same optimizations as https://go-review.googlesource.com/c/go/+/191538 and https://go-review.googlesource.com/c/go/+/191539 to x/exp/rand.

cc @CAFxX @robpike
@gopherbot gopherbot added this to the Unreleased milestone Sep 30, 2019
@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance labels Sep 30, 2019
@robpike
Copy link
Contributor

robpike commented Oct 1, 2019

I just tried this and things got slower.

First I did 538 and that helped, then I did 539 and things slowed down.

I will send the CL anyway but I think without the compiler support this is just pointless complexity.

@gopherbot
Copy link

Change https://golang.org/cl/198099 mentions this issue: rand: apply 'de-virtualization' optimization from math/rand

gopherbot pushed a commit to golang/exp that referenced this issue Oct 2, 2019
@robpike
rand: apply 'de-virtualization' optimization from math/rand
a1355ae 
https://golang.org/cl/19153{8,9} and sped up things by removing a closure
call. It doesn't help here, perhaps because the compiler isn't aware.

BenchmarkRead3-4                 10.7          11.5          +7.48%
BenchmarkRead64-4                113           112           -0.88%
BenchmarkRead1000-4              1608          1588          -1.24%

Part of the reduced improvement is because the compiler doesn't
know about exp/rand (yet).

Update golang/go#34614

Change-Id: Ia439c60d69c30a25d35f21675d60601dbdc98f4f
Reviewed-on: https://go-review.googlesource.com/c/exp/+/198099
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@odeke-em
Copy link
Member

I believe this issue was fixed by CL https://go-review.googlesource.com/c/exp/+/198099 so I shall close it.

@golang golang locked and limited conversation to collaborators Oct 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@josharian @robpike @odeke-em @bcmills @gopherbot