[SERVER-81793] Don't do any syscalls to notify a baton that isn't in poll Created: 03/Oct/23  Updated: 24/Jan/24  Resolved: 15/Nov/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.3.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Mathias Stearn Assignee: Ryan Berryhill
Resolution: Fixed Votes: 0
Labels: perf-8.0, perf-tiger, perf-tiger-handoff, perf-tiger-q4, perf-tiger-triaged
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by SERVER-83053 Use atomic wait/notify in AsioNetwork... Blocked
Related
related to SERVER-81789 Don't kill already completed op in Se... Closed
is related to SERVER-81797 Make our own portable implementation ... In Code Review
Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2023-11-13, Service Arch 2023-11-27
Participants:

 Description   

Here is a simple patch that makes it cheaper to notify an asio baton that isn't currently inside of poll. It queues a notification that will be noticed on the next call to poll, rather than trying pushing data into the eventfd. Next time it enters poll, it will notice this and do a non-blocking call to poll to pick up any ready events. I did that to maintain the behavior of always checking for and processing ready sockets and timers on every call to _poll(), but if that behavior isn't necessary, then you can save a syscall in this case by just returning early from that function.

This is more of an issue because of SERVER-81789 which causes us to unconditionally notify an idle baton on every operation, but even if that gets fixed we should still optimize this code to make notifications cheaper. This change also makes it easier to do a futex (pending SERVER-81797) or condvar wait on the newly-introduced _notificationHandshake instead of calling poll and using the eventfd, if there are no sockets to be monitored, by adding a new state to the enum like kWaitingOnAtomic. (Let me know if you want me to make that patch). Testing has shown that waiting on a futex seems to be cheaper than poll+eventfd (I don't know if this is just because it is 1 syscall rather than 2, or if there is something else that makes those syscalls more expensive).

diff --git a/src/mongo/transport/asio/asio_networking_baton.cpp b/src/mongo/transport/asio/asio_networking_baton.cpp
index 4400ac91eae..07afe73ec84 100644
--- a/src/mongo/transport/asio/asio_networking_baton.cpp
+++ b/src/mongo/transport/asio/asio_networking_baton.cpp
@@ -28,6 +28,9 @@
  */
 
 
+#include "mongo/platform/compiler_gcc.h"
+#include "mongo/stdx/condition_variable.h"
+#include <atomic>
 #include <sys/eventfd.h>
 
 #include "mongo/transport/asio/asio_networking_baton.h"
@@ -160,7 +163,10 @@ void AsioNetworkingBaton::schedule(Task func) noexcept {
 }
 
 void AsioNetworkingBaton::notify() noexcept {
-    efd(_opCtx).notify();
+    NotificationHandshake old =
+        _notificationHandshake.exchange(kNotificationPending, std::memory_order_relaxed);
+    if (old == kInPoll)
+        efd(_opCtx).notify();
 }
 
 Waitable::TimeoutState AsioNetworkingBaton::run_until(ClockSource* clkSource,
@@ -373,13 +379,23 @@ std::list<Promise<void>> AsioNetworkingBaton::_poll(stdx::unique_lock<Mutex>& lk
         _inPoll = true;
         lk.unlock();
 
+        const auto oldState = _notificationHandshake.exchange(kInPoll, std::memory_order_relaxed);
+        invariant(oldState != kInPoll);
+
         const ScopeGuard guard([&] {
+            // Both consumes a notification (if-any) and mark us as no-longer in poll
+            _notificationHandshake.store(kNone, std::memory_order_relaxed);
+
             lk.lock();
             _inPoll = false;
         });
 
         blockAsioNetworkingBatonBeforePoll.pauseWhileSet();
-        int timeout = deadline ? Milliseconds(*deadline - now).count() : -1;
+        int timeout = oldState == kNotificationPending
+            ? 0  // Don't wait if there is a notification pending.
+            : deadline ? Milliseconds(*deadline - now).count()
+                       : -1;
+
         int events = ::poll(_pollSet.data(), _pollSet.size(), timeout);
         if (events < 0) {
             auto ec = lastSystemError();
@@ -429,23 +445,26 @@ Future<void> AsioNetworkingBaton::_addSession(Session& session, short events) tr
 }
 
 void AsioNetworkingBaton::detachImpl() noexcept {
-    decltype(_scheduled) scheduled;
-    decltype(_sessions) sessions;
-    decltype(_timers) timers;
 
-    {
-        stdx::lock_guard lk(_mutex);
+    stdx::unique_lock lk(_mutex);
+
+    invariant(_opCtx->getBaton().get() == this);
+    _opCtx->setBaton(nullptr);
 
-        invariant(_opCtx->getBaton().get() == this);
-        _opCtx->setBaton(nullptr);
+    _opCtx = nullptr;
 
-        _opCtx = nullptr;
+    if (MONGO_likely(_scheduled.empty() && _sessions.empty() && _timers.empty()))
+        return;
 
-        using std::swap;
-        swap(_scheduled, scheduled);
-        swap(_sessions, sessions);
-        swap(_timers, timers);
-    }
+    using std::swap;
+    decltype(_scheduled) scheduled;
+    swap(_scheduled, scheduled);
+    decltype(_sessions) sessions;
+    swap(_sessions, sessions);
+    decltype(_timers) timers;
+    swap(_timers, timers);
+
+    lk.unlock();
 
     for (auto& job : scheduled) {
         job(stdx::unique_lock(_mutex));
diff --git a/src/mongo/transport/asio/asio_networking_baton.h b/src/mongo/transport/asio/asio_networking_baton.h
index 3d3086a30b1..92a20506b59 100644
--- a/src/mongo/transport/asio/asio_networking_baton.h
+++ b/src/mongo/transport/asio/asio_networking_baton.h
@@ -146,6 +146,10 @@ private:
 
     bool _inPoll = false;
 
+    enum NotificationHandshake { kNone, kNotificationPending, kInPoll };
+    std::atomic<NotificationHandshake>  // NOLINT (all operations on this can safely be relaxed)
+        _notificationHandshake;
+
     // Stores the sessions we need to poll on.
     stdx::unordered_map<SessionId, TransportSession> _sessions;



 Comments   
Comment by Githook User [ 15/Nov/23 ]

Author:

{'name': 'Ryan Berryhill', 'email': 'ryan.berryhill@mongodb.com', 'username': 'ryanberryhill'}

Message: SERVER-81793 Don't do any syscalls to notify a baton that isn't in poll
Branch: master
https://github.com/mongodb/mongo/commit/d1c4dfd156f32116d071a448f3bbc24d9bb77b52

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