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

doc: update Go memory model #50859

Closed
rsc opened this issue Jan 27, 2022 · 13 comments
Closed

doc: update Go memory model #50859

rsc opened this issue Jan 27, 2022 · 13 comments

Comments

@rsc
Copy link
Contributor

rsc commented Jan 27, 2022

In June 2021 I posted a series of articles about memory models, ending with an article about changes I thought we should make to the Go memory model. See https://research.swtch.com/mm especially https://research.swtch.com/gomm.

Then I opened a GitHub Discussion to discuss these changes; see #47141.

Based on that discussion, I propose the following concrete changes to the memory model:

  • Document Go's overall approach.
  • Document that multiword races can cause crashes.
  • Document happens-before for runtime.SetFinalizer.
  • Document (or link to) happens-before for more sync types.
  • Document happens-before for sync/atomic, matching C++ sequentially consistent atomics (and Java, JavaScript, Rust, Swift, C, ...)
  • Document disallowed compiler optimizations.

The exact details can be viewed in pending CLs prepared for concreteness, in particular CL 381315 (memory model) and CL 381316 (library docs).

I have filed a separate proposal - #50860 - for another item that arose during that discussion, namely adding typed atomic values to sync/atomic.

@gopherbot
Copy link

gopherbot commented Jan 27, 2022

Change https://golang.org/cl/381315 mentions this issue: doc: update Go memory model

@gopherbot
Copy link

gopherbot commented Jan 27, 2022

Change https://golang.org/cl/381316 mentions this issue: runtime, sync, sync/atomic: document happens-before guarantees

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 27, 2022
@rsc rsc moved this from Incoming to Active in Proposals (old) Feb 2, 2022
@rsc
Copy link
Contributor Author

rsc commented Feb 2, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor Author

rsc commented Feb 16, 2022

Based on feedback from ARM, I revised the examples toward the end of the memory model to remove the implication that hoisting reads (provided they don't fault) out of conditionals is always problematic.

@rsc
Copy link
Contributor Author

rsc commented Feb 16, 2022

Does anyone object to accepting this proposal?

@hboehm
Copy link

hboehm commented Feb 16, 2022

Sorry about leaving a comment on the CL before seeing the comment not to. I continue to believe there are serious problems with the approach, which I will again offer to discuss.

@rsc
Copy link
Contributor Author

rsc commented Feb 18, 2022

Thanks @hboehm. I will take you up on that offer.

@rsc
Copy link
Contributor Author

rsc commented Mar 1, 2022

To update the issue here, I had a good discussion with @hboehm.

Hans's primary objection is that the new text claims that Go provides DRF-SC but that the rules given in the current draft are not strong enough to support that claim.

My goal for the current rules is to try to restrict what compilers are allowed to do, to avoid cleverness like reloading a value from memory rather than spilling it to a private location like the stack. In general my goal is to say to users "don't depend on the behavior of racy programs" while at the same time say to compiler writers (in contrast to C/C++) "don't assume programs are race-free / make races worse". That is, we might call this "DRF-SC but avoid catching fire".

Hans believes that for the user part, it should be possible to reduce the C++ DRF-SC model down to something that is reasonable for Go (and much simpler than C++, since we don't have relaxed atomics), and that the compiler writer part can be delivered as informal implementation advice. I am going to look into this approach.

@rsc
Copy link
Contributor Author

rsc commented Mar 1, 2022

Hans also pointed me at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1217r2.html, in particular the section titled "A new class of out-of-thin-air results", which is certainly worrying. We currently do not allow relaxed atomic loads/stores, but if we did start thinking about those operations, we would need to grapple with what those programs should be allowed to do.

@rsc
Copy link
Contributor Author

rsc commented May 4, 2022

I have updated the text based on a discussion with @hboehm from early April. The main change is to start with a "here are the semantics for race-free programs" section, which explicitly aligns with C, C++, Java, JavaScript, Rust, Swift, and so on. Then the old text limiting the damage possible in racy programs follows as an informal implementation restriction.

In my head, this new text says what I always wanted to say and is a no-op. But formally, it actually says the right things so that race-free programs can be proved to provide sequentially consistent semantics (DRF-SC).

The diff is https://go-review.googlesource.com/c/go/+/381315/5..6.

@rsc
Copy link
Contributor Author

rsc commented May 11, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) May 11, 2022
@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) May 18, 2022
@rsc
Copy link
Contributor Author

rsc commented May 18, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: doc: update Go memory model doc: update Go memory model May 18, 2022
@rsc rsc modified the milestones: Proposal, Backlog May 18, 2022
@rsc rsc modified the milestones: Backlog, Go1.19 Jun 2, 2022
@gopherbot
Copy link

gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410675 mentions this issue: doc/go_mem: update revision date

gopherbot pushed a commit that referenced this issue Jun 6, 2022
CL 381315 added major revisions but neglected to update the date.

For #50859.

Change-Id: I086a55f0c80579c479bca5268109c9f3ae680adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/410675
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopherbot pushed a commit that referenced this issue Jun 6, 2022
A few of these are copied from the memory model doc.
Many are entirely new, following discussion on #47141.
See https://research.swtch.com/gomm for background.

The rule we are establishing is that each type that is meant
to help synchronize a Go program should document its
happens-before guarantees.

For #50859.

Change-Id: I947c40639b263abe67499fa74f68711a97873a39
Reviewed-on: https://go-review.googlesource.com/c/go/+/381316
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

3 participants