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