diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index 2598df93..06de2734 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -972,9 +972,34 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } +#if MOBILEGL_BUILD_DISAGGREGATED + // ID-54 / C7, the native half of "bind once per tuple" (v1, under ID-52/ID-59's grant for + // this file; #if-guarded so the pull build is byte-identical). Under an active transport + // this runs on the apply thread, which is the ONLY thread that ever binds the server's + // context, and the surface it is asked for was made natively current on this very thread + // by its own creation (InitPbufferSurface / InitWindowSurface -> DirectGLES::MakeCurrent). + // A second native eglMakeCurrent for the same surface is then a repeat: it rewrites the + // owner with the same thread, re-registers the same op table and invalidates seven caches + // that describe a context that did not change - the per-client-make-current storm the + // v2 review measured as "2 native binds per process, unchanged". So when the requested + // draw surface IS the active one and EGL itself says this thread holds the context + // (IsBackendContextCurrentOnThisThread re-verifies against eglGetCurrentContext), the + // native call is skipped and only the base class's bookkeeping below runs - which is + // still required: it is what InitCapabilities and SwapEGLBuffers' current-thread record + // hang off. Monolith transport in this build, and the pull build, bind exactly as before. + // Red once by making this arm unconditional: ServerLoopTest's C7 control reads 2 native + // binds at the EGL function table instead of 1. + const Bool nativelyCurrentAlready = MG_Config::Transport != MG_Config::TransportMode::Monolith && + m_eglSurfaceInitialized && m_eglSurface == draw && + DirectGLES::IsBackendContextCurrentOnThisThread(); + if (!nativelyCurrentAlready && !DirectGLES::MakeCurrent()) { + return false; + } +#else if (!DirectGLES::MakeCurrent()) { return false; } +#endif if (!BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) { (void)DirectGLES::ReleaseCurrent(); diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index f0a4bc2f..2c6187e2 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -276,12 +276,22 @@ namespace MobileGL::MG_Remote::Server { outcome.boundNatively = false; return outcome; case EglBindAction::RepeatNoOp: + // ID-54's "a no-op apart from the R-12 republish decision": the decision for an + // identical repeat is NO republish, because nothing ran that could have moved the + // caps - InitCapabilities runs inside the backend's MakeEGLCurrent, which this arm + // does not reach, so a republish here would re-send the snapshot the last real bind + // already sent. (c1's BackendObject_Remote::InitCapabilities does not lean on this + // either way: it asks the server through ServerInitCapabilities, which publishes.) outcome.ok = true; outcome.boundNatively = false; return outcome; case EglBindAction::NativeBind: break; } + // "Forwarded" is the honest word: the backend object decides for itself whether the + // driver needs a native eglMakeCurrent (BackendObject_DirectGLES skips it for a surface + // that is already current on this thread - its own creation bound it), and the C7 + // control counts THAT at the EGL function table. This counter counts forwards. outcome.ok = backend->MakeEGLCurrent(dpy, draw, read, ctx); if (!outcome.ok) return outcome; m_haveCurrentTuple = true; @@ -294,6 +304,19 @@ namespace MobileGL::MG_Remote::Server { return outcome; } + void ServerLoop::ForgetCurrentTuple() { + m_haveCurrentTuple = false; + m_curDpy = EGL_NO_DISPLAY; + m_curDraw = EGL_NO_SURFACE; + m_curRead = EGL_NO_SURFACE; + m_curCtx = EGL_NO_CONTEXT; + } + + void ServerLoop::ForgetCurrentTupleIfItNames(EGLSurface surface) { + if (!m_haveCurrentTuple) return; + if (m_curDraw == surface || m_curRead == surface) ForgetCurrentTuple(); + } + void ServerLoop::ApplyThreadMain() { m_applyThreadId.store(std::this_thread::get_id(), std::memory_order_release); NameThisThread("mgl-srv-apply"); @@ -389,6 +412,9 @@ namespace MobileGL::MG_Remote::Server { "which is the context owner"); m_backend.reset(); } + // N-3: the context died with the backend; a tuple that outlives it would make the next + // session's first make-current onto the same (recycled) handle values a RepeatNoOp. + ForgetCurrentTuple(); // C2: THE m_running CLEAR IS INSIDE THIS SAME CRITICAL SECTION AS THE FINAL DRAIN OF THE // MAILBOX. It used to be a separate store after the block, and that gap was a lost-forever @@ -578,6 +604,9 @@ namespace MobileGL::MG_Remote::Server { // thread called Stop. No context was ever made current from another thread in that // case, which is exactly the condition that makes this safe. if (m_backend != nullptr) m_backend.reset(); + // N-3, same reason as ApplyThreadMain's exit: no thread runs, so the apply-thread-only + // rule on the tuple has no other writer to race. + ForgetCurrentTuple(); m_running.store(false, std::memory_order_release); return; } @@ -674,6 +703,9 @@ namespace MobileGL::MG_Remote::Server { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; ok = backend->CreateEGLWindowSurface(surface, *handle); + // N-3: BackendObject_DirectGLES destroys and recreates the native context to + // create a DIFFERENT surface, so whatever tuple was bound names a dead context. + if (ok) ServerLoopInstance().ForgetCurrentTuple(); return MOBILEGL_OK; } } args{surface, &handle}; @@ -706,6 +738,10 @@ namespace MobileGL::MG_Remote::Server { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; ok = backend->CreateEGLPbufferSurface(surface, width, height); + // N-3: as for the window surface - a (re)creation may have destroyed the context + // the held tuple named. The surface's own creation binds natively, so the client's + // make-current that follows is forwarded and deduped one layer down (ID-54). + if (ok) ServerLoopInstance().ForgetCurrentTuple(); return MOBILEGL_OK; } } args{surface, width, height}; @@ -722,11 +758,13 @@ namespace MobileGL::MG_Remote::Server { MobileGLResult Run() { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; - // C7 / ID-54: the apply thread binds the native context ONCE per tuple and holds - // it for life. ApplyMakeCurrent forwards a real bind only for a new tuple, treats - // an identical repeat as a no-op, and records a client release-current WITHOUT - // unbinding - so "the owner slot is written once" is true here even though - // DirectGLES::MakeCurrent itself has no shortcut. + // C7 / ID-54: the apply thread binds the native context ONCE per context + // lifetime and holds it for life. ApplyMakeCurrent forwards a bind only for a + // tuple it does not hold, treats an identical repeat as a no-op, and records a + // client release-current WITHOUT unbinding; the native call for a surface already + // current on this thread is skipped one layer down (BackendObject_DirectGLES's + // ID-54 arm), which is what makes "the owner slot is written once" TRUE and + // measured (ServerLoopTest's C7 control) rather than claimed. const ServerLoop::MakeCurrentOutcome outcome = ServerLoopInstance().ApplyMakeCurrent(backend, dpy, draw, read, ctx); ok = outcome.ok; @@ -789,6 +827,12 @@ namespace MobileGL::MG_Remote::Server { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; backend->ReleaseEGLSurface(surface); + // N-3: a released surface the held tuple names may have taken the context with + // it (BackendObject::ReleaseEGLSurface -> OnEGLSurfaceReleased -> DestroyEGLContext + // once nothing holds it current). Forgetting when the base class only DEFERRED + // the destroy costs one forwarded bind; remembering when it did not would cost a + // silent no-context-current. + ServerLoopInstance().ForgetCurrentTupleIfItNames(surface); return MOBILEGL_OK; } } args{surface}; @@ -807,6 +851,12 @@ namespace MobileGL::MG_Remote::Server { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; backend->ReleaseEGLResources(); + // N-3: DestroyEGLContext just ran; the tuple names nothing. Without this a + // destroy-recreate with the same handle values (every EGL handle on this host + // is 0x1) classified as a RepeatNoOp, bound nothing, and republished no caps. + // Red once by deleting it: ServerLoopTest's recreate control reads + // NativeBindCount() == 1 where 2 is required. + ServerLoopInstance().ForgetCurrentTuple(); return MOBILEGL_OK; } } args{}; diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index d1eb0cf5..226a5e75 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -32,17 +32,30 @@ // before the client frees any emitter-owned Vector; and the join must be bounded (that test uses // 5 s) so a regression is a red test and not a hung CI job. // -// THE EGL OWNERSHIP MOVE. eglMakeCurrent runs ONCE per (dpy, draw, read, ctx) tuple on this -// thread and the context is then held for life. The "once" is NOT free at the DirectGLES layer - -// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7) - -// so ServerMakeEGLCurrent is where the dedup lives (ID-54): an identical repeat is a no-op apart -// from the R-12 republish decision, a different tuple is a real rebind, and a client -// release-current is RECORDED (NativeBindCount / ClientReleaseCount) but NOT forwarded - the apply -// thread keeps the context current until ~BackendObject_DirectGLES or context loss, which is what -// makes DirectGLES.cpp's six cache invalidations a one-off startup cost and the 16 -// IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() sites answer TRUE on the server. The -// client's nine EGL virtuals become BLOCKING control requests executed here. ReleaseEGLResources -// and ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68) +// THE EGL OWNERSHIP MOVE, AS MEASURED (ID-54; review v2 item 10 and N-3). The native +// eglMakeCurrent for a surface runs ONCE PER CONTEXT LIFETIME on this thread and the context is +// then held for life. That "once" lives in TWO layers, because one is not enough: +// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7), +// and it is reached twice per bring-up - once from InitPbufferSurface/InitWindowSurface when the +// surface is CREATED, and once more from the client's first eglMakeCurrent. So (1) +// ServerMakeEGLCurrent classifies the request against the tuple it last bound +// (ClassifyEglMakeCurrent): an identical repeat is a no-op, and the R-12 republish decision for +// it is "nothing to republish"; a different tuple is a real forwarded bind; a client +// release-current is RECORDED (ClientReleaseCount) and NOT forwarded. And (2) +// BackendObject_DirectGLES::MakeEGLCurrent, under an active transport only, skips the native call +// when the requested draw surface is the one already natively current on this thread +// (IsBackendContextCurrentOnThisThread, which is EGL ground truth). Measured at the EGL function +// table by ServerLoopTest's C7 control on a real llvmpipe context: surface creation + two +// identical make-currents + a client release + a bind after the release = ONE native +// eglMakeCurrent, ZERO native releases, and the apply thread still the owner afterwards. So +// DirectGLES.cpp's six cache invalidations run once per context lifetime rather than once per +// client make-current, and the 16 IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() +// sites answer TRUE on the server. The tuple is FORGOTTEN (N-3) on every event after which the +// native context it names may be gone - ReleaseEGLResources, ReleaseEGLSurface of the surface it +// names, a surface (re)creation, backend destruction - so a recycled handle value after a destroy +// is a real bind again and never a silent no-context-current. The client's nine EGL virtuals +// become BLOCKING control requests executed here. ReleaseEGLResources and +// ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68) // otherwise walks on while the server still holds the context. // // THE FALLBACK IS PRE-DECLARED, NOT INVENTED UNDER PRESSURE (R-1). If the context migration is @@ -130,10 +143,13 @@ namespace MobileGL::MG_Remote::Server { Uint64 DrainedRecords() const; Uint64 ParkCount() const; - // C7 / ID-54 diagnostics, read by the C7 control. NativeBindCount is how many times - // ServerMakeEGLCurrent forwarded a REAL native bind (a new tuple); ClientReleaseCount how - // many client release-current requests were recorded-and-not-forwarded. Two identical - // binds must move the first by one and the second not at all. + // C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is + // how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not + // hold); ClientReleaseCount how many client release-current requests were recorded and + // not forwarded. Two identical binds must move the first by one and the second not at + // all. The number of native eglMakeCurrent calls the DRIVER saw is a different number - + // the backend object skips the native call for a surface already current (header block) + // - and the control reads that one at the EGL function table, not here. Uint64 NativeBindCount() const; Uint64 ClientReleaseCount() const; @@ -148,6 +164,20 @@ namespace MobileGL::MG_Remote::Server { MakeCurrentOutcome ApplyMakeCurrent(MG_Backend::BackendObject* backend, EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx); + // N-3: forget the tuple ApplyMakeCurrent last bound. Apply thread only, like the tuple + // itself (the forwarders that call these run their Args::Run there). Called on every + // event after which the native context that tuple named may no longer exist - + // ReleaseEGLResources, ReleaseEGLSurface of a surface the tuple names, a surface + // (re)creation (BackendObject_DirectGLES destroys the context to create a different + // surface), backend destruction - so the next make-current with the SAME handle values + // (EGL handles are recycled; on this host every one of them is literally 0x1) is + // classified as a real bind, not as a RepeatNoOp that binds nothing, runs no base-class + // bookkeeping and republishes no caps. Forgetting is always safe: the cost of a + // forgotten-but-still-current tuple is one forwarded bind the backend object dedups + // natively; the cost of a remembered-but-dead one is a silent no-context-current. + void ForgetCurrentTuple(); + void ForgetCurrentTupleIfItNames(EGLSurface surface); + private: void ApplyThreadMain(); // Part of the apply thread's park predicate: a posted control request must be able to @@ -224,12 +254,15 @@ namespace MobileGL::MG_Remote::Server { // owner slot g_backendContextOwnerThread (DirectGLES.cpp:11865) is stamped with whatever // thread got there. So BackendObject_Remote's nine EGL virtuals - package c1's - call these // twelve, each of which is a BLOCKING control request that runs the SERVER's backend object - // on mgl-srv-apply. eglMakeCurrent then runs ONCE, on that thread, and is never released: - // g_backendContextOwnerThread is written once, DirectGLES.cpp:11933-11953's six cache - // invalidations become a one-off startup cost instead of a per-migration storm, the - // per-frame EGL re-verification stamp is permanently true, and the 16 + // on mgl-srv-apply. The native eglMakeCurrent then runs once per context lifetime on that + // thread (the surface's own creation binds; the client's make-currents onto that surface + // are deduped at both layers, header block above) and a client release is never forwarded, + // so g_backendContextOwnerThread is written once per context lifetime, DirectGLES.cpp's six + // cache invalidations run once per context lifetime rather than once per client + // make-current, the per-frame EGL re-verification stamp holds, and the 16 // IsBackendContextCurrentOnThisThread() sites plus the 16 CanTouchGLNow() sites answer TRUE - // on the server instead of silently degrading. + // on the server instead of silently degrading. Measured, not assumed: ServerLoopTest's C7 + // control counts the driver's eglMakeCurrent calls at the EGL function table. // // TWO OF THEM MUST BLOCK OR THE PROCESS TEARS ITS OWN CONTEXT DOWN UNDER ITSELF: // ReleaseEGLResources (reached from EGLImpl.cpp:326, which for DirectGLES runs