diff --git a/MobileGL/MG_Impl/Pipe/TextureEmit.h b/MobileGL/MG_Impl/Pipe/TextureEmit.h index e6e1cb50..b970c6cb 100644 --- a/MobileGL/MG_Impl/Pipe/TextureEmit.h +++ b/MobileGL/MG_Impl/Pipe/TextureEmit.h @@ -921,6 +921,35 @@ namespace MobileGL::MG_Pipe { NoteRespecified(entry, desc, accepted); } + // Give an object the server never got its record back: a create with no storage for its + // identity, the storage if it has any, and the parameters once so they land on the record + // that now exists. Returns whether that send was accepted. ONE attempt per caller - the + // caller owns the "one retry, never a loop" rule. + // THE ENTRY IS FETCHED, NOT HANDED IN: Entry is declared further down the class, so a + // parameter list up here could not name it - while this body, which the compiler treats as + // following the complete class, can. It is also the safer shape: the table can grow inside + // this call (a view's owner is acquired), and a reference taken by the caller before it + // would dangle. + Bool HealUnpublishedRecord(ITextureObject& texture, MGPipeHandle handle, Uint16 bindMask, + const MGPTextureParams& params) { + Entry& entry = EntryFor(m_textures, handle); + const MGPResourceDesc healDesc = MGPipeBuildTextureResourceDesc( + texture, handle, bindMask, /*storageDefined=*/false, kMGPipeNullHandle, + kMGPipeNullHandle, 0, 0); + NoteDesc(healDesc, /*isCreate=*/true); + PublishCreate(MGPipeKind::Texture, handle, entry, healDesc); + const auto* mipmap = MG_State::GLState::AsMipmapTexture(&texture); + const Bool hasStorage = mipmap != nullptr + ? mipmap->GetMipmapLevelCount() > 0 + : texture.GetStorageType() == MobileGL::TextureStorageType::Buffer; + if (hasStorage) { + // Can grow the table (a view's owner is acquired inside): no Entry& is held + // across it - the caller re-fetches its own afterwards. + EmitResourceRespecify(texture, MGPipeTextureRespecifyScope::WholeResource, 0, 0); + } + return MGPipeRouteSetTextureParams(params); + } + void EmitTextureParams(ITextureObject& texture) { const MGPipeHandle handle = AcquireTexture(texture.GetLifetimeId(), &texture); Entry& entry = EntryFor(m_textures, handle); @@ -985,10 +1014,30 @@ namespace MobileGL::MG_Pipe { MGPipeBuildTextureParams(texture, handle, entry.BuiltinSampler, entry.ForceParamsResync); m_lastParams = params; ++m_paramSets; + // THE FIRST USE OF AN OBJECT THE SERVER NEVER GOT (P12). The refusal branch below is + // the same repair, but it can only fire if the record was allowed to WAIT - and the + // point of this change is that a record for an object whose existence IS confirmed + // need not wait at all. So the check moves BEFORE the send: the confirming create is + // the one round trip an object owes, it is paid here at its first use, and every + // record after it is fire-and-forget (Wire_SetTextureParams reads the same latch). + // + // IT IS NOT A NEW MECHANISM: PublishCreate is what the respecify path already calls + // when !MGPipeHandleIsPublished (EmitResourceRespecify, above), and the body below is + // the one the refusal branch has always run. This runs it on the client's own + // knowledge instead of after a round trip that told the client nothing it could not + // already see. + Bool accepted = false; + Bool healed = false; + if constexpr (MGPipeTextureRecordsReachTheApplier()) { + if (!MGPipeHandleIsPublished(MGPipeKind::Texture, handle)) { + accepted = HealUnpublishedRecord(texture, handle, entry.BindMask, params); + healed = true; + } + } // Not behind MGPipeTextureRecordsReachTheApplier() (see its comment): the call is // dispatched whenever this emitter runs, so the answer is always a real one. - Bool accepted = MGPipeRouteSetTextureParams(params); - if (!accepted) { + if (!healed) accepted = MGPipeRouteSetTextureParams(params); + if (!accepted && !healed) { // THE SELF-HEAL, the respecify path's shape, and the parameters are the one // publication that may be a texture's FIRST: the context's default textures are // constructed before the backend registers its consumer, so no create ever went @@ -1013,7 +1062,7 @@ namespace MobileGL::MG_Pipe { // across it - `entry` is re-fetched below. EmitResourceRespecify(texture, MGPipeTextureRespecifyScope::WholeResource, 0, 0); } - accepted = MGPipeRouteSetTextureParams(params); + accepted = HealUnpublishedRecord(texture, handle, entry.BindMask, params); } Entry& latched = EntryFor(m_textures, handle); if (!accepted) { diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index cafe15b0..bd806aa8 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -39,6 +39,8 @@ #include #include +// P12: MGPipeHandleIsPublished, the latch that says an object's existence is CONFIRMED. +#include // P5c (ct): EmitObjectDeathRecord resolves the dying object's handle in the CLIENT's own // allocator - a client surface, asked on the client thread, which is the one place the lookup // is legal under rule E (CONTRACT-P5C.md ยง5.2). @@ -328,13 +330,44 @@ namespace MobileGL::MG_Remote::Client { if (status == 1) ++g_declined; } + // A PARAMETER CHANGE FOR AN OBJECT WHOSE EXISTENCE IS CONFIRMED OWES NO ANSWER (P12). + // + // The acceptance this row used to wait for says one thing: "the applier holds a record for + // this handle". That is a fact about the SERVER's table, and the client already keeps its + // own copy of exactly that fact - MGPipeHandleIsPublished, set when the object's create was + // ACCEPTED and never cleared while the record lives (the applier keeps texture records + // across a make-current: PipeFill.cpp:3756-3759). So the wait is asking the server a + // question the client has already answered. + // + // WHAT REPLACES IT, on the device run's numbers: this row paid 18,056 of one frame's + // 43,232 round trips at ~5 ms each, for records carrying a few hundred bytes. The object's + // ONE confirmation is its create, paid at its first use (the emitter sends it proactively + // when the latch is clear - EmitTextureParams' HealUnpublishedRecord); every parameter + // record after that is emitted fire-and-forget and the caller reads a PROVISIONAL accept, + // exactly as resource_subdata has done since item B. + // + // AND WHEN THE LATCH IS CLEAR THIS ROW STILL WAITS, which is the whole fallback: an object + // whose create was refused stays unpublished, so its next parameter record takes the real + // answer and the emitter's existing one-retry heal runs. A refusal can only reach here + // for an object the client thinks it never published, or for a bug - the two cases that + // were already the only ones the per-record wait could name. void Wire_SetTextureParams(const MG_Pipe::MGPTextureParams* payload, MG_Pipe::MGPReplySlot* reply) { if (RunsAsTheServerRole()) { MG_Pipe::MGPipeMonolithContext().SetTextureParams(payload, reply); return; } ClientSession& session = RequireSession("SetTextureParams"); - Int32 status = 0; - const Uint64 seq = session.EmitAndWait(MGPWireOp::SetTextureParams, payload, - sizeof(*payload), nullptr, 0, nullptr, 0, - &status); + const Bool confirmed = MG_Pipe::MGPipeHandleIsPublished(MG_Pipe::MGPipeKind::Texture, + payload->Res); + Int32 status = Wire::ReplySink::kStatusError; + Uint64 seq = Wire::kInvalidSeq; + if (confirmed) { + seq = session.EmitAndWaitTails(MGPWireOp::SetTextureParams, payload, sizeof(*payload), + nullptr, 0, nullptr, 0, &status, nullptr, + /*wantReply=*/false); + status = seq == Wire::kInvalidSeq ? Wire::ReplySink::kStatusDeclined + : Wire::ReplySink::kStatusOk; + } else { + seq = session.EmitAndWait(MGPWireOp::SetTextureParams, payload, sizeof(*payload), + nullptr, 0, nullptr, 0, &status); + } // Cancellation has no wire ordinal; retain the caller's nonzero local ticket. if (seq != Wire::kInvalidSeq) reply->Id = seq; MG_Pipe::MGPipePostReply(*reply, status, status == 0 ? 1u : 0u); diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index 6160b5dc..e552e1cd 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -1033,7 +1033,12 @@ TEST(RemoteClientControls, ReplyWaitsByOpForAnAtlasShapedLoad) { ::_exit(144); if (Transport::LinkMetricsReplyWaitsFor(static_cast(MGPWireOp::ResourceRespecify)) == 0) ::_exit(145); - if (Transport::LinkMetricsReplyWaitsFor(static_cast(MGPWireOp::SetTextureParams)) == 0) + // AND SetTextureParams IS DELIBERATELY NOT ON THAT LIST ANY MORE (P12). It was the control + // that proved this counter was live; the first-use confirmation moved the row to + // fire-and-forget for a confirmed object, so its zero is the FIX rather than a dead counter + // - which is why create and respecify above carry that job now, and why the records line + // below still has to show the parameter records going out. + if (Transport::LinkMetricsRecordsFor(static_cast(MGPWireOp::SetTextureParams)) == 0) ::_exit(146); Uint64 sum = 0; for (Uint32 op = 0; op < Transport::LinkMetricsMaxOps; ++op) @@ -1043,4 +1048,50 @@ TEST(RemoteClientControls, ReplyWaitsByOpForAnAtlasShapedLoad) { }); ExpectChildSuccess(child); } +// ===================================================================================== +// P12: the first-use confirmation - an object pays ONE round trip, at its first use +// ===================================================================================== +// +// The rule the device run asked for: the round trip is owed by an object's CREATE (that is what +// says "the applier holds a record for this handle"), and every control record after it is +// fire-and-forget. The latch that decides is MGPipeHandleIsPublished, which the emitter sets only +// on an ACCEPTED create - so the row was asking the server a question the client had already +// answered, at ~5 ms per record (18,056 of one frame's 43,232 waits were this row). +// +// THE RED, RUN ONCE: put the unconditional EmitAndWait back into Wire_SetTextureParams and the +// last assertion below flips - the parameter records start taking replies again. +TEST(RemoteClientControls, AParameterChangeForAConfirmedObjectTakesNoReply) { + 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(); + const Uint32 paramsOp = static_cast(MGPWireOp::SetTextureParams); + const Uint32 createOp = static_cast(MGPWireOp::ResourceCreate); + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(0x0DE1, texture); + // FIRST USE, AND THE HARD SHAPE: parameters with no storage at all - the default-texture + // case the self-heal comment names. The confirmation is a create with no storage, it is + // BLOCKING (the one answer this object owes), and the parameters land with it. + MG_Impl::GLImpl::TexParameteri(0x0DE1, 0x2801, 0x2601); // MIN_FILTER = LINEAR + MGP_FILL(DrawArrays); + if (Transport::LinkMetricsRecordsFor(createOp) == 0) ::_exit(150); + if (Transport::LinkMetricsReplyWaitsFor(createOp) == 0) ::_exit(151); + // NOW THE OBJECT IS CONFIRMED. Eight real parameter changes: every one must still be SENT, + // and none of them may take a reply. + const Uint64 recordsBefore = Transport::LinkMetricsRecordsFor(paramsOp); + const Uint64 waitsBefore = Transport::LinkMetricsReplyWaitsFor(paramsOp); + for (int i = 0; i < 8; ++i) { + MG_Impl::GLImpl::TexParameteri(0x0DE1, 0x2801, (i % 2) != 0 ? 0x2601 : 0x2600); + MGP_FILL(DrawArrays); + } + if (Transport::LinkMetricsRecordsFor(paramsOp) != recordsBefore + 8) ::_exit(152); + if (Transport::LinkMetricsReplyWaitsFor(paramsOp) != waitsBefore) ::_exit(153); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} #endif