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

DISPATCH-1344 Relaxing C11 sys_atomic_ operations #239

Closed

Conversation

jiridanek
Copy link
Contributor

@jiridanek jiridanek commented Mar 25, 2022

This is a revival of DISPATCH-1344. The issue that was never meant to have been abandoned.

@astitcher The shared_ptr article about atomics is actually not an article, but a SO question, https://stackoverflow.com/questions/28199212/why-does-libcs-implementation-of-shared-ptr-use-full-memory-barriers-instead

Regarding the question "What use are the relaxed atomics on x86_64?", as far as I can tell, the hardware recognizes two "levels": seq_cst and everything else. By that I mean that the generated code for seq_cst store is actually different from non-cst_seq store. But, the relaxation also influences what code will be generated by the compiler. Even if the CPU does not do reorders, the compiler will, and that may be significant for performance, sometimes. (https://stackoverflow.com/questions/61719680/are-memory-orderings-consume-acq-rel-and-seq-cst-ever-needed-on-intel-x86)

So, IMO if this shows promise in benchmark, then there may be value in thinking about this. Ultimately, the atomics, as a way to silence TSan, were not all that great a solution and hopefully they will go away, eventually.

@jiridanek
Copy link
Contributor Author

The test fails are all already known skupper-router issues, with the exception of one qpid-dispatch issue not yet logged here, I think

@jiridanek jiridanek force-pushed the jd_2022_03_19_relaxed_atomics branch from 316f414 to 7df929f Compare April 13, 2022 09:18
@jiridanek
Copy link
Contributor Author

This does not appreciably increase performance for me. I still hope that it's because I don't have the right benchmark for it (this might do better when there is some fanout, these things; so far I only had one sender/receiver pair).

@jiridanek jiridanek marked this pull request as draft May 6, 2022 17:06
@jiridanek
Copy link
Contributor Author

There is no future in this PR, let's close it.

@jiridanek jiridanek closed this Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant