diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp index ce38cffc..6f1f9dd6 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp @@ -96,10 +96,10 @@ namespace MGITest { if (!control) return false; const auto until = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); do { - if (control->appliedSeq.load(std::memory_order_acquire) >= seq) return true; + if (control->Progress.appliedSeq.load(std::memory_order_acquire) >= seq) return true; std::this_thread::sleep_for(std::chrono::milliseconds(1)); } while (std::chrono::steady_clock::now() < until); - return control->appliedSeq.load(std::memory_order_acquire) >= seq; + return control->Progress.appliedSeq.load(std::memory_order_acquire) >= seq; #else (void)seq; (void)timeoutMs; return false; @@ -143,9 +143,9 @@ namespace MGITest { state.presentCredit = MobileGL::MG_Config::Ipc.PresentCredit; state.presentCreditWaits = session->PresentCreditWaits(); if (const auto* control = session->Control()) { - state.appliedSeq = control->appliedSeq.load(std::memory_order_acquire); - state.retiredSeq = control->retiredSeq.load(std::memory_order_acquire); - state.presentAckSerial = control->presentAckSerial.load(std::memory_order_acquire); + state.appliedSeq = control->Progress.appliedSeq.load(std::memory_order_acquire); + state.retiredSeq = control->Progress.retiredSeq.load(std::memory_order_acquire); + state.presentAckSerial = control->Progress.presentAckSerial.load(std::memory_order_acquire); } // The producer's ledger. Every one of these is a plain member read on the encoder diff --git a/MobileGL/MG_Remote/CONTRACT-P6.md b/MobileGL/MG_Remote/CONTRACT-P6.md index 91383634..ace083c4 100644 --- a/MobileGL/MG_Remote/CONTRACT-P6.md +++ b/MobileGL/MG_Remote/CONTRACT-P6.md @@ -432,7 +432,7 @@ path must have no more out-of-line calls than today. That is falsifiable; "withi The watermark cache line is `{appliedSeq, submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}`; `eventRingFull`/`eventDropped` sit in a **separate** `alignas(64)` group with `serverEpoch`/`ringGeneration`/the two park flags. And `submittedSeq` is **producer-written** -(`Ring.h:50`) while the other four are consumer-written — so the group has **mixed writers**, and any +(`Ring.h`'s "THE FIVE WATERMARKS" block) while the other four are consumer-written — so the group has **mixed writers**, and any "single-writer progress block aliased over the watermark line" is unsound. `lk` therefore **regroups**: `submittedSeq` moves to the producer's own line, beside `cmdHead`, @@ -449,7 +449,8 @@ becomes a named `LinkProgress` member with per-field `offsetof` static_asserts. > That is new false sharing on the hottest line in the system, and it would land in exactly the > numbers this package's gate has to hold constant. The two flags stay in the doorbell/generation > group, where mixed writers already live beside `consumerParked`/`producerParked`, and cross the -> seam through a separate `EventFlags()` accessor on its own line. +> seam through a separate `EventFlags()` accessor. *Accessor*, not line: they share +> `serverEpoch`'s line, which is where mixed writers already live. It is free **today** — both peers are the same binary, enforced by the fingerprint — and a wire break the day a second build exists. It is **`lk`'s and not `c6`'s** because the rename blast radius is @@ -479,7 +480,7 @@ have nothing to do with a leak. ### 8.6 `SEG_STAGE`'s cursors are deliberately dead -`Ring.h:105-129` states it and a test pins it: `SEG_STAGE` is **not a ring**; staging is an +`Ring.h`'s "DEAD IN P5, DELIBERATELY" states it and a test pins it: `SEG_STAGE` is **not a ring**; staging is an encoder-local linear allocator reclaiming on `retiredSeq`, and all three stage cursors stay zero for the whole of P5. A stream link's send window either revives them **completely** or does not use them; the header names the middle state as *a guaranteed hang rather than a slow path*. P6.5's, recorded @@ -544,7 +545,7 @@ Negative controls, each run red once: 3. `Transport/ReplySlot.h`'s "P6 debt" for the 2 MiB reply cap and chunked readback. **Declined**: P9's. Note `P6-SPAWN-PLAN §8.5` already reassigned it and the header was never updated — two documents in the tree disagreed; this file settles it. -4. `Ring.h:71-77` — `appliedSeq` is excluded from lazy publication **by name**. P6 does not change +4. `Ring.h`'s "BATCHING MAY ONLY MAKE A WATERMARK LATE" — `appliedSeq` is excluded from lazy publication **by name**. P6 does not change the behaviour; it records that any future stream link **amends** this rule rather than inferring around it. Several designs have argued the late-never-early invariant licenses lazy `appliedSeq`; it explicitly carves `appliedSeq` out. diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 243d24be..605abd64 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -178,7 +178,7 @@ namespace MobileGL::MG_Remote::Client { if (wait == Transport::SessionWait::Reached) { Transport::RingControl* control = producer.Control(); if (control == nullptr || - Transport::Watermark::Reached(control->appliedSeq, seq)) { + Transport::Watermark::Reached(control->Progress.appliedSeq, seq)) { return wait; } DrainEventRing(events); @@ -1263,7 +1263,7 @@ namespace MobileGL::MG_Remote::Client { if (wait != Transport::SessionWait::Reached) break; Transport::RingControl* control = m_producer.Control(); if (control == nullptr || - Transport::Watermark::Reached(control->presentAckSerial, awaited)) { + Transport::Watermark::Reached(control->Progress.presentAckSerial, awaited)) { break; } DrainEventRing(m_events); diff --git a/MobileGL/MG_Remote/Transport/ILink.h b/MobileGL/MG_Remote/Transport/ILink.h index 85370e7d..6dbf2cbe 100644 --- a/MobileGL/MG_Remote/Transport/ILink.h +++ b/MobileGL/MG_Remote/Transport/ILink.h @@ -94,21 +94,21 @@ namespace MobileGL::MG_Remote::Transport { // decoded progress messages. // // THE RULES SURVIVE THE MECHANISM VERBATIM, and that is the point of naming - // the block rather than the page: late-never-early (Ring.h:71-77), `>=` + // the block rather than the page: late-never-early (Ring.h's "BATCHING MAY ONLY MAKE A WATERMARK LATE"), `>=` // never `==` (SessionRings.h's Watermark::Reached), monotone with a // backwards move Fatal (Ring.cpp's AdvanceMonotonic). A stream link changes // only HOW a value arrives, never what it means. // // WHY THESE FOUR AND NOT THE WHOLE PAGE, AND WHY THE EVENT FLAGS ARE NOT - // HERE. RingControl's watermark cache line today holds {appliedSeq, + // HERE. RingControl's watermark cache line HELD, until lk regrouped it, {appliedSeq, // submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}, and - // submittedSeq is PRODUCER-written (Ring.h:50, "Advanced by the PRODUCER - // after it publishes") while the other four are consumer-written - so the + // submittedSeq is PRODUCER-written (Ring.h's "THE FIVE WATERMARKS" block, + // "Advanced by the PRODUCER after it publishes") while the other four are consumer-written - so the // line has MIXED WRITERS as it stands and no single-writer block can be // aliased over it. lk's regroup moves submittedSeq out to the producer's // own line, beside cmdHead, which is the other thing the producer writes; - // what is left IS this struct, and it becomes a named member of - // RingControl with per-field offsetof static_asserts. + // what is left IS this struct, and it IS a named member of RingControl, + // pinned there by per-field offsetof static_asserts (Ring.h). // // THE TWO EVENT FLAGS ARE DELIBERATELY EXCLUDED, and an earlier draft of // this header had them in and was wrong. eventRingFull has TWO writers: @@ -123,8 +123,7 @@ namespace MobileGL::MG_Remote::Transport { // writers already live beside consumerParked/producerParked, and cross the // seam through EventFlags() instead. // - // Free to regroup TODAY, because both peers are the same binary and the - // fingerprint enforces it; a wire break the day a second build exists, + // It was free to regroup while both peers are the same binary; a wire break the day a second build exists, // which is why it is booked now rather than later. struct LinkProgress { std::atomic appliedSeq; // records applied @@ -217,8 +216,10 @@ namespace MobileGL::MG_Remote::Transport { // an attached link. virtual LinkProgress* Progress() = 0; - // The reverse channel's flags. A separate accessor, and a separate - // cache line, for the reason LinkEventFlags documents. + // The reverse channel's flags. A separate ACCESSOR, for the reason + // LinkEventFlags documents. NOT a separate cache line: they sit in the + // doorbell/generation group beside serverEpoch, which is where mixed + // writers already live. virtual LinkEventFlags* EventFlags() = 0; // The command-record arena (producer side) and cursor (consumer side). diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index dec86938..fbf7d39c 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -391,7 +391,7 @@ namespace MobileGL::MG_Remote::Transport { } void AdvanceApplied(RingControl& control, std::uint64_t seq) { - AdvanceMonotonic(control.appliedSeq, seq, "appliedSeq"); + AdvanceMonotonic(control.Progress.appliedSeq, seq, "appliedSeq"); } void AdvanceRetired(RingControl& control, std::uint64_t seq) { @@ -399,16 +399,16 @@ namespace MobileGL::MG_Remote::Transport { // reclaims behind it, so a retire ahead of the apply hands live bytes // back to the producer. Clamped rather than refused, because a // caller that retires "everything applied" is the normal shape. - const std::uint64_t applied = control.appliedSeq.load(std::memory_order_acquire); - AdvanceMonotonic(control.retiredSeq, seq > applied ? applied : seq, "retiredSeq"); + const std::uint64_t applied = control.Progress.appliedSeq.load(std::memory_order_acquire); + AdvanceMonotonic(control.Progress.retiredSeq, seq > applied ? applied : seq, "retiredSeq"); } void AdvanceCompletedFrame(RingControl& control, std::uint64_t serial) { - AdvanceMonotonic(control.completedFrameSerial, serial, "completedFrameSerial"); + AdvanceMonotonic(control.Progress.completedFrameSerial, serial, "completedFrameSerial"); } void AdvancePresentAck(RingControl& control, std::uint64_t serial) { - AdvanceMonotonic(control.presentAckSerial, serial, "presentAckSerial"); + AdvanceMonotonic(control.Progress.presentAckSerial, serial, "presentAckSerial"); } } // namespace Watermark @@ -488,7 +488,7 @@ namespace MobileGL::MG_Remote::Transport { return SessionWait::TimedOut; } RingControl* control = m_control; - return Park([control, seq] { return Watermark::Reached(control->appliedSeq, seq); }, + return Park([control, seq] { return Watermark::Reached(control->Progress.appliedSeq, seq); }, timeoutMs); } @@ -497,7 +497,7 @@ namespace MobileGL::MG_Remote::Transport { return SessionWait::TimedOut; } RingControl* control = m_control; - return Park([control, serial] { return Watermark::Reached(control->presentAckSerial, serial); }, + return Park([control, serial] { return Watermark::Reached(control->Progress.presentAckSerial, serial); }, timeoutMs); } @@ -515,7 +515,7 @@ namespace MobileGL::MG_Remote::Transport { RingControl* control = m_control; return Park( [control, seq] { - return Watermark::Reached(control->appliedSeq, seq) || + return Watermark::Reached(control->Progress.appliedSeq, seq) || control->eventRingFull.load(std::memory_order_acquire) != 0; }, timeoutMs); @@ -529,7 +529,7 @@ namespace MobileGL::MG_Remote::Transport { RingControl* control = m_control; return Park( [control, serial] { - return Watermark::Reached(control->presentAckSerial, serial) || + return Watermark::Reached(control->Progress.presentAckSerial, serial) || control->eventRingFull.load(std::memory_order_acquire) != 0; }, timeoutMs); @@ -554,7 +554,7 @@ namespace MobileGL::MG_Remote::Transport { m_peerBell = peerBell; m_selfBell = selfBell; m_spinUs = spinUs; - m_appliedSeq = control == nullptr ? 0 : control->appliedSeq.load(std::memory_order_acquire); + m_appliedSeq = control == nullptr ? 0 : control->Progress.appliedSeq.load(std::memory_order_acquire); m_retirableCursor = cmd == nullptr ? 0 : cmd->LocalTail(); m_borrowHeld = false; } diff --git a/MobileGL/MG_Remote/Transport/Ring.h b/MobileGL/MG_Remote/Transport/Ring.h index 967cbbf0..d7b44a6b 100644 --- a/MobileGL/MG_Remote/Transport/Ring.h +++ b/MobileGL/MG_Remote/Transport/Ring.h @@ -43,13 +43,19 @@ // land without changing shape. // // --------------------------------------------------------------------------- -// THE FIVE WATERMARKS (P5 R-9). One sentence each, and they are a contract: +// THE FIVE WATERMARKS (P5 R-9). Four of them are Transport::LinkProgress and +// are reached as `control.Progress.`; submittedSeq is the fifth and stays +// at RingControl scope, on the producer's line (lk, CONTRACT-P6 §8.3). +// One sentence each, and they are a contract: // every one of the five was declared here at P0 and written by nobody but // InitRingControl, so until P5 there was nothing to disagree with. // // submittedSeq Advanced by the PRODUCER after it publishes. NOBODY // WAITS ON IT - it is diagnostic, the answer to "how far -// ahead of the server is the client right now". +// ahead of the server is the client right now". It lives +// on the producer's own cache line beside cmdHead; it is +// the ONE of the five that is not in LinkProgress, and +// that is why LinkProgress can be single-writer. // appliedSeq Advanced by the CONSUMER for EVERY SINGLE RECORD it // applies. The client's verb barrier and every reply wait // read it, so it is the one watermark P5 FORBIDS BATCHING: @@ -88,9 +94,11 @@ #pragma once #include "../Protocol/mg_protocol_base.h" +#include "ILink.h" // LinkProgress: the four consumer-written watermarks, lk #include #include +#include #include namespace MobileGL::MG_Remote::Transport { @@ -99,7 +107,13 @@ namespace MobileGL::MG_Remote::Transport { // each contended group on its own cache line. struct alignas(4096) RingControl { // ---- SEG_CMD cursors ------------------------------------------------ + // + // cmdHead and submittedSeq share a line because they share a WRITER: + // the producer publishes the head and then stamps how far it has got. + // submittedSeq used to sit with the four consumer-written watermarks, + // which made that group mixed-writer and unaliasable (lk, §8.3). alignas(64) std::atomic cmdHead; // producer: bytes written + std::atomic submittedSeq; // producer: records published alignas(64) std::atomic cmdAppliedTail; // consumer: bytes decoded/copied out std::atomic cmdRetiredTail; // consumer: borrowed slots released @@ -132,11 +146,16 @@ namespace MobileGL::MG_Remote::Transport { std::atomic stageRetiredTail; // ---- sequence / frame watermarks ------------------------------------- - alignas(64) std::atomic appliedSeq; // records applied - std::atomic submittedSeq; // handed to the driver - std::atomic retiredSeq; // GPU finished - std::atomic completedFrameSerial; - std::atomic presentAckSerial; + // + // THE CONSUMER-WRITTEN FOUR, AND ONLY THOSE. This is Transport::LinkProgress + // (ILink.h), named so that a data plane with no shared page can own the + // same four values and every wait predicate in SessionRings.h keeps ONE + // implementation at source level. Single-writer by construction now that + // submittedSeq has moved up to the producer's line; the two event flags + // stay in the doorbell group below, because the client clears + // eventRingFull with an RMW on every drain and that traffic may not land + // on the line the client also spins on. + alignas(64) LinkProgress Progress; // ---- doorbell / generation ------------------------------------------- alignas(64) std::atomic serverEpoch; // ++ on context loss / server restart @@ -148,6 +167,73 @@ namespace MobileGL::MG_Remote::Transport { }; static_assert(sizeof(RingControl) == 4096, "RingControl must be exactly one page"); + // lk (CONTRACT-P6 §8.3). The regroup is free while both peers are the same binary + // and a LAYOUT BREAK the day they are not, so the shape is pinned here - by asserts + // that can each go red for the reason they exist, which the first revision of this + // block did not manage. + // + // NOTHING ELSE IN THE TREE PINS ANY OF THIS. The ABI fingerprint + // (SessionRings.h's AbiFingerprintInputs) takes three sizeofs, the caps geometry, two + // codec versions, kOpCount, the protocol version and the build stamp - and NO input + // from this page at all. sizeof(RingControl) is 4096 whatever the members do, because + // alignas(4096) makes it so. So a silent field reorder here would leave every gate in + // the tree green and both peers hashing identically. + static_assert(std::is_standard_layout_v && std::is_standard_layout_v, + "the offsetof gates below are defined only for standard-layout types"); + + // The producer's line: cmdHead and submittedSeq, together and nothing else waiting on them. + static_assert(offsetof(RingControl, submittedSeq) == offsetof(RingControl, cmdHead) + 8, + "submittedSeq sits immediately after cmdHead"); + static_assert(offsetof(RingControl, submittedSeq) / 64 == offsetof(RingControl, cmdHead) / 64, + "submittedSeq shares the producer's cache line with cmdHead"); + + // The Progress line, and it owns that line ALONE. + // + // Written as `serverEpoch == Progress + 64` rather than the obvious + // `Progress + sizeof(LinkProgress) <= offsetof(serverEpoch)`, because the obvious one + // CANNOT GO RED: serverEpoch is alignas(64) and is declared next, so the compiler puts + // it at the following 64-multiple no matter what precedes it, and the `<=` holds for + // every layout including a wrong one. + // + // WHAT THIS PAIR DOES NOT CATCH, stated because a gate nobody can falsify is worse + // than no gate: a SMALL member inserted between Progress and serverEpoch lands at + // offset 288, inside Progress's own line - and serverEpoch's alignas(64) absorbs it, + // so both asserts still pass. Verified by trying it. Up to 32 bytes can be hidden + // there. Growth INSIDE LinkProgress is caught, by sizeof below; growth BESIDE it is + // not, and the only thing standing between that and a false-sharing regression is + // this comment and the group's own. + static_assert(offsetof(RingControl, Progress) % 64 == 0, + "LinkProgress starts a cache line of its own"); + static_assert(offsetof(RingControl, serverEpoch) == offsetof(RingControl, Progress) + 64, + "the Progress line holds Progress and nothing else"); + + // PER FIELD, which is what §8.3 asks for and what the aggregate asserts above cannot + // do: the field ORDER inside LinkProgress is a wire-visible fact about a page both + // roles map, and swapping two of them passes every other check in this file. + static_assert(offsetof(LinkProgress, appliedSeq) == 0, "LinkProgress field order: appliedSeq"); + static_assert(offsetof(LinkProgress, retiredSeq) == 8, "LinkProgress field order: retiredSeq"); + static_assert(offsetof(LinkProgress, completedFrameSerial) == 16, + "LinkProgress field order: completedFrameSerial"); + static_assert(offsetof(LinkProgress, presentAckSerial) == 24, + "LinkProgress field order: presentAckSerial"); + static_assert(sizeof(LinkProgress) == 32, + "four consumer-written watermarks; the event flags are NOT in here"); + + // The two event flags stay in the doorbell group on purpose (see the group's comment). + // LinkEventFlags mirrors them, so the mirror's shape is pinned too. + static_assert(offsetof(RingControl, eventDropped) == offsetof(RingControl, eventRingFull) + 4, + "the two event flags are adjacent, in that order"); + static_assert(offsetof(RingControl, eventRingFull) / 64 == offsetof(RingControl, serverEpoch) / 64, + "the event flags share the doorbell group's line, NOT the Progress line"); + static_assert(offsetof(LinkEventFlags, eventRingFull) == 0 && + offsetof(LinkEventFlags, eventDropped) == 4 && sizeof(LinkEventFlags) == 8, + "LinkEventFlags mirrors the pair in RingControl"); + + // 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, 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"); static_assert(std::atomic::is_always_lock_free, "the ring cursors are shared across processes: they must be lock-free"); diff --git a/MobileGL/MG_Remote/Transport/StreamLink.h b/MobileGL/MG_Remote/Transport/StreamLink.h index d47f5a12..4537dc6d 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.h +++ b/MobileGL/MG_Remote/Transport/StreamLink.h @@ -39,12 +39,12 @@ // than hand out the pool. // // WHAT P6.5 STILL HAS TO SETTLE, recorded here so it is not rediscovered: -// - SEG_STAGE's three ring cursors are DELIBERATELY DEAD (Ring.h:105-129, +// - SEG_STAGE's three ring cursors are DELIBERATELY DEAD (Ring.h's "DEAD IN P5, DELIBERATELY", // pinned by a test). Staging is an encoder-local linear allocator that // reclaims on retiredSeq. A send window either revives them COMPLETELY or // does not use them; the header names the middle state as a guaranteed // hang rather than a slow path. -// - appliedSeq is excluded BY NAME from lazy publication (Ring.h:71-77). +// - appliedSeq is excluded BY NAME from lazy publication (Ring.h's "BATCHING MAY ONLY MAKE A WATERMARK LATE"). // P6.5 AMENDS that rule with a flush-on-idle discipline; it does not infer // around it. // - one dedicated reader thread per direction per process, or the event-ring diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 3a6a8343..708acfc4 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -893,7 +893,7 @@ namespace MobileGL::MG_Remote::Wire { if (m_stageRetirementBell == nullptr || m_stageMarkFront == m_stageMarks.size()) break; const Uint64 pending = m_stageMarks[m_stageMarkFront].Seq; const auto ready = [&] { - return m_control->retiredSeq.load(std::memory_order_acquire) >= pending; + return m_control->Progress.retiredSeq.load(std::memory_order_acquire) >= pending; }; if (!ready()) { // The allocation still cannot progress after reclamation. Count this @@ -923,7 +923,7 @@ namespace MobileGL::MG_Remote::Wire { static_cast(m_stageCapacity), static_cast(m_stageHead - m_stageTail), static_cast( - m_control != nullptr ? m_control->retiredSeq.load(std::memory_order_acquire) : 0)); + m_control != nullptr ? m_control->Progress.retiredSeq.load(std::memory_order_acquire) : 0)); std::abort(); } @@ -1201,7 +1201,7 @@ namespace MobileGL::MG_Remote::Wire { if (m_control == nullptr) { return; } - const Uint64 retired = m_control->retiredSeq.load(std::memory_order_acquire); + const Uint64 retired = m_control->Progress.retiredSeq.load(std::memory_order_acquire); Uint64 upTo = m_stageTail; while (m_stageMarkFront < m_stageMarks.size() && m_stageMarks[m_stageMarkFront].Seq <= retired) { diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index e67d9277..d69a9ae8 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -143,10 +143,10 @@ namespace { } const bool result = m_decoder.DecodeAndApply(view); ++m_sessionApplied; - m_control.appliedSeq.store(m_sessionApplied, std::memory_order_release); + m_control.Progress.appliedSeq.store(m_sessionApplied, std::memory_order_release); // Nothing in P5 borrows a ring slot into the GPU timeline, so a record's SEG_STAGE // runs retire as soon as it is applied (table 1's "retires: apply"). - m_control.retiredSeq.store(m_sessionApplied, std::memory_order_release); + m_control.Progress.retiredSeq.store(m_sessionApplied, std::memory_order_release); m_consumer.PublishRetired(); if (applied != nullptr) { *applied = result; @@ -561,8 +561,8 @@ TEST_F(PipeWireCodecTest, KNoneRoundTripsAndReachesItsApplier) { ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_TRUE(applied); EXPECT_EQ(wire.Decoder().AppliedSeq(), 1u); - EXPECT_EQ(wire.Control().appliedSeq.load(), 1u); - EXPECT_EQ(wire.Control().retiredSeq.load(), 1u); + EXPECT_EQ(wire.Control().Progress.appliedSeq.load(), 1u); + EXPECT_EQ(wire.Control().Progress.retiredSeq.load(), 1u); } TEST_F(PipeWireCodecTest, KHasBlobRoundTripsWithARealChunkBlob) { @@ -1742,7 +1742,7 @@ TEST_F(PipeWireCodecTest, AppliedSeqAdvancesByExactlyOnePerRecordAndIsNeverBatch bool applied = false; ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_EQ(wire.Decoder().AppliedSeq(), i); - EXPECT_EQ(wire.Control().appliedSeq.load(), i); + EXPECT_EQ(wire.Control().Progress.appliedSeq.load(), i); EXPECT_EQ(wire.SessionAppliedSeq(), wire.Decoder().AppliedSeq()); } EXPECT_EQ(wire.Encoder().EmitSeq(), 5u); @@ -1763,13 +1763,13 @@ TEST_F(PipeWireCodecTest, TheDecoderWritesNoRingControlFieldOfItsOwn) { bool corrupt = false; ASSERT_TRUE(wire.Consumer().Pop(view, &corrupt)); ASSERT_FALSE(corrupt); - const std::uint64_t appliedBefore = wire.Control().appliedSeq.load(); - const std::uint64_t retiredBefore = wire.Control().retiredSeq.load(); + const std::uint64_t appliedBefore = wire.Control().Progress.appliedSeq.load(); + const std::uint64_t retiredBefore = wire.Control().Progress.retiredSeq.load(); (void)wire.Decoder().DecodeAndApply(view); - EXPECT_EQ(wire.Control().appliedSeq.load(), appliedBefore); - EXPECT_EQ(wire.Control().retiredSeq.load(), retiredBefore); + EXPECT_EQ(wire.Control().Progress.appliedSeq.load(), appliedBefore); + EXPECT_EQ(wire.Control().Progress.retiredSeq.load(), retiredBefore); EXPECT_EQ(wire.Decoder().AppliedSeq(), 1u); // the decoder's own tally did move } diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index 9dd747bb..b63fd9d3 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -644,7 +644,7 @@ TEST(RemoteClientControls, CommandWrapWaitsForRetirementAfterThePriorRecordAppli peer.Install(); // A mailbox round trip also lets the previous drain retire. Srv::ServerLoopInstance().SetBeforeRetireHookForTesting(&HoldRetirementUntilProducerWaits); const auto prior = emit(32768); - if (ClientSessionInstance().Control()->retiredSeq.load() >= prior) ::_exit(111); + if (ClientSessionInstance().Control()->Progress.retiredSeq.load() >= prior) ::_exit(111); retirementRetryStarted.store(true); const auto next = emit(32768); if (next != prior + 1 || !retirementWaitObserved.load()) ::_exit(112); @@ -664,7 +664,7 @@ TEST(RemoteClientControls, StagingWaitsForRetirementAfterThePriorRecordApplied) MGPBindRenderState bind{}; const auto prior = session.EmitAndWait(MGPWireOp::BindRenderState, &bind, sizeof(bind), nullptr, 0, nullptr, 0, nullptr); - if (session.Control()->retiredSeq.load() >= prior) ::_exit(113); + if (session.Control()->Progress.retiredSeq.load() >= prior) ::_exit(113); retirementRetryStarted.store(true); session.Encoder().StageBytes(bytes.data(), bytes.size()); if (!retirementWaitObserved.load() || session.Encoder().StageReclaimWaits() != 1) ::_exit(114); diff --git a/MobileGL/MG_Test/Wire/RemoteClientE1Controls.inc b/MobileGL/MG_Test/Wire/RemoteClientE1Controls.inc index e08718bc..47139920 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientE1Controls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientE1Controls.inc @@ -56,11 +56,11 @@ void CheckE1Boundary(Bool serverCap, MGPWireOp op, Bool expectWait, const char* : session.EmitAndWait(op, &clear, sizeof(clear), nullptr, 0, nullptr, 0, nullptr); // Observe BEFORE allowing the peer to complete a non-waiting emission. const Bool parked = bell.entered.load(); - const Bool appliedBeforeReturn = session.Control()->appliedSeq.load() >= seq; + const Bool appliedBeforeReturn = session.Control()->Progress.appliedSeq.load() >= seq; peer.clientReturned.store(true); producer.*PeerMember(ProducerBellTag{}) = bell.original; if (session.WaitForApplied(seq, 2000) != Transport::SessionWait::Reached) ::_exit(85); - if (seq <= before || peer.seen.load() != 1 || session.Control()->appliedSeq.load() < seq) ::_exit(86); + if (seq <= before || peer.seen.load() != 1 || session.Control()->Progress.appliedSeq.load() < seq) ::_exit(86); const Bool correct = expectWait ? parked && appliedBeforeReturn : !parked && !appliedBeforeReturn; std::printf("E1 observation: op=%s cap=%u barrier=%u parked=%u applied_before_return=%u emitted=1 peer_seen=1\n", op == MGPWireOp::GenerateMipmap ? "GenerateMipmap" : "Clear", diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index df294af2..4710af8b 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -2444,7 +2444,7 @@ TEST(RemoteRunAhead, ACreditOneClientPaysTheCreditAtEveryPresentAfterTheFirst) { // ONE CREDIT PER SWAP, in the client's own 1-based space: the server returned the // second present's credit, so the watermark is at least 2. if (session.Control() == nullptr || - session.Control()->presentAckSerial.load() < 2u) { + session.Control()->Progress.presentAckSerial.load() < 2u) { ::_exit(104); } session.Stop(); diff --git a/MobileGL/MG_Test/Wire/RingTest.cpp b/MobileGL/MG_Test/Wire/RingTest.cpp index 8ffa604e..6c7ab41d 100644 --- a/MobileGL/MG_Test/Wire/RingTest.cpp +++ b/MobileGL/MG_Test/Wire/RingTest.cpp @@ -109,11 +109,27 @@ TEST(RingTest, ControlPageLayoutIsTheSharedContract) { EXPECT_EQ(offset(&control.cmdAppliedTail) % 64, 0); EXPECT_EQ(offset(&control.stageHead) % 64, 0); EXPECT_EQ(offset(&control.stageAppliedTail) % 64, 0); - EXPECT_EQ(offset(&control.appliedSeq) % 64, 0); + EXPECT_EQ(offset(&control.Progress.appliedSeq) % 64, 0); EXPECT_EQ(offset(&control.serverEpoch) % 64, 0); // cmdHead and cmdAppliedTail are written by different processes: they must // not share a line. EXPECT_NE(offset(&control.cmdHead) / 64, offset(&control.cmdAppliedTail) / 64); + + // lk (CONTRACT-P6 §8.3) regrouped this page BY WRITER, and submittedSeq is the field + // it moved. Assert that here rather than borrowing the claim from Ring.h's + // static_asserts: this test is named for the layout being a shared contract, and it + // said nothing about the one field the regroup touched. + EXPECT_EQ(offset(&control.submittedSeq) / 64, offset(&control.cmdHead) / 64) + << "submittedSeq is producer-written and belongs on the producer's line"; + EXPECT_NE(offset(&control.submittedSeq) / 64, offset(&control.Progress.appliedSeq) / 64) + << "a producer-written field on the consumer's watermark line is what lk removed"; + + // The two event flags stay OUT of Progress on purpose: the client clears eventRingFull + // with an RMW on every drain, and Progress's line is the one the client spins on. + EXPECT_NE(offset(&control.eventRingFull) / 64, offset(&control.Progress.appliedSeq) / 64) + << "eventRingFull's per-drain RMW may not land on the spin line"; + EXPECT_EQ(offset(&control.eventRingFull) / 64, offset(&control.serverEpoch) / 64) + << "the event flags live in the doorbell group, where mixed writers already are"; } TEST(RingTest, RejectsANonPowerOfTwoCapacity) { @@ -538,10 +554,10 @@ TEST(RingTest, WatermarksAreAllZeroUntilSomeoneAdvancesThem) { alignas(4096) RingControl control{}; InitRingControl(control); EXPECT_EQ(control.submittedSeq.load(), 0u); - EXPECT_EQ(control.appliedSeq.load(), 0u); - EXPECT_EQ(control.retiredSeq.load(), 0u); - EXPECT_EQ(control.completedFrameSerial.load(), 0u); - EXPECT_EQ(control.presentAckSerial.load(), 0u); + EXPECT_EQ(control.Progress.appliedSeq.load(), 0u); + EXPECT_EQ(control.Progress.retiredSeq.load(), 0u); + EXPECT_EQ(control.Progress.completedFrameSerial.load(), 0u); + EXPECT_EQ(control.Progress.presentAckSerial.load(), 0u); // ... while the two GENERATIONS start at one, because for them zero means // "uninitialized" and must never be a legal value. The two conventions are // opposite on purpose and are next to each other in the same struct. @@ -559,9 +575,9 @@ TEST(RingTest, AWatermarkWaiterMustTestGreaterOrEqualRatherThanEqual) { const std::uint64_t mySeq = 7; // The peer jumps straight past the value this waiter cares about. - control.appliedSeq.store(mySeq + 1, std::memory_order_release); + control.Progress.appliedSeq.store(mySeq + 1, std::memory_order_release); - const std::uint64_t seen = control.appliedSeq.load(std::memory_order_acquire); + const std::uint64_t seen = control.Progress.appliedSeq.load(std::memory_order_acquire); EXPECT_FALSE(seen == mySeq) << "an equality waiter is still asleep at this point"; EXPECT_TRUE(seen >= mySeq) << "the >= waiter this contract mandates has been released"; } @@ -582,11 +598,11 @@ TEST(RingTest, AppliedSeqAdvancesOncePerRecordAndIsNeverBatchedInP5) { RingRecordView view{}; while (ring.Consumer().Pop(view)) { ++applied; - ring.Control().appliedSeq.store(applied, std::memory_order_release); + ring.Control().Progress.appliedSeq.store(applied, std::memory_order_release); // The reader's guarantee, checked at EVERY record rather than at the end: // a batched watermark would sit at 0 here for 63 of every 64 iterations, // and a client barrier reading it would block on work that already ran. - EXPECT_EQ(ring.Control().appliedSeq.load(std::memory_order_acquire), applied); + EXPECT_EQ(ring.Control().Progress.appliedSeq.load(std::memory_order_acquire), applied); } EXPECT_EQ(applied, static_cast(kRecords)); EXPECT_TRUE(ring.Invariants()); @@ -610,13 +626,13 @@ TEST(RingTest, ALazyWatermarkMayTrailTheWorkButMustNeverLeadIt) { ++drained; // A deliberately lazy publisher: only every third record. This is legal. if (drained % 3 == 0) { - ring.Control().retiredSeq.store(drained, std::memory_order_release); + ring.Control().Progress.retiredSeq.store(drained, std::memory_order_release); } - EXPECT_LE(ring.Control().retiredSeq.load(std::memory_order_acquire), drained) + EXPECT_LE(ring.Control().Progress.retiredSeq.load(std::memory_order_acquire), drained) << "retiredSeq ran ahead of the drain; those staged bytes are still live"; } // Trailing at the end is fine and is what "late" means. - EXPECT_LE(ring.Control().retiredSeq.load(), static_cast(kRecords)); + EXPECT_LE(ring.Control().Progress.retiredSeq.load(), static_cast(kRecords)); EXPECT_TRUE(ring.Invariants()); } diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index cbe74d31..8f0329f2 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -693,11 +693,11 @@ TEST(ServerLoopTest, TheSessionWatermarkAndTheDecoderTallyAgreeAfterEveryRecord) // instantly would be asserting a promise R-9 deliberately does not make. The BOUND is what // keeps this a check: a loop that never retires times out here instead of passing. const auto retireDeadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); - while (fixture.clientSegments.CmdControl()->retiredSeq.load() < 8u && + while (fixture.clientSegments.CmdControl()->Progress.retiredSeq.load() < 8u && std::chrono::steady_clock::now() < retireDeadline) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); } - EXPECT_GE(fixture.clientSegments.CmdControl()->retiredSeq.load(), 8u) + EXPECT_GE(fixture.clientSegments.CmdControl()->Progress.retiredSeq.load(), 8u) << "the apply loop advanced appliedSeq but never retired within 2 s, so SEG_STAGE would " "never be reclaimed and the first MOBILEGL_IPC_STAGE_MB would end in " "Fatal{RingOverrun}"; diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp index 668f69e7..bc14bf2a 100644 --- a/MobileGL/MG_Test/Wire/SessionTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -146,7 +146,7 @@ TEST(SessionTest, SessionSegmentsAreRealSharedMemoryAndNotAHeapAllocation) { ASSERT_NE(segments.EventControl(), nullptr); EXPECT_EQ(segments.CmdControl()->ringGeneration.load(), 1u); EXPECT_EQ(segments.EventControl()->ringGeneration.load(), 1u); - EXPECT_EQ(segments.CmdControl()->appliedSeq.load(), 0u); + EXPECT_EQ(segments.CmdControl()->Progress.appliedSeq.load(), 0u); segments.Close(); EXPECT_FALSE(segments.Valid()); } @@ -415,7 +415,7 @@ TEST(SessionTest, TwoThreadsMoveTwentyThousandRecordsAndAgreeOnEveryOne) { EXPECT_EQ(session.Control().submittedSeq.load(), static_cast(kRecords)); // The whole point: the two sides' sequence spaces are identical after ten wraps' worth of // fillers. A side that counted a kRecPad would land here off by the number of wraps. - EXPECT_EQ(session.Control().appliedSeq.load(), static_cast(kRecords)); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), static_cast(kRecords)); EXPECT_EQ(session.consumer.AppliedSeq(), static_cast(kRecords)); EXPECT_TRUE(RingCursorsValid(session.Control(), RingCursorSet::Cmd, session.serverSegments.CmdRingCapacity())); @@ -440,7 +440,7 @@ TEST(SessionTest, SubmittedSeqIsThePublishersAndIsPurelyDiagnostic) { EXPECT_EQ(session.Control().submittedSeq.load(), seq); // It says nothing about what has been APPLIED, which is the distinction a waiter that // picked the wrong watermark would lose. - EXPECT_EQ(session.Control().appliedSeq.load(), 0u); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), 0u); } } @@ -462,7 +462,7 @@ TEST(SessionTest, AppliedSeqAdvancesExactlyOncePerRecordAndIsNeverBatched) { std::uint64_t applied = 0; while (session.consumer.ApplyOne([&](const RingRecordView&) { ++applied; })) { - EXPECT_EQ(session.Control().appliedSeq.load(), applied) + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), applied) << "appliedSeq did not move with the record; a barrier waiter would be blocked on " "work that already ran"; EXPECT_EQ(session.consumer.AppliedSeq(), applied); @@ -483,14 +483,14 @@ TEST(SessionTest, RetiredSeqMayTrailTheApplyButCanNeverOvertakeIt) { session.producer.PublishAndNotify(3); while (session.consumer.ApplyOne([](const RingRecordView&) {})) { } - ASSERT_EQ(session.Control().appliedSeq.load(), 3u); + ASSERT_EQ(session.Control().Progress.appliedSeq.load(), 3u); // Trailing is legal and is what "late" means. Watermark::AdvanceRetired(session.Control(), 1); - EXPECT_EQ(session.Control().retiredSeq.load(), 1u); + EXPECT_EQ(session.Control().Progress.retiredSeq.load(), 1u); // Running ahead is not: clamped to what has actually been applied. Watermark::AdvanceRetired(session.Control(), 99); - EXPECT_EQ(session.Control().retiredSeq.load(), 3u); + EXPECT_EQ(session.Control().Progress.retiredSeq.load(), 3u); // Going BACKWARDS is Fatal, not clamped - see AWatermarkThatMovesBackwardsIsFatal below. } @@ -507,16 +507,16 @@ TEST(SessionTest, CompletedFrameSerialIsTheServersAndIsIndependentOfAppliedSeq) session.producer.PublishAndNotify(5); while (session.consumer.ApplyOne([](const RingRecordView&) {})) { } - EXPECT_EQ(session.Control().appliedSeq.load(), 5u); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), 5u); // Five records applied, no frame completed: the two are not the same number and nothing // may derive one from the other. - EXPECT_EQ(session.Control().completedFrameSerial.load(), 0u); + EXPECT_EQ(session.Control().Progress.completedFrameSerial.load(), 0u); Watermark::AdvanceCompletedFrame(session.Control(), 2); - EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u); + EXPECT_EQ(session.Control().Progress.completedFrameSerial.load(), 2u); // Republishing the SAME serial is legal and is what a lazy publisher does. Watermark::AdvanceCompletedFrame(session.Control(), 2); - EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u); + EXPECT_EQ(session.Control().Progress.completedFrameSerial.load(), 2u); } // presentAckSerial: the only back-pressure that bounds LATENCY rather than bytes. A client @@ -546,7 +546,7 @@ TEST(SessionTest, PresentAckSerialIsWaitedOnWithGreaterOrEqualAndWakesThroughThe throttled.join(); EXPECT_TRUE(released.load()); EXPECT_EQ(result.load(), SessionWait::Reached); - EXPECT_EQ(session->Control().presentAckSerial.load(), 7u); + EXPECT_EQ(session->Control().Progress.presentAckSerial.load(), 7u); EXPECT_EQ(session->Control().producerParked.load(), 0u); } @@ -587,7 +587,7 @@ TEST(SessionTest, AFullEventRingWakesAClientParkedOnAppliedSeq) { // AND THE WATERMARK IS UNTOUCHED, which is what makes the caller's re-read the right way // to tell the two wakeups apart: a third SessionWait value would have turned every // existing `== Reached` site into a bug. - EXPECT_EQ(session->Control().appliedSeq.load(), 0u); + EXPECT_EQ(session->Control().Progress.appliedSeq.load(), 0u); EXPECT_TRUE(session->producer.EventRingIsFull()); } @@ -613,7 +613,7 @@ TEST(SessionTest, AFullEventRingWakesAClientParkedOnThePresentCredit) { parked.join(); EXPECT_TRUE(released.load()); EXPECT_EQ(result.load(), SessionWait::Reached); - EXPECT_EQ(session->Control().presentAckSerial.load(), 0u); + EXPECT_EQ(session->Control().Progress.presentAckSerial.load(), 0u); } // THE SECOND HALF: the drain that empties the ring must RING, not merely clear. A cleared flag @@ -727,7 +727,7 @@ TEST(SessionTest, AWrapFillerDoesNotAdvanceTheSessionsAppliedSeq) { EXPECT_EQ(applied, emitted); // The session's watermark - the number the barrier's waiter and every reply read use - has // to be the record count, with the fillers' bytes invisible to it. - EXPECT_EQ(session.Control().appliedSeq.load(), emitted); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), emitted); } // --------------------------------------------------------------------------- @@ -1114,7 +1114,7 @@ TEST(SessionTestDeath, AWatermarkThatMovesBackwardsIsFatalRatherThanIgnored) { alignas(4096) RingControl control{}; InitRingControl(control); Watermark::AdvanceApplied(control, 10); - ASSERT_EQ(control.appliedSeq.load(), 10u); + ASSERT_EQ(control.Progress.appliedSeq.load(), 10u); EXPECT_DEATH(Watermark::AdvanceApplied(control, 9), "Fatal\\{ProtocolCorruption, \"watermark\"\\} appliedSeq moved backwards, 10 -> 9"); } @@ -1143,16 +1143,16 @@ TEST(SessionTest, TheInprocPeerGetsItsOwnMappingOfTheSameSharedMemory) { // visible through the other, exactly as it is across two processes under spawn. EXPECT_EQ(peer.AnnouncedSize(SessionSegmentSlot::Cmd), owner.AnnouncedSize(SessionSegmentSlot::Cmd)); - owner.CmdControl()->appliedSeq.store(4242, std::memory_order_release); - EXPECT_EQ(peer.CmdControl()->appliedSeq.load(std::memory_order_acquire), 4242u); - peer.CmdControl()->presentAckSerial.store(77, std::memory_order_release); - EXPECT_EQ(owner.CmdControl()->presentAckSerial.load(std::memory_order_acquire), 77u); + owner.CmdControl()->Progress.appliedSeq.store(4242, std::memory_order_release); + EXPECT_EQ(peer.CmdControl()->Progress.appliedSeq.load(std::memory_order_acquire), 4242u); + peer.CmdControl()->Progress.presentAckSerial.store(77, std::memory_order_release); + EXPECT_EQ(owner.CmdControl()->Progress.presentAckSerial.load(std::memory_order_acquire), 77u); peer.Close(); // The owner is untouched by the peer's teardown - there is no shared ownership left whose // Close order has to be got right by hand. EXPECT_TRUE(owner.Valid()); - EXPECT_EQ(owner.CmdControl()->appliedSeq.load(), 4242u); + EXPECT_EQ(owner.CmdControl()->Progress.appliedSeq.load(), 4242u); owner.Close(); } @@ -1189,7 +1189,7 @@ TEST(SessionTest, ABorrowedSlotIsNotHandedBackUntilItIsReleased) { ASSERT_EQ(seen, 3); ASSERT_TRUE(sawBorrow) << "the producer dropped kRecBorrowSlot, so this case proved nothing"; - EXPECT_EQ(session.Control().appliedSeq.load(), 3u); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), 3u); session.consumer.RetireThrough(session.consumer.AppliedSeq()); // The reclaim point stops AT the borrowed record: only the first record's bytes came back. @@ -1378,7 +1378,7 @@ TEST(SessionTest, TheStageCursorTripleStaysDeadAcrossAWholeSession) { EXPECT_EQ(session.Control().stageRetiredTail.load(), 0u); // retiredSeq is the watermark w1's allocator reclaims behind, and it is a SEQUENCE - not one // of the three byte cursors above. - EXPECT_GT(session.Control().retiredSeq.load(), 0u); + EXPECT_GT(session.Control().Progress.retiredSeq.load(), 0u); } // The kVarTail/kRecPad collision, made harmless. Those two are bit 2 of two different flag spaces, @@ -1432,7 +1432,7 @@ TEST(SessionTest, ARecordWearingThePadBitIsDeliveredRatherThanEatenAsAFiller) { EXPECT_EQ(seenKind, 7); EXPECT_NE(seenFlags & kRecPad, 0u) << "the bit arrives intact: the kind check narrows the SKIP, it does not scrub the bit"; - EXPECT_EQ(session.Control().appliedSeq.load(), 1u); + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), 1u); // The opposite control, so that "delivered" cannot be satisfied by not skipping anything: // a header wearing kRecPad whose kind IS kRingPadRecordKind is a genuine wrap filler, still @@ -1448,7 +1448,7 @@ TEST(SessionTest, ARecordWearingThePadBitIsDeliveredRatherThanEatenAsAFiller) { } EXPECT_EQ(seen, 1) << "a genuine filler was delivered as a record: Pop's skip was removed, " "not narrowed"; - EXPECT_EQ(session.Control().appliedSeq.load(), 1u) << "a filler advanced appliedSeq (R-9)"; + EXPECT_EQ(session.Control().Progress.appliedSeq.load(), 1u) << "a filler advanced appliedSeq (R-9)"; } // The kHostSpan/kRecBorrowSlot collision, made loud. P5 implements no borrowed slots and is ruled diff --git a/docs/Disaggregated/ARCHITECTURE.md b/docs/Disaggregated/ARCHITECTURE.md index 1ab732f1..bfd1435f 100644 --- a/docs/Disaggregated/ARCHITECTURE.md +++ b/docs/Disaggregated/ARCHITECTURE.md @@ -375,7 +375,7 @@ P13：删 `SnapshotFromGLContext()` 的非 verify 分支、`MGB_CTX`、`MOBILEGL ### 11.2 `RingControl`（`Ring.h`） -一页 4 KiB，每个争用组各占一条 cache line：`SEG_CMD` 游标三元组 `cmdHead / cmdAppliedTail / cmdRetiredTail`；`SEG_STAGE` 独立三元组；三个水位 `appliedSeq`（释放 `*AppliedTail`）/ `submittedSeq`（释放 staging）/ `retiredSeq` + `completedFrameSerial`（释放 `*RetiredTail` 与 `SEG_ADOPT`）+ `presentAckSerial`；`serverEpoch`、`ringGeneration`、`consumerParked`/`producerParked`、`eventRingFull`、`eventDropped`。两个 tail 是必须的：P11 之后 server 会**借用** ring slot 而不是再拷一次。游标是单调字节计数、2 的幂掩码、永不重置。 +一页 4 KiB，每个争用组各占一条 cache line。**P6 包 `lk` 按写者重组过这一页**（`CONTRACT-P6.md` §8.3）：producer 线 `cmdHead` + `submittedSeq`（producer 写、诊断量，`Ring.h` 的 "THE FIVE WATERMARKS" 段写明无人等待它）；consumer 线 `cmdAppliedTail / cmdRetiredTail`；`SEG_STAGE` 独立三元组（P5 起故意全零，见 `Ring.h`）；**四个 consumer 写的水位收成具名成员 `Progress`**（`Transport::LinkProgress`，`ILink.h`）——`appliedSeq`（释放 `*AppliedTail`）/ `retiredSeq`（释放 staging）/ `completedFrameSerial`（释放 `*RetiredTail` 与 `SEG_ADOPT`）/ `presentAckSerial`，访问写作 `control.Progress.appliedSeq`；最后是 `serverEpoch`、`ringGeneration`、`consumerParked`/`producerParked`、`eventRingFull`、`eventDropped`。**两个 event 标志刻意留在最后这一组**：`eventRingFull` 有两个写者（server `store(1)`、client 每次 drain 一次 `exchange(0)`），放进 `Progress` 会把 client 的 RMW 落到 client 自己自旋读的那条线上。两个 tail 是必须的：P11 之后 server 会**借用** ring slot 而不是再拷一次。游标是单调字节计数、2 的幂掩码、永不重置。 记录头 `RingRecordHeader{kind, flags, size}`，kind 0 保留给 wrap 填充；`RingProducer::Reserve` 在记录会跨 wrap 边界时自动发 pad 记录；`RingConsumer::Pop` 拒绝不可能的头并置 corrupt → `Fatal{ProtocolCorruption}`；`HardDrainRing` 只在两侧静默且 ring 全空时 bump generation。 diff --git a/docs/Disaggregated/notes/p6/lk-itemisation.md b/docs/Disaggregated/notes/p6/lk-itemisation.md index e3b70847..310fa8b0 100644 --- a/docs/Disaggregated/notes/p6/lk-itemisation.md +++ b/docs/Disaggregated/notes/p6/lk-itemisation.md @@ -53,3 +53,52 @@ | `MG_Test/Wire/SessionTest.cpp` | test | 1 | **合计 41 处**（传输内 14 / 生产 9 / 测试 18）。 + +--- + +## C 实际结果（重组落地后补记） + +两个口径在本文里曾经打架：表 B 的 41（受体限定为 `control|m_control|ctrl`）与 +`a54ae47a` 提交信息里的 44（受体不限）。**以表 B 的口径为准**，差额是三处受体不是 +控制页对象的匹配；而实际改写时受体集合还要再宽一档，因为测试里普遍写作 +`session.Control()->appliedSeq` / `owner.CmdControl()->appliedSeq`。 + +最终改写：**库 15 处 + 测试 58 处 = 73 处**（按 `re.subn` 计，一行可含多处）。 + +**"开集成测试构建"的真正理由不是计数，是守卫**：`MG_IntegrationTest/Harness/SplitRuntimePeek.cpp` +的五处改写躲在 `#if defined(MGITEST_SPLIT_RUNTIME_PEEK) && !defined(__ANDROID__)` 后面。 +本轮用 red-once 证明了它确实在编译——往守卫体内塞一个未声明标识符，构建变红： + +``` +SplitRuntimePeek.cpp:99:13: error: use of undeclared identifier 'MGITEST_DELIBERATE_RED_ONCE_PROBE' +``` + +**差点踩坑的一处**：`SplitRuntimePeek.h:77` 另有一个结构体同样有 `appliedSeq` 字段， +而 `SplitRuntimePeek.cpp:146` 一行里两个都在——`state.appliedSeq = control->appliedSeq.load(...)`。 +不限定受体的改写会静默改坏左边，`MagmaRunAheadScenario.cpp` 的九处访问的正是它（那个文件一行未动）。 + +## D 验收 + +| 项 | 结果 | +|---|---| +| pull 构建（G1） | 符号 0 增 / 0 删，`.text` 10822147 不变 | +| split 构建 | 符号 0 增 / 0 删，`.text` 11681395 不变 | +| 单元测试 | 2326 / 2328，与基线同样的两个环境性失败 | +| 新增静态断言 | 逐条 red-once（下表） | + +**口径说明**：上面的 `.text` 是**尺寸**不是字节。字段偏移变了，所以指令里的立即数变了， +但代码尺寸没变。另外这套数字用的是 clang 22.1.6 且关闭 test/benchmark，**与路线图记录的 +P5f 基线（`.text 10806051`）不可比**，只能做"本包前 vs 本包后、同工具链"的比较。 + +### 断言的 red-once + +| 扰动 | 结果 | +|---|---| +| 交换 `LinkProgress` 里 `retiredSeq` 与 `completedFrameSerial` | 红：`LinkProgress field order: retiredSeq` / `completedFrameSerial` | +| 在 `LinkProgress` 内增加第五个字段 | 红：`sizeof(LinkProgress) == 32` | +| **在 `Progress` 与 `serverEpoch` 之间塞一个小成员** | **不红——已具名记在 `Ring.h` 里** | + +最后一行是本轮审查最有价值的产出。`serverEpoch` 是 `alignas(64)` 且紧随 `Progress` 声明， +所以编译器总把它放在下一个 64 倍数处：**一个塞在 288 的小成员会落在 `Progress` 自己那条线上， +而两个断言照样通过**，最多能藏 32 字节。审查建议的修法（`serverEpoch == Progress + 64`） +也抓不住——实测过。`Ring.h` 的注释现在写明了这一点，因为**一道没人能证伪的门比没有门更糟**。