mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
[MG_Remote] (P6/lk): regroup RingControl by writer, and make the asserts that pin it falsifiable
The shared control page both roles map had a mixed-writer cache line:
{appliedSeq, submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}
sat together, but submittedSeq is producer-written (Ring.h's "THE FIVE
WATERMARKS") while the other four are consumer-written. That is why no
single-writer progress block could be aliased over it, and it is what this
package fixes: submittedSeq moves up beside cmdHead, the other thing the
producer writes, and the four that remain become a named LinkProgress member -
the struct c6 declared in ILink.h - so 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.
The two event flags deliberately stay in the doorbell group. Promoting them
would put the client's per-drain exchange(0) on eventRingFull (EventRing.h:200)
onto the line the client's own spin predicate reads 1e5-1e6 times a frame.
73 call sites rewritten, 15 in the library and 58 in tests. The rewrite is
receiver-scoped rather than field-scoped, because SplitRuntimePeek.h:77 has its
own struct with a field of the same name and SplitRuntimePeek.cpp:146 has both
on one line - a field-scoped rename would have silently broken the peek struct
that MagmaRunAheadScenario reads nine times.
THE ASSERTS ARE THE PART THAT NEEDED A SECOND PASS. The first revision shipped
four aggregate asserts and an adversarial review broke them by counterexample:
swapping retiredSeq and completedFrameSerial inside LinkProgress compiled
clean, passed all four, and passed the runtime layout test too - because
appliedSeq stays at offset 0. Three wire-visible offsets on a shared page were
pinned by nothing. Nothing else in the tree covers it either: the ABI
fingerprint takes no input from this page, and sizeof(RingControl) is 4096
whatever the members do, because alignas(4096) makes it so.
So the block is now per-field as CONTRACT-P6 §8.3 actually asked, plus a
standard-layout precondition, a tail bound that alignas cannot satisfy for us,
and the LinkEventFlags mirror. Each was run red: the field swap reds the
per-field asserts by name, a fifth member inside LinkProgress reds sizeof.
One of them cannot go red, and Ring.h now says so rather than implying
otherwise. A SMALL member inserted between Progress and serverEpoch lands at
offset 288, inside Progress's own line, and serverEpoch's alignas(64) absorbs
up to 32 bytes of it - so both line asserts still pass. The review's proposed
fix does not catch it either; I tried it. A gate nobody can falsify is worse
than no gate, so the limit is written down beside the asserts.
Also: RingTest.ControlPageLayoutIsTheSharedContract said nothing about the one
field this package moved, and now asserts the producer/consumer separation and
the event flags' exclusion itself rather than borrowing the claim from Ring.h.
Ring.h grew 30 lines, so the Ring.h:NN citations this package and c6 authored
are re-anchored to named blocks instead of line numbers. ARCHITECTURE.md §11.2
is updated for the new layout, and a factual error introduced in the previous
revision of that paragraph - submittedSeq annotated as releasing staging, which
retiredSeq does - is corrected.
Gates: pull 0 added / 0 removed, .text size 10822147 unchanged; split 0/0,
.text size 11681395 unchanged; unit suite 2326/2328 with the same two
pre-existing environmental failures as the baseline. The five edits behind
MGITEST_SPLIT_RUNTIME_PEEK were proven compiled by a red-once, not assumed.
Note on the .text claim: that is a SIZE, not the bytes - field offsets moved,
so immediates moved with them. And this page's layout IS observable across
binaries; what is unobservable is behaviour within one binary. The contract
already books the layout change deliberately, before spawn exists to care.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<std::uint64_t> 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).
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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.<name>`; 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 <atomic>
|
||||
#include <cstddef>
|
||||
#include <type_traits>
|
||||
#include <cstdint>
|
||||
|
||||
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<std::uint64_t> cmdHead; // producer: bytes written
|
||||
std::atomic<std::uint64_t> submittedSeq; // producer: records published
|
||||
alignas(64) std::atomic<std::uint64_t> cmdAppliedTail; // consumer: bytes decoded/copied out
|
||||
std::atomic<std::uint64_t> cmdRetiredTail; // consumer: borrowed slots released
|
||||
|
||||
@@ -132,11 +146,16 @@ namespace MobileGL::MG_Remote::Transport {
|
||||
std::atomic<std::uint64_t> stageRetiredTail;
|
||||
|
||||
// ---- sequence / frame watermarks -------------------------------------
|
||||
alignas(64) std::atomic<std::uint64_t> appliedSeq; // records applied
|
||||
std::atomic<std::uint64_t> submittedSeq; // handed to the driver
|
||||
std::atomic<std::uint64_t> retiredSeq; // GPU finished
|
||||
std::atomic<std::uint64_t> completedFrameSerial;
|
||||
std::atomic<std::uint64_t> 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<std::uint32_t> 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<RingControl> && std::is_standard_layout_v<LinkProgress>,
|
||||
"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<std::uint64_t>::is_always_lock_free,
|
||||
"the ring cursors are shared across processes: they must be lock-free");
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<unsigned long long>(m_stageCapacity),
|
||||
static_cast<unsigned long long>(m_stageHead - m_stageTail),
|
||||
static_cast<unsigned long long>(
|
||||
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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<std::uint64_t>(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<std::uint64_t>(kRecords));
|
||||
EXPECT_LE(ring.Control().Progress.retiredSeq.load(), static_cast<std::uint64_t>(kRecords));
|
||||
EXPECT_TRUE(ring.Invariants());
|
||||
}
|
||||
|
||||
|
||||
@@ -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}";
|
||||
|
||||
@@ -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<std::uint64_t>(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<std::uint64_t>(kRecords));
|
||||
EXPECT_EQ(session.Control().Progress.appliedSeq.load(), static_cast<std::uint64_t>(kRecords));
|
||||
EXPECT_EQ(session.consumer.AppliedSeq(), static_cast<std::uint64_t>(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
|
||||
|
||||
@@ -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。
|
||||
|
||||
|
||||
@@ -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` 的注释现在写明了这一点，因为**一道没人能证伪的门比没有门更糟**。
|
||||
|
||||
Reference in New Issue
Block a user