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/v2: submit chacha8 and runtime code [freeze exception] #64540

Closed
rsc opened this issue Dec 4, 2023 · 5 comments
Closed

math/rand/v2: submit chacha8 and runtime code [freeze exception] #64540

rsc opened this issue Dec 4, 2023 · 5 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Dec 4, 2023

freeze exception request for CL 546020 (fixing a rolled-back CL from before the freeze) and CL 516860 (finishing math/rand/v2, most of which was submitted before the freeze)

cc @golang/release

@rsc rsc added this to the Go1.22 milestone Dec 4, 2023
@rsc
Copy link
Contributor Author

rsc commented Dec 5, 2023

I see thumbs up from bcmills and dmitshur. What do I do next? Is there a state transition I am waiting for?

@dmitshur
Copy link
Contributor

dmitshur commented Dec 5, 2023

We will leave an explicit comment soon. I left a reaction to express my opinion even sooner. Thanks.

@dmitshur dmitshur added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 5, 2023
@rsc
Copy link
Contributor Author

rsc commented Dec 5, 2023

As far as the risk of the two CLs:

  • CL 546020 does not possibly break anything, since math/rand/v2 is new. (It was rolled back due to not being big-endian-safe and due to a bug in the RISCV compiler SSA rules; both are fixed.)
  • CL 516860 makes the runtime start using this code, but the CL looks bigger than it is because I rationalized the names and linkname usage, causing a lot of renaming. The CL has two different random generators in it used by the runtime. The ChaCha8 is used directly in all map creation and iteration, so it is heavily exercised. If it were broken, things would blow up quickly. The "cheap" random generator (code unchanged since before, just repackaged a bit) is used during interface checks, allocation, and any kind of blocking due to the randomized balanced tree of waiters. If it were broken, things would similarly blow up quickly.

So these should both be very low risk. If they were bad, trybots would have failed spectacularly in all kinds of different ways.

@cagedmantis
Copy link
Contributor

As discussed in the release meeting, this has been approved. There are additional considerations that we would like to discuss regarding the release timing.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Dec 5, 2023
@prattmic
Copy link
Member

CL 546020 and CL 516860 are submitted. There were a few follow-up issues (filed as separate bugs), all resolved now as far as I know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
Archived in project
Development

No branches or pull requests

4 participants