Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks to @vtjnash on #multithreading who pointed out the atomics don't do anything, because we aren't using
atomic_not!
to flip the boolean one, for example. He emphasized that atomics are relations, not objects, and it's the act of flipping the boolean that could be atomic (i.e. so you could get the current value and negate that, and then set that to be the value, all while knowing another thread hasn't changed the value inbetween getting and setting the value).Here, however, we don't need to get and set together (atomically), we are just setting, so there is no need for an atomic (and it doesn't do anything).
I think we could use locks if we were concerned about multiple threads changing these values at once, but one just shouldn't do that (just choose the settings at the start of the script; doesn't really make sense to change it partway through a multithreaded computation). I don't think there's a problem with multiple threads reading the values at the same time.
This partially reverts #372 (i.e. reverts the atomic stuff, not the DCP warnings).