Transport: a client parked on the present credit is rung only by the credit, not by every retired drain batch (apply-thread futex wakes 0.056 ms/frame on FCL inproc).

MobileGL/MG_Remote/Transport/Ring.cpp

@@ -500,6 +500,10 @@ namespace MobileGL::MG_Remote::Transport {
     // The wake target for the duration of one wait; 0 again afterwards, so a later wait that
     // needs any progress (or one outside this class on the same flag) is rung as before.
     namespace {
+        // A wake target with this bit names a present-ack serial rather than an appliedSeq.
+        // Neither counter comes near 2^63, so the tag cannot collide with a real sequence.
+        constexpr std::uint64_t kWakeTargetPresentAck = 1ull << 63;
+
         struct ScopedWakeTarget {
             std::atomic<std::uint64_t>& target;
             ScopedWakeTarget(std::atomic<std::uint64_t>& t, std::uint64_t seq) : target(t) {
@@ -555,6 +559,10 @@ namespace MobileGL::MG_Remote::Transport {
             return SessionWait::TimedOut;
         }
         RingControl* control = m_control;
+        // P15: a present-credit wait is rung only by the credit (or a full event ring), not by
+        // every drain batch the server retires meanwhile - those were futile wake-ups of the
+        // GL thread and a futex syscall per batch on the apply thread (FCL inproc: 0.056 ms/frame).
+        const ScopedWakeTarget wake(control->producerWakeSeq, kWakeTargetPresentAck | serial);
         return Park(
             [control, serial] {
                 return Watermark::Reached(control->Progress.presentAckSerial, serial) ||
@@ -690,9 +698,14 @@ namespace MobileGL::MG_Remote::Transport {
             std::atomic_thread_fence(std::memory_order_seq_cst);
             if (m_control->producerParked.load(std::memory_order_acquire) == 0) return;
             const std::uint64_t target = m_control->producerWakeSeq.load(std::memory_order_seq_cst);
-            if (target != 0 && !Watermark::Reached(m_control->Progress.appliedSeq, target) &&
+            const bool eventRingFull = m_control->eventRingFull.load(std::memory_order_acquire) != 0;
-                m_control->eventRingFull.load(std::memory_order_acquire) == 0)
+            if ((target & kWakeTargetPresentAck) != 0) {
+                if (!Watermark::Reached(m_control->Progress.presentAckSerial, target & ~kWakeTargetPresentAck) &&
+                    !eventRingFull)
+                    return;
+            } else if (target != 0 && !Watermark::Reached(m_control->Progress.appliedSeq, target) && !eventRingFull) {
                 return;
+            }
             m_peerBell->Notify();
         }
     }

MobileGL/MG_Remote/Transport/Ring.h

@@ -168,7 +168,8 @@ namespace MobileGL::MG_Remote::Transport {
         // ---- the parked producer's wake target -------------------------------
         //
         // What a parked client is waiting for, so the server rings only when it is there: 0 =
-        // any progress (present credit, ring space, events), otherwise the appliedSeq it needs.
+        // any progress (present credit, ring space, events), otherwise the appliedSeq it needs -
+        // or, with bit 63 set, the present-ack serial a present-credit wait needs (Ring.cpp).
         // Without it every record the server applied while the client waited for a later one
         // was a socket write and a futile wake-up. Written by the client before it parks (and
         // reset to 0 after), read by the server only after it saw producerParked set; on a link

MobileGL/MG_Test/Wire/SessionTest.cpp

@@ -683,6 +683,43 @@ TEST(SessionTest, DrainingAFullEventRingClearsTheLatchAndRingsTheServer) {
     EXPECT_EQ(serverBell.rings.load(), 1u);
 }
 
+// P15: a client parked for the present credit is rung by the credit (or a full event ring) and
+// by nothing else. Every drain batch the server retires while the GL thread waits for the
+// previous present used to ring it: a futile wake-up of the GL thread and a futex syscall on the
+// apply thread per batch (FCL inproc: 0.056 ms/frame).
+//
+// Red-once: drop the ScopedWakeTarget from WaitForPresentAckOrEventBacklog and the retire below
+// rings (rings == 1 before the credit).
+TEST(SessionTest, AClientParkedOnThePresentCreditIsRungOnlyByTheCreditOrABacklog) {
+    auto session = std::make_shared<SessionFixture>();
+    ASSERT_TRUE(session->Build(TestSizes()));
+    CountingBell clientBell;
+    RingControl* serverControl = session->serverSegments.CmdControl();
+    session->consumer.Attach(serverControl, &session->cmdConsumer, &clientBell,
+                             &session->serverTransport->SelfDoorbell(), kDefaultSpinUs);
+
+    std::atomic<bool> released{false};
+    std::thread parked([session, &released] {
+        (void)session->producer.WaitForPresentAckOrEventBacklog(2, 5000);
+        released.store(true, std::memory_order_release);
+    });
+    while (session->Control().producerParked.load() == 0 && !released.load()) {
+        std::this_thread::yield();
+    }
+
+    session->consumer.RetireThrough(0);
+    session->consumer.CompleteFrame(1);
+    EXPECT_EQ(clientBell.rings.load(), 0u) << "progress the parked client does not wait for rang it";
+
+    session->consumer.ReturnPresentCredit(2);
+    EXPECT_EQ(clientBell.rings.load(), 1u);
+
+    // The counting bell wakes nobody; release the waiter through the real one.
+    session->serverTransport->PeerDoorbell().Notify();
+    parked.join();
+    EXPECT_TRUE(released.load());
+}
+
 // ---------------------------------------------------------------------------
 // kRecPad (R-9's last sentence, and the one with no other detector)
 // ---------------------------------------------------------------------------