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

Why do atomic<int>::wait on linux use timed futex wait? #57677

Open
MBkkt opened this issue Sep 11, 2022 · 7 comments
Open

Why do atomic<int>::wait on linux use timed futex wait? #57677

MBkkt opened this issue Sep 11, 2022 · 7 comments
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@MBkkt
Copy link

MBkkt commented Sep 11, 2022

https://github.com/llvm/llvm-project/blob/main/libcxx/src/atomic.cpp#L42

@EugeneZelenko EugeneZelenko added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. and removed new issue labels Sep 11, 2022
@philnik777
Copy link
Contributor

I don't think it does. std::atomic<int>::wait() is at atomic:1591,
which calls std::__cxx_atomic_wait from atomic:1503,
which calls std::__cxx_atomic_wait from atomic:1470,
which calls std::__libcpp_poll_with_backoff from __thread/poll_with_backoff.h.

@MBkkt
Copy link
Author

MBkkt commented Jan 15, 2023

@philnik777 please see ink, it does, even author of code answered (in PR) it does.

In general code called from
https://github.com/llvm/llvm-project/blob/main/libcxx/src/atomic.cpp#L127

It's called for int from here
https://github.com/llvm/llvm-project/blob/main/libcxx/src/atomic.cpp#L187

Which is called from backoff cycle implementation
https://github.com/llvm/llvm-project/blob/main/libcxx/include/atomic#L1458

The single reason why here is sleep it's because it's needed for correct implementation for types which use static entries.
But author for some not understandable reason
author don't call futex function twice with different arguments (for native wait without timeout and for entries with).
And it's why this issue exist.

Another issue only int use platform wait, other 32 bit types don't use.
I think it's really awful because mainly atomic wait/notify uses as counters (unsigned)
Btw libstdc++ use futex, not entries, for any sizeof(int) type.

@MBkkt
Copy link
Author

MBkkt commented Jan 15, 2023

Or you are talking about not Linux/macos implementation?
https://github.com/llvm/llvm-project/blob/main/libcxx/src/atomic.cpp#L77

Issue about Linux implementation

macos btw probably incorrect and can wait forever for types which isn't int

@philnik777
Copy link
Contributor

What exactly is even the issue? Is it somehow wrong, or is is inefficient?

@MBkkt
Copy link
Author

MBkkt commented Jan 15, 2023

@philnik777
This issue about strange inefficient implementation.
Which call timed wait instead of unlimited wait in case of atomic<int> under Linux.

Also with atomic wait exist few other issues:

  1. only int supported for native wait, instead of any type which accepted by syscall
  2. macOS implementation can wait forever
  3. maybe some other, implementation is poor. Did author see boost.atomic? rhetorical question

@tambry
Copy link
Contributor

tambry commented Oct 10, 2023

I believe the rationale for the 2s timeout is the same as described here.

macOS implementation can wait forever

Are you referencing the problem described here?

@MBkkt
Copy link
Author

MBkkt commented Oct 11, 2023

@tambry

I believe the rationale for the 2s timeout is the same as described

Yes, but it's case only for atomic which can not be natively used as futex, for an example, atomic_int8/16/64_t, for libc++ it's also all unsigned types (because it specializes only atomic_int to native futex usage).

So initially issue was about why natively supported types of atomics (only one now atomic<int>) used timed wait? It's completely unnecessary.
At the same time I was really disappointed in that atomic<unsigned> don't use native implementation
Because on practice for something like "counters" unsigned a lot of more useful than signed.

Are you referencing the problem described here?

I referred to libc++ code care about linux overflow issue in case of table of futexes used (with this 2 seconds timeout) and doesn't care about same issue for macOS.
I think I missed that __cxx_atomic_contention_t is int64_t on macOS, so there is no such issue for it
Sorry for confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

4 participants