rust-lang / nomicon

The Dark Arts of Advanced and Unsafe Rust Programming
https://doc.rust-lang.org/nomicon/
Apache License 2.0
1.75k stars 256 forks source link

Rewrite atomics section #378

Open SabrinaJewson opened 1 year ago

SabrinaJewson commented 1 year ago

Rendered

This PR is for my attempted rewrite of the atomics section of the Nomicon, to give it a more spec-focused explanation and avoid misconceptions like time and reordering. Currently it’s far from finished, containing only an explanation of multi-threaded execution and the start of the “relaxed” section, but I’m opening this PR for early review and feedback on the explanation method.

Reading order:

  1. atomics.md
  2. multithread.md
  3. relaxed.md
  4. acquire-release.md
  5. seqcst.md
  6. fences.md
    Edit: I now see that copyright means this won’t work, so I’ve removed it.

I also copy-pasted in the C++ spec, but changed a couple things, due things like the lack of consume and sig_atomic_t:

I’m not certain about the copyright of this — ISO says that they only hold copyright over published versions, but I couldn’t find information about copyright of drafts.

jwakely commented 1 year ago

I’m not certain about the copyright of this — ISO says that they only hold copyright over published versions, but I couldn’t find information about copyright of drafts.

https://en.cppreference.com/w/cpp/language/memory_model and https://en.cppreference.com/w/cpp/atomic/memory_order#Formal_description paraphrase the relevant parts of the C++ spec, and are CC-BY-SA and GFDL dual-licensed. See What can I do with the material on this site?.

SabrinaJewson commented 1 year ago

Given that my free time has been reduced since I’ve started this PR and the signals section (which I was part way through writing) is a mostly different area of expertise to the rest of atomics, I'm going to mark this as ready to review now.

Open questions:

cc @cbeuw for fact-checking since you seem to be an expert on the model (only if you have time of course).

SabrinaJewson commented 1 year ago

I have now rendered this PR for easier review.

cbeuw commented 1 year ago

This looks absolutely amazing! The Unicode diagrams are nicely rendered on both my Mac and Android phone, and are very pretty. This may well be the best tutorial on C++ memory model out there, including all the ones I've seen targeting C++ exclusively.

I definitely wouldn't call myself a weak memory expert, considering whenever I work with it I always come up with new questions instead of being able to tell the definite behaviours (but this may be a good thing sometimes). Unfortunately for me, the next 3 months will be extremely busy. I won't be able to go through it in detail, but I'll have my eye on it from time to time, both now and after it's been merged. There will almost certainly be extremely subtle errors given the topic's nature, but that shouldn't prevent this from going live.

newpavlov commented 1 year ago

I have minor rendering issues with the diagrams on Linux (in both Firefox and Chromium):

I suggest replacing them with inline SVG or separate image files.

JanBeh commented 1 year ago

From Issue #104814 on Rust:

[…] it should be clarified whether the difference between the Rust documentation and the C++20 reference exists intentionally, i.e. is Rust deliberately giving less guarantees to the programmer than the C++20 standard does? (Even though still following the C++20 memory model in practice "as of now".)

I have been told that the C++20 reference takes precedence over Rust's documentation, but wanted to raise this issue nonetheless as it may be relevant for future backward compatibility.

WaffleLapkin commented 1 year ago

What is the status here? 👀

ehuss commented 1 year ago

This is in my queue to review, but I have 100+ PRs in that queue, and this one is quite large and a challenging topic. Realistically I probably won't get to it soon.

In the meantime, I recommend people check out Mara's book at https://marabos.nl/atomics/ which is now free.

EDIT to add: To be clear, I very much want to see this land. I just may not be able to make time for it for a while.

WaffleLapkin commented 6 months ago

@ehuss do you still have too much in the review queue?

SabrinaJewson commented 4 months ago

I just pushed a commit to fix the incorrect claim that one needs 4 threads to observe the difference between SeqCst and AcqRel, which was a misunderstanding I had. I also changed the example that shows the need for SeqCst to a simpler one that only uses two threads.

The changed sections include the SeqCst chapter and the part on SeqCst fences. Thanks @ibraheemdev for pointing this out!