Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-81793

Don't do any syscalls to notify a baton that isn't in poll

XMLWordPrintableJSON

    • Service Arch
    • Fully Compatible
    • Service Arch 2023-11-13, Service Arch 2023-11-27

      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).

      Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
      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;

            Assignee:
            ryan.berryhill@mongodb.com Ryan Berryhill
            Reporter:
            mathias@mongodb.com Mathias Stearn
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: