search

max0x7ba / atomic_queue

C++ lockless queue.
MIT License
1.47k stars 176 forks source link

why compare_exchange_strong() in a loop? #57

Closed Andriy06 closed 10 months ago

Andriy06 commented 10 months ago

out of curiousity, why compare_exchange_strong() is used in loops where I believe compare_exchange_weak() would be sufficient? based on benchmarks results? do you still have them or remember details?

and while I have you here :) why platform dependent definition of CACHE_LINE_SIZE? any known issues with std::hardware_destructive_interference_size?

thanks

max0x7ba commented 10 months ago

out of curiousity, why compare_exchange_strong() is used in loops where I believe compare_exchange_weak() would be sufficient? based on benchmarks results? do you still have them or remember details?

It is documented in README. To prevent caller's non-atomic stores and loads from being reordered across push and pop.

and while I have you here :) why platform dependent definition of CACHE_LINE_SIZE? any known issues with std::hardware_destructive_interference_size?

This library requires C++14. std::hardware_destructive_interference_size is a C++17 feature.

Andriy06 commented 10 months ago

thanks for the quick reply. Sorry I still don't get it. The linked line of README talks about "synchronising with" between push and pop that, I believe, is governed solely by the memory order. I believe that _strong version is the same as _weak regarding reordering guarantees, and the only difference is that it prevents spurious failures, which is not a problem in this case as on failure the code will retry in a loop. What am I missing?

max0x7ba commented 10 months ago

out of curiousity, why compare_exchange_strong() is used in loops where I believe compare_exchange_weak() would be sufficient? based on benchmarks results? do you still have them or remember details?

I believe that _strong version is the same as _weak regarding reordering guarantees, and the only difference is that it prevents spurious failures, which is not a problem in this case as on failure the code will retry in a loop.

Ah, this is about compare_exchange_strong vs compare_exchange_weak, not the memory order. My mistake.

The library originated on x86-64 platform where both the weak and strong versions generate identical assembly code. Reviewing the generated assembly code on other platforms supported today reveals that on power64 the strong version generates an extra retry loop, https://godbolt.org/z/se1h1M8cK

Since this library implements its own retry loops, this extra retry loop introduced by the strong version is unnecessary and should be removed. As you astutely spotted and kindly made an effort to bring this issue to my attention.

If you'd like to submit a pull request addressing this issue I'd be delighted to merge it.

Andriy06 commented 10 months ago

glad that misunderstanding is resolved :) will submit a pull request in a bit

max0x7ba commented 10 months ago

Thank you for your contribution.