From 33e02f4bcf0c5cdcd3ba605dd897c7ed360d4c6a Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Fri, 25 Sep 2026 19:44:38 +0800 Subject: [PATCH] [Perf] (MG_Pipe, MG_Remote/Client): on the wire arm a texture upload stops owning a reply - the wait moves from every record to the SEG_STAGE window - MGPipeSubDataWantsItsReply is the ONE place that decides whether a resource_subdata record waits for its answer (WireTables.cpp:370 and PipeRoute.h:328 both read it, and the comment beside it says that a second spelling is how the client comes to wait for an answer the emitter told the server not to bother with). The buffer half has answered "no" since P5e; the texture half now does too, ON THE WIRE ARM ONLY. - THE ANSWER IS DERIVABLE THERE, which is the argument - not that nobody reads it: * The emission is already gated on the answer's content. A texture record is produced only where FamilyIsLive(kMGPipeSubsystemTextureResources, ...) holds, and under split that gate IS CapsMirrorInstance().ServerConsumes(bit 10) (PipeFill.cpp:1719-1740) - the same published fact the applier's own acceptance belt reads (PipeApply.cpp:1408-1419, NoP4aConsumer). So the one refusal this acceptance exists to carry, "no consumer" (ID-39 / D-D5 / ID-18 M3, which is why the texture half ever asked), cannot be the answer to a record this client emitted. * What remains is a bug class, and it is loud already: a record that names no texture, declares no texels or carries no bytes is an MGP_TRIP_WIRE_REPORT (PipeApply.cpp:1201-1218, :2245-2252), and a handle the applier does not hold is counted and named (ResolveResourceIn, :721-728) with the client's own tracker preventing it from arising (PipeFill.cpp:1045-1051). * Every answer the client can still get for free, it still gets: the emitter reads this route's Bool exactly as before, so a record the CLIENT cancelled (the device-lost latch, an exhausted stage) still answers "not accepted" and the level stays dirty and is retried - D-D5's safe direction, unchanged. - THE WAIT MOVES TO THE STAGE. A run that does not fit SEG_STAGE reclaims retired bytes and parks on the stage doorbell (PipeWireCodec.cpp StageAllocate / ReclaimStagedBytes) - the one wait a run-ahead client owes, and it is proportional to the WINDOW rather than to the call count. On Minecraft 26.2's atlas frame that is the handoff's 55,428 reply waits at ~4.4 ms each: 96 MiB of uploads through a 32 MiB window. - THE GATE IS THE NUMBER ITSELF. LinkMetricsReplyWaits is added so a case can read back the same `wait_replies` the per-frame P65LinkMetrics line prints - LinkMetrics.cpp:76-80 counts exactly the records that own a reply slot. TextureUploadsStopOwningAReplyOnTheWireArm asserts that a pure glTexSubImage2D crosses the wire and moves it by ZERO, while a resource_create row - kWaitReply, and it always was - still moves it, so the zero cannot be "nothing was counted". Red-once run: with the arm test removed the child exits 142. - Monolith and every non-wire arm are untouched, and that is pinned twice: the existing RemoteRunAhead.OnlyTheTextureHalfOfResourceSubDataWantsItsReply still reads true with no arm installed, and the new test sits inside MOBILEGL_BUILD_DISAGGREGATED with the original expression as its fall-through, so a pull build preprocesses to the line it had (G1). - Verification debt, stated rather than implied: the host gate is the COUNT, not a wall clock. In-process control sessions run lockstep (the caps peer publishes no kCapRunAheadApply), so the verb barrier still waits there; the end-to-end wall-clock win is the device measurement the handoff describes and it has not been taken yet. --- MobileGL/MG_Pipe/PipeRoute.h | 48 +++++++++++++++++-- MobileGL/MG_Remote/Transport/LinkMetrics.cpp | 1 + MobileGL/MG_Remote/Transport/LinkMetrics.h | 8 ++++ .../MG_Test/Wire/RemoteClientControls.inc | 48 +++++++++++++++++++ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 1 + 5 files changed, 103 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index 5044a126..810cec73 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -298,14 +298,56 @@ namespace MobileGL::MG_Pipe { // THE BUFFER HALF DOES NOT: its Bool is discarded at the only call site there is // (MGPipeEmitResourceSubData), and because the row carries kReplySlot the client used to // wait for it anyway - one full round trip per 64 KB persistent-map block, whose answer - // nobody looked at. THE TEXTURE HALF DOES: DrainTextureSubData clears the level's dirty - // flag on an ACCEPTED reply, so its answer is load-bearing (D-D5; making that half - // fire-and-forget is a trailing item, not this package's). + // nobody looked at. + // + // THE TEXTURE HALF DOES NOT EITHER, ON THE WIRE ARM (P12, the handoff's item B), and the + // argument is that its answer is DERIVABLE there - not that anybody stopped reading it. + // DrainTextureSubData still clears the level's dirty flag on an accepted answer (D-D5), and + // it still gets one; what it does not get any more is a ROUND TRIP for it: + // + // * THE EMISSION IS ALREADY GATED ON THE ANSWER'S CONTENT. A texture record is produced + // only where FamilyIsLive(kMGPipeSubsystemTextureResources, ...) holds, and under split + // that gate IS CapsMirrorInstance().ServerConsumes(bit 10) (PipeFill.cpp:1719-1740) - the + // same published fact the applier's own acceptance belt reads (PipeApply.cpp:1408-1419, + // NoP4aConsumer). So the one server-side refusal this acceptance exists to carry - "no + // consumer", the ID-39 / D-D5 / ID-18 M3 case that is WHY the texture half ever asked - + // cannot be the answer to a record this client emitted. + // * WHAT REMAINS IS A BUG CLASS, AND IT IS LOUD ALREADY. A record whose resource target + // names no texture, or that declares no texels or no bytes, is an MGP_TRIP_WIRE_REPORT on + // the server (PipeApply.cpp:1201-1218, :2245-2252); a record for a handle the applier + // does not hold is counted and named (ResolveResourceIn, :721-728), and the client's own + // tracker prevents it by creating before it ever sends sub-data (PipeFill.cpp:1045-1051). + // * AND EVERY ANSWER THE CLIENT CAN STILL GET FOR FREE, IT STILL GETS: the emitter reads + // this route's Bool exactly as before, so a record the CLIENT cancelled (the device-lost + // latch, an exhausted stage) still answers "not accepted" and the level stays dirty and + // is retried - D-D5's safe direction, unchanged. + // + // THE WAIT DOES NOT DISAPPEAR, IT MOVES TO THE STAGE. A record whose run does not fit + // SEG_STAGE reclaims retired bytes and parks on the stage doorbell (PipeWireCodec.cpp + // StageAllocate / ReclaimStagedBytes) - the one wait a run-ahead client owes, and the one + // that is proportional to the WINDOW rather than to the call count. That is the whole of item + // B: on Minecraft 26.2's atlas frame, 55,428 reply waits at ~4.4 ms each (96 MiB of uploads + // through a 32 MiB window) become the handful of stage retirements those same 96 MiB need. + // (Measured on the host by RemoteClientControls.TextureUploadsStopOwningAReplyOnTheWireArm, + // which counts the reply slots the client opens for a batch of uploads: N -> 0.) + // + // MONOLITH IS UNTOUCHED, and the arm test is what says so: there the applier's answer is + // returned by Mono_ResourceSubData on the calling thread and costs nothing, so there is + // nothing to stop waiting for and the predicate keeps its old answer. // // The test is MGPSubData::Target == kMGPipeResourceTargetBuffer, whole field, which is the // invariant MGPipeTypes.h asserts beside the packer and which the applier's own // SubDataNamesABuffer already reads. inline Bool MGPipeSubDataWantsItsReply(const MGPSubData& record) { +#if MOBILEGL_BUILD_DISAGGREGATED + // THE WIRE ARM ONLY. Everything above is an argument about a client talking to a server + // over SEG_CMD/SEG_STAGE; a monolith run inside a split build (every unit and + // integration-gpu lane is one) keeps the answer it always gave. + if (MGPipeInstalledArm() == MGPipeRouteArm::kClientWire) return false; +#endif + // THE PULL BUILD PREPROCESSES TO EXACTLY THIS LINE AND NOTHING ELSE, which is what G1 + // compares: the arm test above sits inside the build switch, so the pull arm emits the + // expression this function has always had. return record.Target != kMGPipeResourceTargetBuffer; } diff --git a/MobileGL/MG_Remote/Transport/LinkMetrics.cpp b/MobileGL/MG_Remote/Transport/LinkMetrics.cpp index 923643f3..c4e96fac 100644 --- a/MobileGL/MG_Remote/Transport/LinkMetrics.cpp +++ b/MobileGL/MG_Remote/Transport/LinkMetrics.cpp @@ -88,6 +88,7 @@ void LinkMetricsReplyApplied(std::uint64_t startedNs) { ++w->samples; w->replyNs += ns; ++w->histogram[bucket]; } } +std::uint64_t LinkMetricsReplyWaits() { return metrics.total.waitReplies; } void LinkMetricsStageBytes(std::uint64_t bytes) { if (!metrics.active) return; metrics.current.stageBytes += bytes; metrics.total.stageBytes += bytes; diff --git a/MobileGL/MG_Remote/Transport/LinkMetrics.h b/MobileGL/MG_Remote/Transport/LinkMetrics.h index a4087f45..3fd8d1cd 100644 --- a/MobileGL/MG_Remote/Transport/LinkMetrics.h +++ b/MobileGL/MG_Remote/Transport/LinkMetrics.h @@ -9,6 +9,14 @@ namespace MobileGL::MG_Remote::Transport { std::uint64_t LinkMetricsBeginReply(bool wantsReply); void LinkMetricsReplyApplied(std::uint64_t startedNs); void LinkMetricsStageBytes(std::uint64_t bytes); + + // THE SAME NUMBER THE PER-FRAME LINE PRINTS AS `wait_replies` (P65LinkMetrics), read back + // in-process. P12 item B needs it as a GATE rather than as a log line: "the texture half no + // longer buys a round trip per record" is otherwise only observable by timing, and a timing + // is a measurement, not a gate. It reads 0 while MOBILEGL_PIPE_STATS is unset - which is + // exactly why a case that asserts 0 for one row must ALSO assert that a reply-owning row + // still counts, or it would pass on a session where nothing was counted at all. + std::uint64_t LinkMetricsReplyWaits(); void LinkMetricsPresent(); void LinkMetricsServerPresent(std::uint64_t serial); } diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index f21066a7..8789a86e 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -862,4 +862,52 @@ TEST(RemoteClientControls, DeviceLostRespecifyDoesNotDieOfTheUncarriedBytesCheck ExpectChildSuccess(child); } +// ===================================================================================== +// P12 item B: the texture half stops owning a reply on the wire arm +// ===================================================================================== +// +// THE NUMBER IS THE GATE. `wait_replies` is what the handoff measured 55,428 of in one +// Minecraft frame; LinkMetrics.cpp:76-80 counts exactly the records that OWN a reply slot, so +// it is the same quantity the per-frame P65LinkMetrics line prints. The case drives it over a +// live split session: a pure level upload must cross the wire and open NO reply wait, while a +// create row - which is kWaitReply and always was - still moves the number. +// +// THE SECOND HALF IS THE NEGATIVE CONTROL FOR THE FIRST. Without it, a session that counted +// nothing at all (metrics off, no records) would pass the half that exists to fail. +// +// THE RED, RUN ONCE: take the `kClientWire` arm out of MGPipeSubDataWantsItsReply (PipeRoute.h) +// and the upload delta becomes 1 - the child exits 142. +TEST(RemoteClientControls, TextureUploadsStopOwningAReplyOnTheWireArm) { + const auto child = RunInChild([] { + ::setenv("MOBILEGL_PIPE_STATS", "1", 1); + CapsPeer backend; + StartControlSession(&backend); + MG_Config::Features.PipePush |= kMGPipeSubsystemResources | + kMGPipeSubsystemTextureResources | kMGPipeSubsystemSamplers; + MG_State::pGLContext = MakeUnique(); + Uint8 pixels[4 * 4 * 4]; + std::memset(pixels, 0x21, sizeof(pixels)); + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(0x0DE1, texture); + // THE CONTROL ROW. resource_create is kWaitReply (PipeCalls.def), so it owns a reply + // slot and this counter MUST move on it. + const auto beforeCreate = Transport::LinkMetricsReplyWaits(); + MG_Impl::GLImpl::TexImage2D(0x0DE1, 0, 0x8058, 4, 4, 0, 0x1908, 0x1401, pixels); + MGP_FILL(DrawArrays); + if (Transport::LinkMetricsReplyWaits() <= beforeCreate) ::_exit(140); + // THE ROW UNDER TEST. A second write into the same level is a pure resource_subdata: the + // resource is created, so no create crosses, and the level is dirty again - the only + // record it can produce is the one item B is about. + const auto beforeRecords = ClientWireRecordsEmitted(); + const auto beforeWaits = Transport::LinkMetricsReplyWaits(); + MG_Impl::GLImpl::TexSubImage2D(0x0DE1, 0, 0, 0, 2, 2, 0x1908, 0x1401, pixels); + MGP_FILL(DrawArrays); + if (ClientWireRecordsEmitted() <= beforeRecords) ::_exit(141); + if (Transport::LinkMetricsReplyWaits() != beforeWaits) ::_exit(142); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + #endif diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 347e0d06..cf91cf0e 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include