From d627fca387c8ea0fe45971d9be61de2f02911842 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 4 Oct 2026 05:40:14 -0400 Subject: [PATCH] [Perf] (Transport): the server rings a parked client only once the sequence it waits for is applied --- MobileGL/MG_Remote/Transport/Doorbell.h | 5 ++++- MobileGL/MG_Remote/Transport/Ring.cpp | 25 ++++++++++++++++++++++++- MobileGL/MG_Remote/Transport/Ring.h | 14 ++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Remote/Transport/Doorbell.h b/MobileGL/MG_Remote/Transport/Doorbell.h index afe230d1..8d281305 100644 --- a/MobileGL/MG_Remote/Transport/Doorbell.h +++ b/MobileGL/MG_Remote/Transport/Doorbell.h @@ -274,7 +274,10 @@ namespace MobileGL::MG_Remote::Transport { for (;;) { // Announce, FENCE, then re-test. The fence is the mechanism - // see the file header - so setting the flag itself is relaxed. - parked.store(1, std::memory_order_relaxed); + // Release, so a notifier that acquires this 1 also sees what the waiter wrote + // before parking (RingControl::producerWakeSeq). The fence below is still what + // closes the lost-wakeup window. + parked.store(1, std::memory_order_release); std::atomic_thread_fence(std::memory_order_seq_cst); if (ready()) { parked.store(0, std::memory_order_relaxed); diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index efa3e257..0164392b 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -497,11 +497,24 @@ namespace MobileGL::MG_Remote::Transport { return m_selfBell->Dead() ? SessionWait::ShutDown : SessionWait::TimedOut; } + // 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 { + struct ScopedWakeTarget { + std::atomic& target; + ScopedWakeTarget(std::atomic& t, std::uint64_t seq) : target(t) { + target.store(seq, std::memory_order_seq_cst); + } + ~ScopedWakeTarget() { target.store(0, std::memory_order_seq_cst); } + }; + } // namespace + SessionWait SessionProducer::WaitForApplied(std::uint64_t seq, std::uint32_t timeoutMs) { if (!Valid()) { return SessionWait::TimedOut; } RingControl* control = m_control; + const ScopedWakeTarget wake(control->producerWakeSeq, seq); return Park([control, seq] { return Watermark::Reached(control->Progress.appliedSeq, seq); }, timeoutMs); } @@ -527,6 +540,7 @@ namespace MobileGL::MG_Remote::Transport { return SessionWait::TimedOut; } RingControl* control = m_control; + const ScopedWakeTarget wake(control->producerWakeSeq, seq); return Park( [control, seq] { return Watermark::Reached(control->Progress.appliedSeq, seq) || @@ -670,7 +684,16 @@ namespace MobileGL::MG_Remote::Transport { void SessionConsumer::NotifyClient() { if (m_link) m_link->ProgressChanged(); if (m_control != nullptr && m_peerBell != nullptr) { - NotifyIfParked(*m_peerBell, m_control->producerParked); + // NotifyIfParked, narrowed by the client's wake target: a client parked for an + // applied sequence the server has not reached yet is not rung (unless the event + // ring filled, which that wait also wakes for). + 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) && + m_control->eventRingFull.load(std::memory_order_acquire) == 0) + return; + m_peerBell->Notify(); } } diff --git a/MobileGL/MG_Remote/Transport/Ring.h b/MobileGL/MG_Remote/Transport/Ring.h index d040c620..09139317 100644 --- a/MobileGL/MG_Remote/Transport/Ring.h +++ b/MobileGL/MG_Remote/Transport/Ring.h @@ -164,6 +164,16 @@ namespace MobileGL::MG_Remote::Transport { std::atomic producerParked; // client asleep, server must ring std::atomic eventRingFull; // SEG_EVENT full, server stopped applying std::atomic eventDropped; // dropped lossy events + + // ---- 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. + // 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 + // without this shared page it stays 0, which is the old ring-on-any-progress rule. + alignas(64) std::atomic producerWakeSeq; }; static_assert(sizeof(RingControl) == 4096, "RingControl must be exactly one page"); @@ -232,6 +242,10 @@ namespace MobileGL::MG_Remote::Transport { // The page's real content must fit the page it claims. sizeof(RingControl) == 4096 // above cannot catch a member that grew, because alignas(4096) forces that number; // the struct's content ends far short of it and the tail is padding. + static_assert(offsetof(RingControl, producerWakeSeq) / 64 != offsetof(RingControl, serverEpoch) / 64, + "the wake target has a line of its own"); + static_assert(offsetof(RingControl, producerWakeSeq) + sizeof(std::uint64_t) <= 4096, + "RingControl's members must fit the page it claims to be"); static_assert(offsetof(RingControl, eventDropped) + sizeof(std::uint32_t) <= 4096, "RingControl's members must fit the page it claims to be"); static_assert(alignof(RingControl) == 4096, "RingControl must be page aligned");