Skip to content

Conversation

lnkuiper
Copy link
Contributor

Fixes #14389 and reverts #14173 (bringing back the salt).

Although I don't think this should affect correctness (from cppreference.com):

When a compare-and-exchange is in a loop, the weak version will yield better performance on some platforms. When a weak compare-and-exchange would require a loop and a strong one would not, the strong one is preferable.

This gave us correctness issues with older GCC versions (< 10.x) on ARM64. We rarely use CAS operations, so this isn't a big diff. I've added std::memory_order to improve the performance (copying the usage in concurrentqueue.h). Since it is difficult to programmatically disable "incorrect" usage, I've added a comment in atomic.hpp that describes the issue.

I've also changed usage of MetadataPointer to a const reference since older GCC versions were throwing a warning.

@hannes
Copy link
Member

hannes commented Oct 28, 2024

CC @gropaul

@lnkuiper
Copy link
Contributor Author

I think this is ready to go, but due to some arrow failures a bunch of tests got skipped

@Mytherin Mytherin merged commit 8664b71 into duckdb:main Oct 29, 2024
37 of 38 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@gropaul
Copy link
Contributor

gropaul commented Oct 29, 2024

Thank you for the fix!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Nov 2, 2024
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Nov 2, 2024
Cas strong (duckdb/duckdb#14592)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
@lnkuiper lnkuiper deleted the cas_strong branch April 14, 2025 09:14
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.

exists Keyword not Working on AArch64 Linux
4 participants