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

Should we keep compareExchange and compareExchangeStrong? #13837

Closed
ronawho opened this issue Aug 22, 2019 · 10 comments
Closed

Should we keep compareExchange and compareExchangeStrong? #13837

ronawho opened this issue Aug 22, 2019 · 10 comments

Comments

@ronawho
Copy link
Contributor

ronawho commented Aug 22, 2019

We implement compareExchange(), which is just an alias of compareExchangeStrong(). It seems odd to me to have both, and I propose we deprecate one of them. I don't have a strong preference on which one we deprecate. C/C++ have compare_exchange_weak() and compare_exchange_strong(). Rust went with compare_exchange_weak() and compare_exchange()

For some additional context compare_exchange_weak() can spuriously fail, and is intended to be used in situations where you'd be looping anyways as it can offer better performance on some platforms. https://stackoverflow.com/questions/25199838/understanding-stdatomiccompare-exchange-weak-in-c11 has a pretty good explanation of weak vs. strong CAS.

@mppf
Copy link
Member

mppf commented Aug 23, 2019

compareExchange stong should be the "default" since compareExchangeWeak is an optimization. I think deprecating compareExchangeStrong is reasonable.

@bradcray
Copy link
Member

I feel inclined to keep compareExchangeStrong() so that someone coming from C++ will find the familiar name easily and to keep compareExchange() for convenience. I might also consider adding a param strong = true argument to the compareExchange() overload so that (1) someone could switch between weak and strong via a config param or somesuch without having to have a param conditional that called one of two functions (i.e., for brevity) and (2) so that there's slightly more reason for both compareExchange() and compareExchangeStrong() to exist (i.e., it's not just the same function twice with two different names).

@ronawho
Copy link
Contributor Author

ronawho commented Feb 10, 2020

I think my preference is to just have compareExchange() and compareExchangeWeak().

Adding compareExchangeStrong() just to match C++ doesn't seem very compelling to me since we're already different with our atomic read()/write() (called load()/store() in C/C++).

Plus it wouldn't be a breaking change to add compareExchangeStrong() later if there is demand for it.

@ronawho
Copy link
Contributor Author

ronawho commented Feb 21, 2020

FWIW Rust went with compare_exchange and compare_exchange_weak

@mppf
Copy link
Member

mppf commented Feb 21, 2020

I feel inclined to keep compareExchangeStrong() so that someone coming from C++ will find the familiar name easily and to keep compareExchange() for convenience.

Suppose we did that. Suppose such a user coming from C++ is reading some Chapel code. That Chapel code might use compareExchange (which we are supposing exists only for convenience). In this case there is no benefit to that user to keeping compareExchangeStrong, right? At that point isn't it just a matter of writing something they can find with grep/search in the documentation?

@ronawho
Copy link
Contributor Author

ronawho commented Feb 21, 2020

Vote for compareExchange() (and compareExchangeWeak())

@ronawho
Copy link
Contributor Author

ronawho commented Feb 21, 2020

Vote for compareExchange() and compareExchangeStrong() (and compareExchangeWeak())

@ronawho
Copy link
Contributor Author

ronawho commented Feb 24, 2020

Vote for compareExchange() (and compareExchangeWeak())

This is what's on master with #14883.

@bradcray you had originally been in favor of having a compareExchangeStrong() -- do you still feel that way?

@bradcray
Copy link
Member

Not enough to fight against this tide. I still find the notion of a param arg on compareExchange() intriguing, but not enough to fight for that either (and it can always be added later if someone wants to argue for it).

@ronawho
Copy link
Contributor Author

ronawho commented Feb 24, 2020

Ok, thanks

Since compareExchange() and compareExchangeWeak() are on master, and nobody is currently fighting for compareExchangeStrong() I'm going to close this.

@ronawho ronawho closed this as completed Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants