swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Perf] (MG_Impl/Pipe, MG_Remote/Client): a parameter change for an object whose existence is confirmed takes no reply - the round trip moves to the object's FIRST USE

Browse Source 
- The device run put 18,056 of one atlas frame's 43,232 round trips on this row, at ~5 ms each, for
  records carrying a few hundred bytes. The acceptance it waited for says exactly one thing - "the
  applier holds a record for this handle" - and the client already keeps its own copy of that fact
  in MGPipeHandleIsPublished, which is set when the create was ACCEPTED and is never cleared while
  the record lives (the applier keeps texture records across a make-current, PipeFill.cpp:3756-3759).
  The wait was the client asking the server a question it had already answered.
- SO THE CONFIRMATION MOVES TO THE FIRST USE. EmitTextureParams now checks the latch BEFORE it
  sends: an unpublished object gets HealUnpublishedRecord - the same create-with-no-storage, the
  same storage respecify, the same one re-send the refusal branch has always run, extracted so both
  callers share it - and that create is the ONE blocking record the object owes. Every parameter
  record after it is emitted fire-and-forget and the caller reads a provisional accept, exactly as
  resource_subdata has done since item B. Wire_SetTextureParams reads the same latch to decide, so
  the two halves cannot disagree about whether a wait is owed.
- THE FALLBACK IS THE OLD PATH, unchanged: an object whose create was refused stays unpublished, so
  its next parameter record still takes the real answer and the existing one-retry heal still runs.
  That is also the red-once: with the unconditional EmitAndWait put back, the new case fails -
  RemoteClientControls.AParameterChangeForAConfirmedObjectTakesNoReply exits 153, having asserted
  that the eight parameter records still go OUT while none of them takes a reply.
- WHAT IS GIVEN UP, stated rather than implied: a loss the client believes is published (a bug, or a
  served-context teardown whose only caller today is a test) is now silent on the CLIENT - it stays
  counted and named on the server, which is the same residue item B accepted for the upload row.
- The measurement case's "the counter is live" control moves to create/respecify, because this row's
  zero is now the fix rather than a dead counter. 168/168 in RemoteClientTest; the unit lane is
  2617/2617. Tracks: docs/Disaggregated/notes/p12/DEVICELOST-AND-UPLOAD-WAITS.md.
This commit is contained in:
BZLZHH
2026-09-25 22:31:16 +08:00
parent c05e702d8c
commit b62709604d
3 changed files with 141 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/Pipe/TextureEmit.h
+52 -3
View File
@@ -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) {
MobileGL/MG_Remote/Client/WireTables.cpp
+37 -4
View File
@@ -39,6 +39,8 @@
#include <MG_Pipe/MGPipe.h>
#include <MG_Pipe/PipeRoute.h>
// P12: MGPipeHandleIsPublished, the latch that says an object's existence is CONFIRMED.
#include <MG_Pipe/PipeMutation.h>
// 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);
MobileGL/MG_Test/Wire/RemoteClientControls.inc
+52 -1
View File
@@ -1033,7 +1033,12 @@ TEST(RemoteClientControls, ReplyWaitsByOpForAnAtlasShapedLoad) {
::_exit(144);
if (Transport::LinkMetricsReplyWaitsFor(static_cast<Uint32>(MGPWireOp::ResourceRespecify)) == 0)
::_exit(145);
if (Transport::LinkMetricsReplyWaitsFor(static_cast<Uint32>(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<Uint32>(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<MG_State::GLState::GLContext>();
const Uint32 paramsOp = static_cast<Uint32>(MGPWireOp::SetTextureParams);
const Uint32 createOp = static_cast<Uint32>(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