Tweaked concurrency.md #27503
Tweaked concurrency.md #27503
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| it's actually not safe: if we had a reference to `data` in each thread, and the | ||
| thread takes ownership of the reference, we have three owners! That's bad. We | ||
| can fix this by using the `Arc<T>` type, which is an atomic reference counted | ||
| pointer. The 'atomic' part means that it's safe to share across threads. |
There was a problem hiding this comment.
this revision seems to lose the 'atomic' explanation, which is especially important given that Swift means something else by Arc
|
I like these changes, but want to retain the 'atomic' explanation, can we bring that back? Also, yeah, the 'would have' is kind of awkward, but I still think this is better. Thanks so much! |
c-nixon
force-pushed
the
master
branch
2 times, most recently
from
0b0054d to
9253204
Compare
|
I've updated the pull request. How's it look now? I originally had a full exposition on Reference counting and Atomic operations but decided to strip it out for brevity's sake since "Atomic Reference Count" is mentioned at the top of the file in the explanation of Sync. However I can definitely see how it's useful to have it explained near the code that uses it. |
|
☔ The latest upstream changes (presumably #27508) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Looks good. r=me after a rebase, due to @bors |
|
📌 Commit 9253204 has been approved by |
|
@bors: r- |
|
That's it rebased, thanks for the feedback! |
|
@bors: r+ rollup |
|
📌 Commit d5b522e has been approved by |
I was reading through the docs and came across a section that felt awkward. I've tried to improve the flow by splitting up and reversing the explanations of Arc and Mutex with some example code in between. The "This would have have happened" bit is unfortunate but I couldn't see any other way to illustrate it. The compiler errors didn't really help tell the story in this particular instance so it still feels a bit forced. However I do think it's an a small improvement... Does anyone have any other ideas that might flow better?
I was reading through the docs and came across a section that felt awkward.
I've tried to improve the flow by splitting up and reversing the explanations of
Arc and Mutex with some example code in between.
The "This would have have happened" bit is unfortunate but I couldn't see any
other way to illustrate it. The compiler errors didn't really help tell the
story in this particular instance so it still feels a bit forced. However I do think it's
an a small improvement...
Does anyone have any other ideas that might flow better?