[Docs] (MG_State): the early AST drop frees only the re-parsed shaders - the compile node co-owns the rest

MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h

@@ -93,11 +93,15 @@ namespace MobileGL::MG_State::GLState {
             //
             // MEMORY NOTE: this is the one thing the split makes live LONGER than it used to -
             // a glslang arena per stage, megabytes for a shaderpack, now alive from the end of
-            // phase A until phase B has generated its SPIR-V instead of dying with the link
+            // phase A until phase B runs instead of dying with the link body, so a deep
-            // body. Phase B clears this vector as soon as GlslangToSpv returns, but a deep
+            // phase-B backlog holds one arena per queued program. Phase B clears this vector
-            // phase-B backlog still holds one arena per queued program. If peak RSS ever
+            // as soon as GlslangToSpv returns, but read that call site's comment before
-            // becomes the binding constraint on a pack load, THIS is the field to attack (by
+            // relying on it: for the COMMON case (a shader linked into exactly one program)
-            // bounding the backlog, or by moving GlslangToSpv back into phase A).
+            // the compile node co-owns the same TShader and phase A pins that node, so the
+            // clear frees nothing and only the re-parsed CAS-loser shaders are actually
+            // released. If peak RSS ever becomes the binding constraint on a pack load, THIS
+            // is the field to attack - by bounding the backlog, by releasing the compile
+            // node's own reference at claim time, or by moving GlslangToSpv back into phase A.
             Vector<SharedPtr<glslang::TShader>> shaders;
             // GL enum per entry of `in.shaders`, in the same order (GetSpirvBinaryFromProgram
             // walks it to pick the intermediates).

MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp

@@ -109,11 +109,32 @@ namespace MobileGL::MG_State::GLState {
 
         MGLOG_D("ProgramObject %u: Starting SPIR-V generation", externalIndex);
         GenerateSpirv(handoff, externalIndex);
-        // GlslangToSpv was the only consumer of the parsed ASTs, and they are by far the
+        // GlslangToSpv was the only consumer of the parsed ASTs; everything after this point
-        // largest thing this node keeps alive (one glslang arena per stage, megabytes for a
+        // works on the SPIR-V and on the TProgram's own self-contained reflection pool. Drop
-        // shaderpack). Everything after this point works on the SPIR-V and on the TProgram's
+        // them here rather than at the end of the body, which is ~87% of this node's runtime
-        // own self-contained reflection pool, so drop them here rather than at the end of the
+        // earlier (spirv-opt plus routing).
-        // body - spirv-opt plus routing is ~87% of this node's runtime.
+        //
+        // WHAT THIS ACTUALLY FREES, precisely - it is LESS than "the glslang arenas", and the
+        // difference matters for the peak-RSS story:
+        //   * CAS-LOSER shaders (the re-parse in ShaderCompileTask::ClaimParsedShader, i.e.
+        //     the 2nd..Nth link of a shared shader): freed here in full. The handoff is their
+        //     ONLY owner.
+        //   * CAS-WINNER shaders (the common case - one shader object linked into one
+        //     program, which is every program of an Iris pack load): NOT freed here. The
+        //     winner branch returns a COPY of ShaderCompileTask::artifacts.shader
+        //     (ShaderCompileTask.cpp:320) and the node never releases its own reference, while
+        //     phase A holds that node through in.shaders[i].compiled for its whole life - and
+        //     phase A lives until PhaseAReleaser fires at the end of this body. So the
+        //     refcount goes 2 -> 1 here and the arena dies where it would have died anyway.
+        //
+        // Making it free the winner's arena too means releasing whatever pins the TShader
+        // inside the compile node, and neither obvious route is safe as a drive-by: moving out
+        // of artifacts.shader at claim time races ShaderObject::GetCompiledShader() on the GL
+        // thread and breaks JobNode's "a terminal node is immutable" invariant, and dropping
+        // phase A's in.shaders[i].compiled reference only helps when nothing else holds the
+        // node (the adoption map is a WeakPtr index, so it would also change which nodes stay
+        // adoptable). Both belong in a change that can be reviewed against the consume-once
+        // and adoption semantics on their own terms.
         handoff.shaders.clear();
 
         MGLOG_D("ProgramObject %u: Building global-UBO routing tables", externalIndex);