[SERVER-81797] Make our own portable implementation of atomic notify() and wait() with timeout support Created: 03/Oct/23 Updated: 07/Feb/24 |
|
| Status: | In Code Review |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Mathias Stearn | Assignee: | Vinod Kumar |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | perf-8.0, perf-tiger, perf-tiger-handoff, perf-tiger-q4, perf-tiger-triaged, risk | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Assigned Teams: |
Service Arch
|
||||||||||||||||||||||||||||||||
| Sprint: | Service Arch 2023-12-11, Service Arch 2023-12-25, Service Arch 2024-01-08, Service Arch 2024-01-22, Service Arch 2024-02-05, Service Arch 2024-02-19 | ||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||
| Description |
|
In a few places we've either wanted to use C++20's std::atomic::wait() and notify apis, or have code currently using a mutex+condvar+enum/bool that could be using that instead. Unfortunately, the sandardized APIs don't support timeouts or deadlines (due to issues around freestanding support) while effectively all of our usages need them. Luckily, all of the OSes we support provide APIs that do take timeouts or deadlines, so we should just build our own wait/notify abstraction on top of them. We can use the try_wait_for and try_wait_until APIs proposed here. We can base it on the implementation in libc++ (covering linux, apple/darwin, and freebsd) and use WaitOnAddress / WakeByAddressSingle / WakeByAddressAll on windows (the MSSTL implementation is convoluted to support old windows versions without those APIs, but we dont need to support them). I think it would be best to add a new AtomicWaitable<T> type that publicly derives from AtomicWord<T> but ensures that `T` is a size that is supported natively by the current platform (all platforms seem to support 32bit words, so we could just require that everywhere). We could also consider adding a fallback implementation that adds a mutex+condvar to the subclass and uses them for notifications. If sizeof(T) is smaller than the platform natively supports (eg bool on linux) we could consider padding it out and using an underlying AtomicWaitable<uint32_t> rather than failing to compile and requiring the caller to do their own padding. |
| Comments |
| Comment by Mathias Stearn [ 04/Oct/23 ] |
|
To add some background, I confirmed with the original proposer of the atomic wait/notify APIs for c++20 that they were omitted at the time to avoid issues with freestanding (ie C++ implementations without an OS). <atomic> has always been part of freestanding but <chrono> isn't, so it That said, I still think it is worth adding a separate type for notifiable atomics so that we can use a different representation with extra inline data for types that the platform doesn't natively support waiting on. That isn't an option in the standard because std::atomic has a fixed ABI in practice, but we are not bound by that restriction in our code. |