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: change fastrandn to use % again #22253

Closed
rsc opened this issue Oct 13, 2017 · 3 comments
Closed

runtime: change fastrandn to use % again #22253

rsc opened this issue Oct 13, 2017 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Oct 13, 2017

See #21806. For Go 1.9.2 we want to just go back to %, which is what Go 1.8 and earlier used and is well tested.

@rsc rsc added this to the Go1.9.2 milestone Oct 13, 2017
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Oct 13, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/70991 mentions this issue: [release-branch.go1.9] runtime: use simple, more robust fastrandn

@rsc rsc added the CherryPickApproved Used during the release process for point releases label Oct 15, 2017
@rsc
Copy link
Contributor Author

rsc commented Oct 15, 2017

CL 70991 OK for Go 1.9.2.

gopherbot pushed a commit that referenced this issue Oct 25, 2017
CL 36932 (speed up fastrandn) made it faster but introduced
bad interference with some properties of fastrand itself, making
fastrandn not very random in certain ways. In particular, certain
selects are demonstrably unfair.

For Go 1.10 the new faster fastrandn has induced a new fastrand,
which in turn has caused other follow-on bugs that are still being
discovered and fixed.

For Go 1.9.2, just go back to the barely slower % implementation
that we used in Go 1.8 and earlier. This should restore fairness in
select and any other problems caused by the clever fastrandn.

The test in this CL is copied from CL 62530.

Fixes #22253.

Change-Id: Ibcf948a7bce981452e05c90dbdac122043f6f813
Reviewed-on: https://go-review.googlesource.com/70991
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@rsc
Copy link
Contributor Author

rsc commented Oct 26, 2017

go1.9.2 has been packaged and includes:

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Oct 26 21:09:23 UTC

@rsc rsc closed this as completed Oct 26, 2017
@golang golang locked and limited conversation to collaborators Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants