[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:
Depends
is depended on by SERVER-72616 Remove futex usage in favour of std::... Backlog
is depended on by SERVER-83053 Use atomic wait/notify in AsioNetwork... Blocked
Related
related to SERVER-81793 Don't do any syscalls to notify a bat... Closed
is related to SERVER-86362 Move platform based code in separate ... Needs Scheduling
is related to SERVER-72064 Implement AtomicWord::wait similar to... Backlog
is related to WT-11836 Use futexes when waiting for a resource Investigating
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 isn't wasn't possible to use the time types from std::atomic. Since then, there has been a relaxation of freestanding to allow fine grained selection of what is and is not required to be available in a freestanding implementation, so it is now possible to add those methods and have just them be optional on freestanding implementations without affecting the rest of the std::atomic API. That is what the try_wait* APIs in P2643 are doing. That proposal is currently working its way through the committee and it seems on track for C++26. So we should probably mirror those APIs if we want to have the option to cut over to the standard APIs later. Plus, if we run into issues with them, we can provide feedback and hopefully get them addressed prior to standardization.

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.

Generated at Thu Feb 08 06:47:27 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.