Use atomics instead of volatile #26
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rely on well defined behaviour for message passing, instead of volatile. Three versions have been tested, to make sure this wouldn't cause a slowdown on any platform. v1: Sequentially consistent atomics No mesurable regression, despite the extra memory barriers on x86. Even with 15 threads and extreme time pressure, both acting as a magnifying glass: threads=15, tc=2+0.02 ELO: 2.59 +-3.4 (95%) LOS: 93.3% Total: 18132 W: 4113 L: 3978 D: 10041 threads=7, tc=2+0.02 ELO: -1.64 +-3.6 (95%) LOS: 18.8% Total: 16914 W: 4053 L: 4133 D: 8728 v2: Acquire/Release semantics This version generates no extra barriers for x86 (on the hot path). As expected, no regression either, under the same conditions: threads=15, tc=2+0.02 ELO: 2.85 +-3.3 (95%) LOS: 95.4% Total: 19661 W: 4640 L: 4479 D: 10542 threads=7, tc=2+0.02 ELO: 0.23 +-3.5 (95%) LOS: 55.1% Total: 18108 W: 4326 L: 4314 D: 9468 As suggested by Joona, another test at LTC: threads=15, tc=20+0.05 ELO: 0.64 +-2.6 (95%) LOS: 68.3% Total: 20000 W: 3053 L: 3016 D: 13931 v3: Final version: SeqCst/Relaxed threads=15, tc=10+0.1 ELO: 0.87 +-3.9 (95%) LOS: 67.1% Total: 9541 W: 1478 L: 1454 D: 6609 Resolves #474
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Rely on well defined behaviour for message passing, instead of volatile. Three
versions have been tested, to make sure this wouldn't cause a slowdown on any
platform.
v1: Sequentially consistent atomics
No mesurable regression, despite the extra memory barriers on x86. Even with 15
threads and extreme time pressure, both acting as a magnifying glass:
threads=15, tc=2+0.02
ELO: 2.59 +-3.4 (95%) LOS: 93.3%
Total: 18132 W: 4113 L: 3978 D: 10041
threads=7, tc=2+0.02
ELO: -1.64 +-3.6 (95%) LOS: 18.8%
Total: 16914 W: 4053 L: 4133 D: 8728
v2: Acquire/Release semantics
This version generates no extra barriers for x86 (on the hot path). As expected,
no regression either, under the same conditions:
threads=15, tc=2+0.02
ELO: 2.85 +-3.3 (95%) LOS: 95.4%
Total: 19661 W: 4640 L: 4479 D: 10542
threads=7, tc=2+0.02
ELO: 0.23 +-3.5 (95%) LOS: 55.1%
Total: 18108 W: 4326 L: 4314 D: 9468
As suggested by Joona, another test at LTC:
threads=15, tc=20+0.05
ELO: 0.64 +-2.6 (95%) LOS: 68.3%
Total: 20000 W: 3053 L: 3016 D: 13931
v3: Final version: SeqCst/Relaxed
threads=15, tc=10+0.1
ELO: 0.87 +-3.9 (95%) LOS: 67.1%
Total: 9541 W: 1478 L: 1454 D: 6609
Resolves #474