mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-13 21:13:24 +09:00
[Docs] (P12): the no-reply flag reviewed - the arithmetic holds and the flag is cheap, but the predicate has to be "this answer will never be read", not "fire-and-forget"
An independent review of the one design door the measurement left open (stop sending
the ~51,000 answers nobody reads, so the 8-slot pool stops churning and the create
window becomes collectable). Verdict: feasible, worth doing, and the proposal AS
WRITTEN would disable the thing it exists to enable.
THE CATCH. The windowed create is wantReply=false AND its answer is read -
WireTables.cpp's drain reads exactly those seqs. A bit keyed on "fire-and-forget"
therefore stops the server answering the creates the window is waiting for: the
window goes from "cannot be collected" to "there is nothing to collect", and the
readfail counter that is the only way to diagnose it disappears with it. It fails
silently, not loudly. The predicate has to be per-record "this answer will never be
read" - true of params / respecify-escape / sub-data, false of a windowed create -
carried as a separate input from wantReply (willReadReply), with
willReadReply && !rowCarriesReplySlot rejected rather than ignored.
VERIFIED HERE, not taken on trust (the reviewer's file:line claims, checked):
- the flag space is RingRecordFlags (Ring.h:281-285), bits 0-4 taken, BIT 5 FREE,
and the header's own table (PipeWire.inc:35-65) says "bit 5 kOptional vs (nothing)
no ring counterpart" - so a new ring bit aliases kOptional, which is exactly what
the assert at PipeWireCodec.cpp:142-145 refuses;
- the three static_asserts that will trip are real and deliberate
(PipeWireCodec.cpp:106, :142-145, RingTest.cpp:740);
- WireFingerprint mixes payload-layout, catalogue, render-state digests + OpCount,
none of which a ring bit moves - so no protocol revision bump is required, and
both cross-version directions degrade to today's behaviour. That makes this
"must upgrade both ends to take EFFECT", not "must upgrade both ends not to
BREAK" - one notch cheaper than handoff-2 section 3.2's protocol surface;
- StreamLink.cpp:564-571 is the independent progress flush (>= 64 records or
>= 1 ms) and :577 is PostReply piggybacking SendProgress on the reply - so
skipping replies coarsens progress granularity. Bounded, but unmeasured.
AND IT CORRECTED TWO OF MY OWN NUMBERS. The frame's reply count is 55,903, not
59,673: op 4 ResourceDestroy carries kNone in the catalogue (PipeWire.inc:179) and
PostReply Fatals on a row with no kReplySlot, so it never answers - I had added its
3,770 records to the total. The derived figures move with it (12.3 replies per create,
0.65 of a create of survival, 6,988 pool churns), and every copy is fixed here:
Config.h, ConfigLoader.cpp, WireTables.cpp's section comment, CONTRACT-P5E.md,
CREATE-WINDOW-MEASURED.md.
TWO MORE THINGS THE FOLLOW-UP MUST CARRY, both recorded in the note's section 7:
- LinkMetricsBeginReply counts ownsReplySlot, so once the flag lands by_op 2 falls
to ~0 - and section 1 of the note tells the reader to use by_op 2 to check whether
the window engaged. That check becomes a FALSE GREEN; the flag needs a
server-side "how many times did this op answer" counter beside it.
- the ~112,000 sendmsg/frame the flag removes are on the apply thread, so the
flag's own value is a measurable claim that nobody has measured.
172/172 and 13/13 after the comment corrections.
This commit is contained in:
@@ -347,13 +347,14 @@ namespace MobileGL::MG_Remote::Client {
|
||||
// slot is reused, and the pool is kDefaultReplySlotCount = 8 slots deep. The frame posts a
|
||||
// reply for EVERY reply-owning record - not only for the rows that wait, because the server
|
||||
// cannot know which those are (ClientSession.cpp:1897-1900: "The server posts either way")
|
||||
// - and that is 59,673 replies in this one frame:
|
||||
// - and that is 55,903 replies in this one frame (op 4 ResourceDestroy is NOT counted: its
|
||||
// catalogue flags are kNone, so PostReply would Fatal on it and it never answers):
|
||||
//
|
||||
// create 4,536 + respecify 20,633 + params 18,056 + sub-data 12,671 + 3,770 + 6 + 1
|
||||
//
|
||||
// 8 slots against 59,673 replies is a pool that churns 7,459 times per frame, i.e. 13.2
|
||||
// 8 slots against 55,903 replies is a pool that churns 6,988 times per frame, i.e. 12.3
|
||||
// replies between two creates: an answer posted at create N is overwritten after EIGHT
|
||||
// replies, which is 0.61 of a create. There is no window depth for which the deferred
|
||||
// replies, which is 0.65 of a create. There is no window depth for which the deferred
|
||||
// answer still exists when the client comes back for it - the deferral is not slow, it is
|
||||
// UNREADABLE. (A 1-deep "window" is the blocking path, which is what the row does.)
|
||||
//
|
||||
@@ -364,8 +365,13 @@ namespace MobileGL::MG_Remote::Client {
|
||||
// prevent. So the default is 1: the row is the pre-window shape unless someone sets
|
||||
// MOBILEGL_IPC_CREATE_WINDOW explicitly, and the machinery below is kept as the record of
|
||||
// what was tried and what it cost (a fix would have to make a deferred answer outlive
|
||||
// 59,673 intervening replies, which means a deeper pool or a way for the client to tell
|
||||
// the server "do not answer this one" - both are wire changes, not tuning).
|
||||
// 12.3 intervening replies, which means a deeper pool, or NOT sending the ~51,000 answers
|
||||
// nobody will read - and that second one needs its predicate stated exactly, because
|
||||
// "fire-and-forget" is the WRONG one: the window's own creates are fire-and-forget and
|
||||
// the drain above reads them, so a bit keyed on wantReply disables the mechanism it is
|
||||
// meant to enable. The bit has to mean "this answer will never be read" - which is true
|
||||
// of respecify/params/sub-data and false of a windowed create. See
|
||||
// docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md section 7, reviewed).
|
||||
//
|
||||
// The gates that pin it are in MG_Test/Wire/RemoteClientControls.inc, and they still hold:
|
||||
// the window is a coherent mechanism, it is simply unusable on a frame with this much
|
||||
|
||||
Reference in New Issue
Block a user