Files
MobileGL/docs/Disaggregated/REVIEW.md
T

110 KiB
Raw Blame History

拆分设计评审记录(feat/disaggregated

生成于 2026-09-05,配合 PLAN.md 阅读。记录设计竞标的结论、对抗性审查发现及其处置,便于日后追溯"为什么是这个方案"。

1. 候选方案与评分

四个独立架构方案(各自从不同角度出发),三位评审按 7 项加权打分(性能/roundtrip 0.20、实现成本与风险 0.20、GL 语义完整性 0.20、跨平台 0.10、monolith 保留 0.10、可测试/可增量 0.10、复用既有工作 0.10)。

方案 角度 三位评审加权分
Replica-Server Disaggregation: a risk-first vertical slice to OpenRA-on-device, then one hard semantic at a time Risk-first incremental delivery. The server is the same libMobileGL binary running a real MG_State GLContext driven by a 8.6 / 8.8 / 8.9
MG_Mirror: a thin, delta-fed state model inside the server — split without rewriting the backends Thin server / delta-consuming backend. The server owns its own state model (MG_Mirror, ~4k lines, zero MG_State translat 6.4 / 6.3 / 6.6
MG_Wire: Disaggregating MobileGL by replaying MG_State mutations into a server-side replica GLContext Replica-state first: the server process runs an unmodified MobileGL backend against its own MG_State::pGLContext, recons 8 / 7.4 / 7.2
Wire: a replayed GL command stream over a lock-free shared-memory ring Command-stream / render-thread first. The primary channel is an SPSC lock-free shared-memory ring carrying fixed-layout 8 / 7.8 / 7.4 / 7.7 / 7.2 / 7.5

三位评审一致选择 Replica-Server / risk-firstserver = 未改动 backend + replica GLContext)作为基底,并嫁接其余方案的要点:Design 1 的 per-level serverAuthoritative 位与 composite pipeline program 处理、Design 2 的 nm --defined-only + .text size monolith 门与 DirectVulkan 内部 shader 构建期烘焙、Design 3 的 SPSC shm ring + FlatBuffers struct 热路径与 shadow-in-shm 前移。

评审指出的致命缺陷(已在综合稿中处理)

  • DESIGN 2 — the conformance gate cannot detect the failure it exists to prevent. Its generated static_asserts check is_same_v on accessor SIGNATURES and sizeof/alignof/offsetof on shared PODs. Neither verifies SEMANTICS. A mirror IsStorageDirty, a dirty-rect merge, a change-serial bump rule or a persistent-map state transition that behaves differently from MG_State compiles clean, passes every assert, and renders wrong. This is the whole risk of the design and its named mitigation does not address it.
  • DESIGN 2 — the mirror's scope is materially under-counted. I grepped MG_Backend: the backends call 15 distinct MUTATOR families on frontend objects across 94 sites, not just readers — SyncPersistentMappedRange x20 (which re-enters BufferBackendOps::FlushMappedRange, so the mirror BufferObject must reproduce the entire persistent-map state machine), MarkStorageDirty x19, AllocateStorage x8, SetInternalFormat x7, WritebackFromBackend x8, EnsureGpuResidentStorage x3, UpdateMipmapSubData. The design's ~450-line BufferObject and ~1,100-line texture estimates do not cover this.
  • DESIGN 2 — it never mentions GetProgramForDraw's composite-pipeline path. Core.cpp:592-660 joins every stage program, computes ComputeDrawProgramSignature, and on a cache miss constructs and LINKS an unnamed ProgramObject(0u), mirroring uniform values and block bindings into it. The mirror's ~700-line ProgramObject must reproduce all of it, or every program-pipeline application breaks. Unpriced.
  • DESIGN 3 — GL-name-space divergence is detected, not prevented. The whole identity model rests on both processes running the same IndexGenerator over the same call sequence, including on-demand object creation inside BindBuffer_State. The mitigation is a periodic XOR checksum of live names. That catches drift after the fact; Designs 1 and 4 prevent it structurally by carrying the name and lifetime id in an explicit create record and calling ctx.CreateBufferObject(name) directly. For a silent-corruption failure mode, prevention is the correct choice.
  • DESIGN 3 — the texture path contradicts the replay premise. Section 3.4 states pixels are already resolved client-side by ProcessTexturePixelsDataUnpack and that the record carries 'level identity plus the dirty description, not the upload plan: {name, target, level, unionBox, rectCount, rects}'. That is a delta, not a replay of glTexSubImage2D, so the generator's claim to cover the entry-point table mechanically does not hold for the texture family — and the design never specifies how the server's replica MipmapStorage obtains the bytes (it says only 'route their allocator to SEG_SHADOW the same way', which for buffers required an explicit new PipeResource kSharedShadow mode that is never specified for textures).
  • DESIGN 3 — the emit-after-error rule silently drops partial-effect calls. Skipping the record when PendingErrorCount moved is conservative for the common case but wrong for the spec-level exceptions the design itself acknowledges, and the only detector named is a CTS run at P4.
  • DESIGN 1 — hooks are hand-placed inside MG_State mutators, i.e. inside the state authority, and a missed mutator is a silent divergence with no structural detector. Ops.def plus sizeof tripwires catch SCHEMA drift, not HOOK OMISSION. The design says so honestly, but it is the largest residual risk in the winner-adjacent option and it is why Design 4's approach (emit at the ~152 already-enumerated GLFunctionsTable/BufferBackendOps boundary sites, replay via mutators on the server) is the safer placement of the same idea.
  • DESIGN 4 — the reflectionDigest is too narrow to catch the divergence it is designed to catch. It hashes uniform (name, location, type) triples plus maxUniformLocation and the XFB layout. It does not cover the generated SPIR-V itself, nor uniformIndexInTProgram, explicitProgramOpaqueBindings or storageBlocksWithoutBinding. This project's own bisect history records that glslang reflection and generation ORDER is load-bearing and that desktop byte-identity is a corpus-limited false green — so a server relink could produce different SPIR-V, pass the digest, and render wrong. Fix: extend the digest to an xxHash over the SPIR-V modules and the full LinkArtifacts field set, and make it a hard Fatal, which the design already does for the narrow version.
  • ALL FOUR — none notes that ProgramObject.h transitively includes ShaderObject.h -> ShaderCompileTask.h and SpvcSession.h (verified). Any server that links a real MG_State ProgramObject therefore pulls the shader-compile machinery whether or not it runs it. This only invalidates a binary-size argument, and only Design 2 makes one (which it solves by not linking MG_State at all), but every plan that claims a 'glslang-free server' after ProgramPublish should verify it with nm rather than assert it.
  • DESIGN 2 — the 'zero MG_State symbols in the server' gate is contradicted by three verified sites the design under-costs: VulkanRenderer.cpp:4214/4222/4290/4300 construct MG_State::GLState::ShaderObject and :4233/:4313 call ->Link(false); UniformManager.cpp:161-179 constructs 8 MG_State::GLState::TextureObject* kinds; DirectGLES.cpp:170 constructs a free-standing MG_State::GLState::SamplerObject(0). The design budgets ~145 lines for the first and '0 logic change' for UniformManager — but zero lines in UniformManager means the mirror must implement AllocateStorage / SetInternalFormat / UpdateMipmapSubData / MarkStorageDirty across 8 texture kinds with faithful MipmapStorage semantics. The cost is not eliminated, only moved into the line-count estimate it is missing from.
  • DESIGN 2 (the decisive one) — the conformance generator closes only the half of drift a compiler can see. is_same_v on accessor signatures and sizeof/alignof/offsetof on PODs cannot detect BEHAVIOURAL divergence in MipmapStorage::InsertDirtyRect's cascade-merge and the summedArea4 >= unionArea3 threshold (MipmapStorage.cpp:287-312), VecRange1D::Add's 7%-of-span gap ratio, or PipeResource::ResizeShadow's bit_ceil. The backend consumes all of these directly (Managers.cpp:4304-4310, :1891-1970), and this is precisely the area where the project has already measured a +6 ms/frame cliff between rect-list and union-box upload shapes (Managers.cpp:4311-4319). The design names drift as its 'single real risk' and then mitigates only the mechanical half.
  • DESIGN 2 — mirror sizing. Verified: TextureState 3,144 lines, ProgramState 7,992, BufferState 1,154. The ~5,350-line total mirror estimate is defensible for ProgramObject (most of ProgramState is glslang link tasks and the job graph, which the mirror does not need) but not for textures, where the code the backend depends on is behaviour rather than generation. Expect ~8-10k lines, i.e. optimistic by roughly 2x.
  • DESIGN 3 — GL names as wire identity make name-space divergence a SILENT-CORRUPTION class. The mitigation (a per-kind XOR checksum of live names every 4096 records) detects the fault up to 4096 records after it happens, i.e. after a frame or more of wrong pixels. Every other design uses never-reused GetLifetimeId() handles with explicit create/delete records, which cannot drift by construction. Mitigable by auditing every batch instead of every 4096 records, but it is a structural weakness of the entry-point-replay approach, not an implementation detail.
  • DESIGN 3 — the claim that a thread_local pGLContext changes 'zero of the 1494 call sites' is false. I count 65 non-arrow uses across MG_Impl / MG_Backend / MG_State: GL_Debug.cpp:99 (.get()), GL_Program.cpp:1630 (== nullptr), DirectGLES.cpp:146 (.get()), Managers.cpp:3608/3737/3808/4663/7120/7128/7131/8678 (truthiness), BackendObject_DirectVulkan.cpp:388/788, DirectVulkan.cpp:347-386 (MOBILEGL_ASSERT). An operator-> shim needs get(), operator bool and null comparison too. Not fatal, but the claim is, and .get() returning a thread-local pointer changes lifetime semantics that MOBILEGL_ASSERT sites depend on.
  • DESIGN 3 — composite pipeline programs are unaddressed. GLContext::GetProgramForDraw() links a NEW ProgramObject on a pipeline cache miss (Core.cpp:592-640). Under entry-point replay the server independently reaches that miss and links its own composite, allocating a name from ITS IndexGenerator — a second, undiscussed source of exactly the name-space divergence the design's audit is meant to catch. Design 1 is the only design that solves this explicitly.
  • DESIGN 3 — the 11.6-week estimate is not credible for the scope: a 682-opcode generator plus ~40 hand normalizers, a rewrite of 312 _State forwarders plus ~367 new wrappers, a TLS refactor of the state authority, a name-audit subsystem, a shm ring with mirror-mapping, SCM_RIGHTS, three adoption tiers, Android packaging AND a production Service. Read it as 16-18 weeks and re-baseline the phase gates accordingly.
  • DESIGN 1 — the ~150 recorder hooks are the least enumerable completeness surface in the replica family. Unlike GL entry points (a machine-readable 682-line macro table, verified) there is no single list of MG_State mutators to generate from; I count 76 mutator-shaped methods in Core.h alone with the remainder spread across BufferObject, MipmapStorage, VertexArrayObject, FramebufferObject, SamplerObject and ProgramObject. MGWIRE_MUTATOR_COUNT is a cardinality tripwire, not a coverage proof: a hook placed on the wrong side of a mutator, or a mutator with a side effect on a sibling object, passes it.
  • DESIGN 1 — InstallPublishedLink's Visit() + sizeof static_assert is explicitly acknowledged to miss any LinkArtifacts field change that does not alter sizeof. Since LinkArtifacts drives every uniform location and every XFB stride, a silent miss produces misrouted glUniform deltas with no diagnostic. It needs Design 4's reflectionDigest cross-check as a runtime companion.
  • DESIGN 1 — the in-process mode's std::swap(pGLContext, m_replica) around ApplyBatch is a data race if anything on the app thread touches pGLContext concurrently. It is confined to a test transport, but it makes the in-process oracle less trustworthy than Design 3's TLS or Design 4's separate-process-first ladder — and an untrustworthy oracle is worse than none when it is the primary correctness gate for Phase 1.
  • DESIGN 4 — reconciler completeness has only a TEST tripwire (Phase-2 name-for-name parity), not a build tripwire. Anything the backend gates on that the WireMirror forgets to walk diverges silently until a scenario happens to exercise it. This is the winner's single weakest point and is why grafting Design 3's generated command table and Design 2's generated read-surface assert is not optional.
  • DESIGN 4 — Phase 1's 'server relinks from source' runs glslang and a full compile pool in BOTH processes for Phases 1-4, on a platform whose existing compile pool is already clamped to 4 workers purely as an RSS ceiling (ShaderCompilePool.h:77-82). The Phase-1 device gate is a single OpenRA trace so it will pass; Minecraft would not. This should be stated as an explicit Phase-1 non-goal so nobody measures MC before Phase 5.
  • DESIGN 4 — like Design 3, it never addresses the composite pipeline link at Core.cpp:592-640. Its applier calls the backend table, the backend calls GetProgramForDraw(), and on a pipeline cache miss the SERVER links a composite ProgramObject. This must be either client-resolved (Design 1's mechanism) or explicitly banned with an assert; leaving it implicit is a latent divergence.
  • CROSS-CUTTING (credit where due) — all four designs correctly diagnose that Feat/CS-Delta-IPC never implemented SCM_RIGHTS (LocalSocketTransport.cpp:296 hardcodes fd = -1), so its data plane could not move a byte cross-process on Linux or Android; that ServerHost/main.cpp does not compile while being in the default ALL target, so the branch tip cannot build; and that the committed per-draw fprintf(stderr) at DirectGLES.cpp:+2583-2590 poisoned every measurement taken on that branch. All four schedule fd-passing in the first transport commit and all four remove the uncommitted [IBOTX]/[BUFTX] fprintfs before baselining. None of the four repeats the prior branch's inversion of landing a state-model refactor before a triangle renders.
  • D2 — TextureLevelPull is a novel synchronous reverse stall in the middle of a draw, and its dismissal is wrong. Because the mirror deliberately does not retain texel bytes, any server-side driver-object re-mint must ask the client to re-send. D2 pre-empts only one of three causes (RequireImageBindableStorage, via imageBindableHint); it dismisses full format regeneration (Managers.cpp:3950-4195) with 'already re-uploads every level today, so it is not a new cost class'. That is false across processes: in the monolith the bytes are in the same address space and the re-upload is free; in the split it is a blocking server-initiated round trip the client did not initiate and cannot predict, on a path that fires on ordinary glTexImage format changes. This is the only genuinely new stall class any of the four designs introduces.
  • D2 — the drift guard is narrower than advertised. D2 claims divergence between MG_Mirror and MG_State is 'a build error, not a review item'. The generated is_same_v/sizeof/alignof/offsetof asserts catch signature and layout drift only. They cannot catch behavioural drift in the ~1,100 lines of mirror texture logic that reimplement IsStorageDirty/MarkStorageDirty/GetStorageDirtyRegion/GetStorageDirtyRects, including the 96-rect cascade-merge and the summedArea4 >= unionArea3 fallback. A semantic change to MipmapStorage compiles clean and renders wrong.
  • D3 — 'zero of the 1494 call sites change' is not accurate, and the shim is harder than stated. I measured ~71 non-arrow uses of pGLContext (34 as a passed argument, 3 as pGLContext., plus the assignment at GLState/Core.cpp:20 and the deliberately-leaked definition at :1487). Crucially the declared type is extern UniquePtr<GLState::GLContext>&, not a pointer, so a thread-local shim must emulate operator->, get(), operator=, operator bool and reference binding, and the Init/Destroy lifetime path must be reworked. Small in absolute terms, but it is presented as free and it lands on the monolith's hottest access path.
  • D3 — the divergence oracle is disabled precisely where the divergence risk lives. D3's correctness rests on GL name-space determinism holding across 682 entry points, on-demand object creation in BindBuffer_State, internal cross-domain GLImpl:: calls, and an emit-after-error rule. Its two guards are periodic name-set checksums and MOBILEGL_WIRE_VALIDATE_SERVER — but the latter is explicitly CI-only, short-circuited in shipping builds by kPrevalidated + g_wireSkipValidation. A shipped build therefore turns a name-space divergence into undefined behaviour rather than a GL error, with silent wrong pixels as the symptom.
  • D3 — the schedule is not credible for the stated scope. P1 is budgeted at 2.0 weeks for: a generator over all 682 entry points, ~40 hand-written pointer normalizers, one-line wrappers for the ~367 entry points that have no _State counterpart, the W-AUDIT-1 internal-caller audit across MG_Impl and MG_Backend, the server-side validation oracle, name-audit records, and EmitFullSnapshot. The total 11.6 weeks is the most aggressive of the four for the largest protocol surface of the four.
  • D1 — the persistent-map defaults are inverted. For coherent persistent maps D1 ships option (b), a 4KiB-block xxHash-gated whole-mapped-range copy, as the v1 default, with option (c) 'decline the persistent bit' behind a config switch. SyncPersistentMappedRange() is invoked by the backend at draw time (Managers.cpp:1547, DirectGLES.cpp:262/4412/4666-4667/4768-4769, MultiDraw.cpp:498), so (b) puts a hash scan of a potentially large mapped range on the per-draw path. The frontend already tolerates a null AcquirePersistentMap at three sites (BufferObject.cpp:174, 439-442, 470-472) and MOBILEGL_DISABLE_LARGE_BUFFER_ADOPTION already exists, so (c) is the correct default and (b) the opt-in.
  • D1 — the wire byte budget is internally inconsistent. It states ~35KB of opcode stream for a 3000-draw frame while also specifying 24-40B draw records and a rule that binds are never coalesced. 3000 draws alone is 72-120KB before any state or bind traffic. Does not change the ranking (the volume is small either way) but it is an unverified figure presented as a budget in a design that otherwise cites line numbers.
  • D4 (winner, so worth naming) — the reflectionDigest gate is narrower than the divergence it must catch. During Phases 1-4 the server relinks from source, and the digest covers only (uniformName, location, type) triples plus maxUniformLocation and the XFB layout. The backends additionally read GetUniformTypeFacts, GetUniformSamplerOrImageUnitIndex, GetUniformBlockBinding, GetShaderStorageBlockBindingOverrides, PointSizeDemoted, GetTransformFeedbackStride and GetTransformFeedbackPackedStride. A relink divergence in any of those passes the gate silently. Widen the digest to the full backend-read reflection set before Phase 1 lands.
  • D4 (winner) — server-side RSS during Phases 1-4 is unaccounted for. Relinking from source means a second full glslang link pipeline plus its arenas in a second process, on a device where ShaderCompilePool is already clamped to 4 workers purely as an RSS ceiling (ShaderCompilePool.h:77-82) and where the project has an LMK-kill history. The exposure peaks during MC startup, which links hundreds of programs — exactly the Phase 5 workload D4 measures. Add a server-RSS acceptance bound to Phase 1, not only to Phase 5.
  • Cross-cutting (all four, but D1/D2/D4 most): none of the four commits to a measured per-frame byte or call-volume number, because none exists in the tree — MG_Util/Metrics is format arithmetic only (BufferMetrics.h:14-27, TextureMetrics.h:15-44), Tracy has zones but no TracyPlot counters, and the MC 26.3 campaign's PANDIAG/STALLDIAG instrumentation is gone. Every ring size, batch threshold, inline-vs-shm cutoff and frames-ahead credit in all four designs is therefore an estimate. Whichever design proceeds should land the byte counters in its FIRST phase, not (as D2, D3 and D4 all schedule it) in a later performance phase.

2. 对抗性审查(三个视角)

GL 语义正确性(refuted=False13 条)

  • [major] P1's day-13 on-device milestone omits the entire Android delivery chain it depends on
    • 问题:§15 P1 lists only C++ components (WireMirror, EmitTable, BackendObject_Remote, ReplicaContext, Applier, ServerLoop, ServerMain, spawn) yet its acceptance step 3 is run_android_retrace_local.py --case OpenRA --backend DirectGLES on 35d0befa in split mode. That path needs three things not in the deliverables. (a) Env plumbing: every MobileGL knob reaching the device is an explicit Intent extra threaded through five files — android-plugin/trace-replay-ci.sh:368-420 builds --es/--ez extras one by one, android-plugin/app/src/trace/cpp/trace_replay_core.cpp:134-207 is a hand-written setenv list, plus TraceReplayActivity.java, the JNI Request marshalling, and run_android_retrace_local.py:122-201. There is no generic env passthrough. (b) Server packaging: the plugin APK's native build is split between android-plugin/app/src/trace/cpp/CMakeLists.txt (app module) and the root CMakeLists.txt via implementation(project(":MobileGL")); a libMobileGLServer.so must come from the root build, and MobileGL/build.gradle sets no targets list, so the claim in §13 that AGP will package an add_executable renamed lib*.so is asserted, not verified. (c) Exec permission: the reader's SIGILL (exit 132) evidence was obtained through run-as, i.e. the runas_app domain, not from the app's own untrusted_app process, which is what the trace Activity is.
    • 修法:Move the Android delivery chain into P0 as a 30-minute spike with its own gate: build a trivial libMobileGLServer.so from the root CMakeLists, confirm AGP packages it into lib/arm64-v8a/, and have TraceReplayActivity posix_spawn it from getApplicationInfo().nativeLibraryDir and print a line — proving untrusted_app exec before any protocol work. Add a single generic --es mobilegl_env "K=V;K=V" passthrough to the trace path (one change in each of the five files) instead of a per-knob extra. Re-baseline P1 acceptance to the Linux inproc + spawn gates only, and make the device retrace the P2 exit criterion.
  • [major] Nothing stops the spawned server from taking the remote branch and forking again
    • 问题:§12 selects the split at MG_Backend/Init.cpp on MG_Config::Transport, which ConfigLoader.cpp reads from the environment (same shape as features.CoherentAsFlush = QueryEnvFlag(...) at ConfigLoader.cpp:185). §11 spawns the server with fork/execve and fd 3, so the child inherits MOBILEGL_TRANSPORT=spawn. §13 then says the server is a stub that dlopen(libMobileGL.so) + dlsym("mobilegl_server_main"); that entry must stand up a real backend, which runs MG_Backend::Init() (MobileGL/MG_Backend/Init.cpp:48-70). With the inherited variable still set, it constructs another BackendObject_Remote and spawns again — an unbounded fork chain on first GL call. The plan never states how the child's mode is forced.
    • 修法:Make mobilegl_server_main set MG_Config::Transport = Monolith before it can reach MG_Backend::Init(), AND scrub MOBILEGL_TRANSPORT/MOBILEGL_IPC_* from the child environment at spawn time (build an explicit envp rather than inheriting). Add a P0 MG_Test/Wire test that spawns a server and asserts the process tree gains exactly one child.
  • [major] MG_IntegrationTest's fork pre-flight will spawn a second, orphaned server holding the GPU device
    • 问题:MobileGL/MG_IntegrationTest/Harness/HeadlessGL.cpp:344-368 forks a child that runs the complete EGL bring-up and then _exit(step), with the comment at :364-366 stating this is deliberate — 'every atexit handler and static destructor in this address space belongs to the parent's copy of the world.' In split mode that child's bring-up reaches MG_Backend::Init() and spawns a server process; _exit runs no teardown, so that server is orphaned and lives until it notices EOF or hits MOBILEGL_IPC_IDLE_EXIT_S (default 30s per the plan's appendix). The parent then immediately brings up its own server against the same device. HeadlessGL.cpp:585-589 already names exactly this failure mode ('a leaked exclusive device, an environment the child did not have') as the reason it distinguishes 'pre-flight passed, parent failed'. §15's P1/P2 acceptance runs the whole integration suite through this path and the plan does not mention the pre-flight at all.
    • 修法:Make the server's EOF detection immediate and its exit unconditional (sub-second, not the 30s idle watchdog), and have the client spawn with the socket fd marked so _exit closes it deterministically. Add a readiness handshake with one bounded retry on device-busy so a lingering pre-flight server cannot flake the parent. Validate this specific interaction as part of P1 acceptance step 1, before any breadth work.
  • [major] Server discovery via dladdr does not work for either desktop gate
    • 问题:§11 locates the server with dladdr(&MobileGL::Initialize) → dirname → libMobileGLServer.so. But MobileGL/MG_IntegrationTest/CMakeLists.txt:31-32 sets MGL_ITEST_MOBILEGL_TARGET MobileGL_s, i.e. the integration binary links MobileGL statically on desktop, so dladdr resolves to the test executable's own path, not a library directory. For trace replay, tools/trace_replay/CMakeLists.txt:285-290 passes an explicit -DMOBILEGL_LIBRARY=$<TARGET_FILE:MobileGL>, whose directory is the MobileGL build output dir, while CMake places an add_executable in the defining directory's binary dir by default. Both P1 acceptance steps therefore fail to find the server as designed, and the plan's proposed mgl_itest_join_environment(... "MOBILEGL_TRANSPORT=inproc" ...) snippet does not set MOBILEGL_IPC_SERVER_PATH.
    • 修法:Make MOBILEGL_IPC_SERVER_PATH the primary discovery mechanism and dladdr the fallback. Set RUNTIME_OUTPUT_DIRECTORY of MobileGLServer to $<TARGET_FILE_DIR:MobileGL>, and add "MOBILEGL_IPC_SERVER_PATH=$<TARGET_FILE:MobileGLServer>" to every new ctest ENVIRONMENT list (joined via mgl_itest_join_environment with ${MGL_ITEST_COMMON_ENV}) and to the new SPLIT argument of add_trace_replay_test. Confirm the absolute path survives the CI artifact hop — .github/workflows/test.yml:174-185 rewrites only cmake paths inside CTestTestfile.cmake, not ENVIRONMENT values.
  • [major] Split mode's ban on COHERENT_AS_FLUSH invalidates the P2 gate for the two Create/Flywheel fixtures, and app-native coherent persistent maps have no mitigation at all
    • 问题:§6.8 states 'split mode must not apply MOBILEGL_COHERENT_AS_FLUSH'. tools/trace_replay/trace_cases.json has exactly two cases with coherent_as_flush: trueminecraft-1.21.1-neoforge-create-indirect-in-world and minecraft-1.21.1-neoforge-create-instancing-in-world. So P2's gate ('CI 全部 trace case … 在 Linux split 模式 SSIM ≥ 0.99' compared name-for-name against monolith) would run those two through a different buffer path in each mode, making the comparison meaningless for the two most buffer-stressing fixtures in the suite. Separately and more seriously, the plan addresses only the rewrite flag, not an application that requests GL_MAP_PERSISTENT_BIT|GL_MAP_COHERENT_BIT itself. With adoption declined in P1-P6 (§6.8 tier T2), such a map skips every early-out in BufferObject::SyncPersistentMappedRange() (MobileGL/MG_State/GLState/BufferState/BufferObject.cpp:238-250: returns early for GPU-resident, non-persistent, read-only, and FlushExplicit — a coherent persistent write map matches none of them) and reaches NotifySubData(whole mapped range) on every draw, which over IPC becomes a whole-buffer wire transfer per draw. The plan's copy-accounting table in §6.4 does not contain this row. MG_Config::Features.CoherentAsFlush defaults false (MobileGL/Config.h:174), so the ban itself is narrow — but the underlying cliff is not.
    • 修法:Two changes. (1) Run the two Create cases in split mode with the flag ON so the P2 comparison is honest, or state explicitly that they are excluded and why. (2) Add a third tier for non-adopted persistent-coherent maps in P1-P6: pull the shadow-in-shm work (currently P4.5) forward to cover this case specifically, or ship a dirty-range tracker for coherent maps, and add the row to the §6.4 copy table. Measure it on the two Create fixtures before P2 exit, not at P7.
  • [minor] The byte-identical-monolith gate is contradicted by P4.5's allocator change
    • 问题:§12 and P0's acceptance require nm --defined-only and stripped .text size on libMobileGL.so to be unchanged when MOBILEGL_BUILD_DISAGGREGATED=OFF, and §12 layer 1 says MG_Remote sources simply leave SOURCE_FILES. But P4.5 (§6.4, §15) changes PipeResource's MapAlignedAllocator and MipmapStorage's level vectors to use a shm arena for ≥256KiB — these live in MG_State, not MG_Remote, and changing a container's allocator changes the type. Unless every one of those edits is #if MOBILEGL_BUILD_DISAGGREGATED-guarded, the P0 gate goes red at P4.5 and the plan says nothing about it.
    • 修法:State that the shm arena is a guarded allocator specialization that compiles to the current MapAlignedAllocator when the option is OFF, and re-run the nm/.text gate as a phase-exit criterion for every phase (P0 through P9), not only P0.
  • [minor] Non-arrow pGLContext usage count is understated 2x, and the lifecycle sites are omitted
    • 问题:§12 and R8 say the inproc thread-local pGLContext shim must cover '约 65 处' non-arrow usages. Measured on dev@81b17c0b: grep -rn pGLContext MobileGL/ --include=*.cpp --include=*.h | grep -v 'pGLContext->' yields 133 lines. Of those only 2 are in MG_Impl (GL_Debug.cpp:99 .get(), GL_Program.cpp:1630 == nullptr); the bulk are in MG_Backend, including roughly 90 MOBILEGL_ASSERT(MG_State::pGLContext, ...) truthiness checks in DirectVulkan.cpp alone, plus DirectGLES.cpp:146 .get() and ten if (MG_State::pGLContext) guards in Managers.cpp. It also omits the lifecycle sites the shim must handle: MG_State/GLState/Core.cpp:20 (pGLContext = MakeUnique<...>()), Core.cpp:1487 (the leaked-reference definition), Core.h:564 (the extern UniquePtr& declaration), and MobileGL/Init.cpp:63 (pGLContext.reset()).
    • 修法:Correct the count and note that the shim must provide operator->, operator bool, get(), == nullptr, assignment from MakeUnique, and reset(). Since the backend-side usages are exactly the ones that must see the replica, prototype the shim against MG_Backend/DirectVulkan/DirectVulkan.cpp's assert block first — it is the densest cluster.
  • [minor] The FlatBuffers submodule stays mandatory even with a committed generated header, and the option has no guard
    • 问题:§13 says committing protocol_generated.h means 'cross-compilation never needs flatc', which is true — but the REUSE table (§14) keeps Feat/CS-Delta-IPC:MobileGL/Protocol/CMakeLists.txt's flatc resolution, and that file at :22-38 does add_subdirectory(3rdparty/flatbuffers) with FLATBUFFERS_BUILD_FLATC ON whenever MOBILEGL_FLATC_EXECUTABLE is unset — i.e. the exact NDK trap the plan says it fixes is preserved by the reuse decision. Independently, the runtime headers are still needed: the same file at :61-64 adds 3rdparty/flatbuffers/include to MobileGL_Protocol. So with MOBILEGL_BUILD_DISAGGREGATED=ON and the submodule not initialised, MG_Remote/** lands in SOURCE_FILES (§13) and the build fails with no guard, since the existing if (EXISTS .../flatbuffers/CMakeLists.txt) only wraps the Protocol subdirectory. The tree currently has 12 submodules and none is flatbuffers.
    • 修法:Do not reuse the flatc resolution block as-is: make codegen a scripts/gen_protocol.py developer target that is never part of the build graph, and delete add_subdirectory(3rdparty/flatbuffers) from the default path entirely (keep MOBILEGL_FLATC_EXECUTABLE only for the CI flatc-check step). Add an explicit guard that force-sets MOBILEGL_BUILD_DISAGGREGATED=OFF with a message(WARNING ...) when 3rdparty/flatbuffers/include is absent.
  • [minor] Ring decode has no stated bounds discipline, only the socket path does
    • 问题:§7.2 specifies magic and 64MiB length validation on read for the CTRL socket, correctly citing the prior branch's unbounded make_shared<std::vector<uint8_t>>(size) (verified at Feat/CS-Delta-IPC:MobileGL/Remote/LocalSocketTransport.cpp, the async_read header handler). But §6.3's RecHeader { kind; flags; size; } is read out of SEG_CMD, a region the peer writes concurrently, and the plan's only integrity mechanism there is the static_assert on sizeof(T) at compile time. A corrupted or truncated size lets the applier's cursor walk past the ring; a kind whose record is shorter than sizeof(T) lets it read past the record.
    • 修法:State the invariant explicitly and generate it: alongside each MGL_REC_SIZE_CHECK, emit a runtime size >= sizeof(T) && size <= remainingRingBytes && (size % 8) == 0 precondition in the applier's dispatch switch, and treat a violation as Fatal{ProtocolCorruption} rather than undefined behaviour.
  • [minor] Two small test-infrastructure mechanics the plan understates
    • 问题:(a) add_trace_replay_test names its test MobileGLTraceReplay.${CASE_NAME}.${BACKEND} (tools/trace_replay/CMakeLists.txt:330-332); a SPLIT argument as proposed in §13 would produce a duplicate ctest name for the same case+backend unless the name is extended. (b) The test command is cmake -P run_trace_case.cmake with ~18 -DTRACE_* variables; a new mode must be threaded through that script too, which the plan does not list among the files it touches. Neither is hard, but both sit on the P2 gate.
    • 修法:Extend the generated name to MobileGLTraceReplay.${CASE_NAME}.${BACKEND}${SPLIT_SUFFIX} and add -DTRACE_TRANSPORT= to the -P invocation plus its consumption in run_trace_case.cmake, listing both files in the P2 deliverables.
  • [minor] Windows: 'inherited handles remove accept/connect' does not carry over to asio's overlapped requirement
    • 问题:§11 says the Windows spawn uses 'CreateProcess + 继承句柄' and §16 R9 claims this 'completely removes accept/connect'. asio's windows::stream_handle (the transport the plan defaults to on Windows) requires an overlapped handle for its IOCP service; an anonymous pipe pair from CreatePipe is not overlapped-capable, so the pair must be constructed with CreateNamedPipeW(..., FILE_FLAG_OVERLAPPED) plus a matching CreateFileW(..., FILE_FLAG_OVERLAPPED) and only then inherited. The plan's AF_UNIX observation is correct — asio 1.38.2 defines ASIO_HAS_LOCAL_SOCKETS for everything except ASIO_WINDOWS_RUNTIME (3rdparty/asio/asio/include/asio/detail/config.hpp:1085-1092) — but that is the fallback, not the default.
    • 修法:Spell out the Windows handle-pair construction (named pipe with a GUID-unique name, both ends FILE_FLAG_OVERLAPPED, server end inherited) in §11, and keep the AF_UNIX evaluation in P6 as written.
  • [minor] mobilegl_server_main will not be dlsym-able in the shipping configuration
    • 问题:§13 makes the Android server a ~30-line stub that does dlopen(libMobileGL.so) + dlsym("mobilegl_server_main"). But CMakeLists.txt:498-510 sets C_VISIBILITY_PRESET hidden / CXX_VISIBILITY_PRESET hidden / VISIBILITY_INLINES_HIDDEN ON on the shared target for every non-Debug build — which is exactly the RelWithDebInfo configuration the plugin and FCL ship (MobileGL/build.gradle's fordebug type forces -DCMAKE_BUILD_TYPE=RelWithDebInfo). The symbol will not be exported unless it is explicitly annotated, so this works in a Debug build and silently fails on device.
    • 修法:Declare the entry point extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int, char**) (and add it to MG_Impl/DyldInterpose/ExportedSymbols.txt and wgl.def equivalents if those platforms ever host a server), and add a nm -D | grep mobilegl_server_main assertion to the P0 acceptance alongside the existing nm --defined-only gate.
  • [minor] Phase effort is optimistic where it matters most, and the plan's own risk register does not cover schedule
    • 问题:P0 = 3 days covers SCM_RIGHTS fd passing (hand-rolled sendmsg/recvmsg on asio's native handle), a four-platform shm layer, the SPSC ring with RingControl, validating framing, the committed-header flatc pipeline with a CI diff gate, a code-generating coverage assert with a second CI diff gate, the RenderbufferObject change, working-tree cleanup, and Tracy byte counters. P1 = 10 days covers the entire client (WireMirror, EmitTable, EmitBufferOps, BackendObject_Remote, CapsMirror, ClientArrayBounds, CompositeResolver) and the entire server (ReplicaContext, Applier, ServerLoop, ServerMain, spawn), plus — implicitly, per finding 1 — the whole Android delivery chain. For calibration, Feat/CS-Delta-IPC produced 6,668 lines across 10 commits and never rendered a frame; its own HANDOFF records four days lost to a non-reproducible regression. The 74-day total is internally consistent (3+10+8+3+5+5+4+6+6+8+6+10=74) but the front-loaded milestone is the weakest claim in the document, and §16 has no schedule risk row.
    • 修法:Split P1 into P1a (client emit + inproc applier + Linux inproc gate, 6 days) and P1b (spawn transport + Linux spawn gate, 4 days), and make the device retrace a P2 exit criterion. Add a schedule row to §16 whose mitigation is the P2.5 falsification gate already in the plan — it is the right instrument, it is simply not linked to the schedule risk it retires.

已验证的优点:

  • The core architectural decision (D1: server runs a real replica GLContext driven by mutator replay, backends untouched) is well-founded and the evidence cited for it checks out. Composite pipeline programs really are anonymously linked at MobileGL/MG_State/GLState/Core.cpp:644 (MakeShared<ProgramObject>(0u)), which is exactly the gap §5.7 identifies and solves.
  • Every DROP claim about Feat/CS-Delta-IPC verified true. MobileGL/ServerHost/main.cpp really writes interface_.ops.Start(&interface_, &config) on a MobileGLTransport* (compile error, branch tip cannot build ALL). MobileGL/Remote/LocalSocketTransport.cpp really has asio::async_write(stream, asio::buffer(next), [this, next](...)) where next is a local moved-from vector (use-after-free on every send), really allocates std::make_shared<std::vector<uint8_t>>(size) straight from the wire length with no cap, and really hardcodes out->fd = -1 in PollOffer — so the 'no POSIX fd passing, no Linux/Android data plane' conclusion is correct.
  • The Android flatc trap is real and correctly diagnosed: Feat/CS-Delta-IPC:MobileGL/Protocol/CMakeLists.txt:25-38 does add_subdirectory(3rdparty/flatbuffers) with FLATBUFFERS_BUILD_FLATC ON whenever the override is unset, while the root hook guards only CS.cmake with NOT ANDROID.
  • The FCL process-model correction is right and materially changes the design: FCL/src/main/AndroidManifest.xml:113 declares .activity.JVMActivity with no android:process, and the only :jvm entry is com.tungsten.fclcore.download.ProcessService at :137-141. The game really does run in the main process, so a second process must be created.
  • asio 1.38.2 is vendored (3rdparty/asio/asio/include/asio/version.hpp: ASIO_VERSION 103802) and does define ASIO_HAS_LOCAL_SOCKETS on Win32 (detail/config.hpp:1085-1092, excluded only for ASIO_WINDOWS_RUNTIME), so the plan's Windows transport reasoning starts from a correct premise.
  • The 'one hook point' claim (D2) is accurate: MobileGL/MG_Backend/Init.cpp:48-70 is a single switch on MG_Config::ActiveBackendType followed by InitSpecificBackendLibs(), which is the only place gBackendFunctionsTable and pActiveBackendObject are assigned. A single guarded branch there really does cover the whole boundary with no #ifdef at the ~250 downstream call sites.
  • RenderbufferObject genuinely lacks GetLifetimeId() (no match under MobileGL/MG_State/GLState/RenderbufferState/), so the P0 item is real and not busywork.
  • The integration-test extension point is exactly as described: mgl_itest_join_environment exists (MG_IntegrationTest/CMakeLists.txt:306), and the comment at :339-343 states verbatim that a ctest ENVIRONMENT property REPLACES rather than appends and that every list must build on MGL_ITEST_COMMON_ENV. Eleven gtest_discover_tests registrations already follow that shape, so a Split lane per backend is a genuine one-registration change.
  • add_trace_replay_test really does set an ENVIRONMENT property per test (tools/trace_replay/CMakeLists.txt:353-360), so threading a transport variable through the trace lane is mechanically available.
  • The P1-P4 'server relinks from source' scheme has the inputs it needs: ProgramObject::GetLinkedShaderSnapshot() exists (ProgramState/ProgramObject.h:157) and deliberately holds SharedPtrs to the linked shaders (comment at :1716), so shader sources survive glDeleteShader and can be shipped.
  • MG_Config::Features.CoherentAsFlush defaults to false (MobileGL/Config.h:174), so §6.8's prohibition is a narrow, low-blast-radius rule rather than a default flip — and the reasoning behind it is correct, since BufferObject::SyncPersistentMappedRange (BufferObject.cpp:238-250) early-returns on FlushExplicit exactly as the plan assumes.
  • The working-tree hygiene item is real: MobileGL/MG_Backend/DirectGLES/{DirectGLES,Managers}.cpp are the only two modified files in the tree, and P0's insistence on removing per-draw instrumentation before any measurement is the correct lesson from the prior branch's poisoned measurements.

性能与异步(refuted=True13 条)

  • [fatal] Persistent-mapped writes are severed: no map/unmap delta exists, and SyncPersistentMappedRange has zero client-side callers
    • 问题:Plan §5.3's trigger→delta table has no map/unmap state at all, and §6.8 defers adoption (AcquirePersistentMap returns nullptr) until P7, so every persistent map stays shadow-backed in P1-P6. The push-down for a shadow-backed persistent map is BufferObject::SyncPersistentMappedRange (MobileGL/MG_State/GLState/BufferState/BufferObject.cpp:238-250), whose first line is if (!m_isMapped) return; and whose last line is NotifySubData(m_mappedRange.start, ...). grep -rn SyncPersistentMappedRange MobileGL/ returns callers ONLY inside MG_Backend/: DirectGLES.cpp:262,4412,4666,4667,4768,4769; Managers.cpp:1547; MultiDraw.cpp:498; DirectVulkan.cpp:290,481,895; UniformManager.cpp:2022; VkBufferManager.cpp:573,620; VulkanRenderer.cpp:3432,3511,3826,7070,12015,12016. There is not one call in MG_Impl or MG_State. In the split that backend code runs on the SERVER against the replica, whose BufferObject::m_isMapped is false (no map delta was ever sent), so it returns immediately; and nothing on the client ever calls it. Worse, the backend's clean-check explicitly depends on the map bit: Managers.cpp:1446-1447 // A live non-zero-copy map may owe a per-draw SyncPersistentMappedRange push / if (frontend->IsMapped()) return false; — the replica reports the buffer clean and skips the sync entirely. Result: writes made through glMapBufferRange(PERSISTENT|WRITE) without FLUSH_EXPLICIT are silently lost. This is the exact failure class the project already burned a campaign on (memory note flywheel-indirect-lessons: 'unflushed persistent maps' as root cause #1 of the Create/Flywheel fix). It is not a tuning problem — a whole delta kind is missing from the design. The naive repair is also a performance trap the plan never budgets: SyncPersistentMappedRange emits the WHOLE mapped range every draw, so a persistently-mapped chunk arena with adoption disabled (the P1-P6 default) becomes a per-draw whole-range copy into SEG_STAGE plus a per-draw whole-range record.
    • 修法:Add map/unmap to the delta model: RecBufferMap{handle, range, accessFlags} and RecBufferUnmap{handle} emitted from glMapBuffer*/glUnmapBuffer, so the replica's m_isMapped/m_mappedRange/m_mappingAccess track the client's and both IsBufferDrawClean's IsMapped() gate and the server-side SyncPersistentMappedRange push behave as in monolith. Then make the CLIENT own the range narrowing that the whole-range push lacks: track dirty 64KiB blocks of the mapped span (the same block watermark P4.5 already proposes for WAR) and emit only touched blocks as RecBufferSubData, so the replica's push is a no-op. Add a Split integration scenario that maps PERSISTENT|WRITE|COHERENT without FLUSH_EXPLICIT, writes, draws, and reads back — today no gate in the plan would catch this.
  • [fatal] Zero-timeout sync/query polls answered from a local watermark livelock: nothing publishes the ring, and fence completion becomes present-granular
    • 问题:Plan §8 and D4 answer GetSyncStatus, ClientWaitSync(timeout=0), IsQueryResultAvailable and GetQueryResult64(wait=false) from a single acquire load on RingControl, with fence/query handles minted client-side and emitted fire-and-forget. Two independent breakages. (a) LIVELOCK: §7.2's Publish() triggers are 64KiB of records, SEG_STAGE below 1/4, any blocking request, Present, eglMakeCurrent, glFlush. A locally-answered poll is none of these. So the canonical LWJGL/Sodium idiom do { r = glClientWaitSync(s, GL_SYNC_FLUSH_COMMANDS_BIT, 0); } while (r == GL_TIMEOUT_EXPIRED); never publishes the ring, the server never sees the RecFenceSync record, the watermark never moves, and the loop spins forever. The repository documents that this flush is load-bearing: DirectVulkan.cpp:1150-1156 — 'GL_SYNC_FLUSH_COMMANDS_BIT: flush regardless of timeout, so a zero-timeout poll loop makes progress across calls'. MG_Impl forwards the flags unconditionally (GL_Sync.cpp:96 return backendClientWaitSync(syncObject->backendHandle, flags, timeout);), so the client cannot claim the app didn't ask. (b) GRANULARITY: the watermarks the plan proposes (retiredSeq / completedFrameSerial) are advanced on DirectGLES only inside Present() — DirectGLES.cpp:10626-10643 polls the 4-deep g_frameFenceRing after eglSwapBuffers — or inside WaitForFrameSerialCompleted (:10583). A fence created mid-frame therefore reports unsignalled until the NEXT present retires, i.e. fence completion degrades to frame-count inference. DirectVulkan.cpp:1120-1128 states in so many words that this is the bug that was fixed: 'The fence is signaled once that submission's VkFence has been observed signaled, so completion tracks the GPU itself rather than the frame-count inference; MC 1.21.5's fence-paced ring buffers depend on this to recycle their space instead of growing without bound.' The project memory note magma-mc1215-fence-oom records the consequence as a shipped native-heap OOM kill. The plan reintroduces it structurally.
    • 修法:(a) Make any ClientWaitSync/GetSynciv call carrying GL_SYNC_FLUSH_COMMANDS_BIT an unconditional non-blocking Publish() (release-store head + doorbell if parked) before it answers locally, and add an escalation: after N consecutive locally-answered TIMEOUT_EXPIRED on the same handle, promote to a blocking request. Do the same for IsQueryResultAvailable. (b) Do not resolve fences against a present watermark. Give each RecFenceSync a server-side real backend FenceSync() and publish EvFenceSignaled{handle} from the server's existing per-fence poll; the client's local fast path must be 'handle <= a watermark the server derived from actual per-fence retirement', which on DirectGLES means the server polls its own live syncs outside Present too (it already has WaitForFrameSerialCompleted's fence-selection logic at DirectGLES.cpp:10586-10600 to build on).
  • [fatal] MarkGpuWritten modelled as a server→client event is a read-after-write race, not an optimisation
    • 问题:Plan §5.6 routes MarkGpuWritten/EnsureGpuResidentStorage back to the client as EvGpuWritten and claims it is 'better than monolith' because the server can name only ranges a shader actually wrote. That inverts the ordering the flag exists to provide. In monolith the flag is set SYNCHRONOUSLY inside the draw call, before it returns: MarkShaderStorageBuffersGpuWritten (DirectGLES.cpp:459-467) walks GetTouchedBufferBindingPointCount(ShaderStorage) and calls obj->MarkGpuWritten(), and is invoked from SyncNeccessaryBuffers on the draw path (DirectGLES.cpp:687,697); likewise SyncAtomicCounterBuffers (:509) and MarkWritableImageBufferTexturesGpuWritten (:1809). In the split the draw is fire-and-forget, so glDrawElements(...); glMapBufferRange(GL_SHADER_STORAGE_BUFFER, ..., GL_MAP_READ_BIT); runs entirely on the client before the server has even applied the draw. AcquireMemoryRange calls SyncGpuWrites() (BufferObject.cpp:454), SyncGpuWrites returns immediately because m_gpuWritePending is false (BufferObject.cpp:266), and the app gets the STALE shadow with no error and no round trip. Compounding it, §7.4's event-drain points are glGetError, glGetQueryObject*, glClientWaitSync, eglSwapBuffers and kNeedsAck waits — glMapBuffer*, glGetBufferSubData and glGetNamedBufferSubData (GL_Buffer.cpp:957,995) are not on the list, so even a late-arriving event would not be observed. This breaks an entire family of existing gates the plan schedules for P4 (SsboArrayLengthScenario, AtomicCounterScenario, StorageBufferRegrowScenario) in a way that is data-dependent and will look like flakiness.
    • 修法:Invert the direction. The client already has every input MarkShaderStorageBuffersGpuWritten uses (GetTouchedBufferBindingPointCount / GetBufferBindingPoint), so WireMirror must set MarkGpuWritten() locally and conservatively at draw/dispatch emit time, mirroring DirectGLES.cpp:459-467, :509 and :1809 exactly. EvGpuWritten{handle, ranges[]} then becomes a pure narrowing hint that can clear the flag or shrink the readback range, and arriving late is harmless. Separately, add glMapBuffer/glMapBufferRange/glGetBufferSubData/glGetNamedBufferSubData to §7.4's drain-point list.
  • [major] §6.4's copy table understates both monolith and split; the real split cost is 4 copies (3 after P4.5), not 2 (1)
    • 问题:The table in §6.4 claims 'glBufferSubData → shadow store' is 1 copy in monolith, 2 in P1-4, 1 at P4.5. All three numbers are wrong. Monolith is already 2: (1) app→shadow in BufferObject::UploadSubData's Memcpy, then (2) shadow→destination inside FlushPendingRangesNow, which is Memcpy(dst, bufferObject.MappedData() + start, size) into an invalidating map (Managers.cpp:914) or Memcpy(g_uploadRing.store.mappedPtr + ringOffset, bufferObject.MappedData() + start, size) into the upload ring (Managers.cpp:922). Split P1-4 is 4: app→client shadow (1), client shadow→SEG_STAGE (2), then on the server the applier replays the mutator, so BufferObject::UploadSubData memcpys SEG_STAGE→REPLICA shadow (3), and the server's unchanged FlushPendingRangesNow then memcpys replica shadow→upload ring (4). P4.5 shadow-in-shm removes only copy (2), leaving 3 — it cannot remove (3), because SEG_SHADOW is client-owned/server-read-only by §6.1 while the replica BufferObject owns its own PipeResource allocation. Reaching the claimed 1 would require the applier to hand the backend ops the shm pointer directly instead of calling BufferObject::UploadSubData, which destroys the 'applier = mutator replay, therefore side-effect-identical' invariant that risk R1 rests on, and bypasses the change-serial bump IsBufferDrawClean compares (Managers.cpp:1453). The map path is worse still: glMapBufferRange(WRITE)+unmap is already 3 in monolith (seed staging from shadow at BufferObject.cpp:487, staging→shadow at :200-202, shadow→ring) and becomes 5 in the split. At the plan's own MC pan figure of ~9 MB/frame of section-mesh writes this is 27-36 MB/frame of memcpy, ~1.6-2.2 GB/s of phone memory bandwidth at 60 fps, against a monolith baseline of ~18 MB/frame.
    • 修法:Correct the table and re-derive the P4.5 target. Either (a) accept 3 and say so, or (b) give the replica BufferObject a PipeResource mode that ADOPTS the client's SEG_SHADOW mapping read-only — a third PipeResource state alongside shadow and gpuMapped, where Bytes() returns the mapped client segment — so the applier's UploadSubData becomes a no-op range note and only the server's ring copy remains (1 copy end to end). That keeps mutator replay intact for every side effect except the byte move. Whichever is chosen, put the TracyPlot byte counters from P0 on BOTH sides of the wire and gate P4.5 on the measured total, not on the client-side number alone.
  • [major] The 64 KiB publish threshold serialises the two halves and pre-emptively kills the P2.5 hypothesis
    • 问题:§7.2 sets Publish() at 'records ≥ 64KiB', SEG_STAGE below 1/4, blocking request, Present, eglMakeCurrent, glFlush. With the §6.3 record sizes (RecDrawArrays 32B, RecBindBuffer 24B, RecDrawElements 56B) 64 KiB is roughly 1200-2700 records — i.e. an entire Minecraft frame, which the plan itself sizes at 1000-4000 draws. The server therefore cannot begin a frame's work until the client has finished emitting it. That is not asynchrony; it is a pipeline with a one-frame bubble, and it adds a full frame of latency on top of the present credit. It also invalidates P2.5 before it runs: the stated purpose of inproc is to move PrepareForDraw off the GL thread so the two overlap, and a frame-granular publish guarantees zero overlap within a frame. There is no throughput reason for the threshold either — SEG_CMD is an SPSC ring, so 'publishing' is a release store of head; the only thing worth amortising is the doorbell write, and §6.2 already gates that on consumerParked.
    • 修法:Delete the byte threshold. Release-store head every record (or every 8-16 records to amortise the store), and ring the doorbell only when RingControl.consumerParked is set. Keep Present/blocking-request/glFlush as explicit doorbell points. Then measure the doorbell rate with the P0 Tracy counters; if the wakeup rate is the problem, raise the consumer's spin window rather than delaying the producer.
  • [major] No wakeup path for any client-side wait: present credit, readback replies and ring-full escalation must all busy-spin
    • 问题:§7.3 states explicitly: 'server 端不发 credit 消息:它对 RingControl 做 release store'. §6.2 specifies a doorbell only for the CONSUMER (consumerParked + a 1-byte socket write from the producer). There is no producer-side park/wake, so every place the client waits has nothing to block on: the present-credit wait in eglSwapBuffers when presentsSent - presentAckSerial >= 2 (§9), every kNeedsAck blocking request (readback, ClientWaitSync>0, GetQueryResult64 wait=true, AcquirePersistentMap at P7), and the §6.5 escalation's 'bounded 50ms wait on the oldest unretired batch'. All of them reduce to polling a shared cache line across a process boundary. On the target hardware a present-credit wait is up to a full frame (16.6 ms at 60 Hz) of spinning; on Android that is a big core held at full clock against the GPU and the game JVM, and the codebase has no affinity control to keep it off a little core (grep -rn 'sched_setaffinity\|cpu_set_t' MobileGL/ → 0 hits). The 50 ms escalation wait is a 50 ms spin. This directly contradicts the plan's own framing that the client merely 'blocks on a socket/futex read instead of vkWaitForFences'.
    • 修法:Add the symmetric doorbell: a producerParked flag in RingControl plus a second byte-stream direction (the socketpair already exists — reserve one byte code for 'watermarks advanced'). Client waits become spin-N-microseconds → set producerParked → blocking read on the socket; server does a release store then, only if producerParked, one byte. Specify a bounded spin (e.g. 50 µs, tuned per phase) and make the spin budget a config knob so it can be measured on 35d0befa and 3B159D009VZ00000 rather than guessed.
  • [major] SEG_EVENT has no overflow policy: a full event ring while the client waits on present credit is a two-sided deadlock
    • 问题:§6.1 sizes SEG_EVENT at 256 KiB, server-owned, client-read-only, and §7.4 has the server produce EvQueryResult, EvFenceSignaled, EvGpuWritten, EvBufferWriteback, EvReadbackDone, EvGlError, EvDefaultFramebufferInfo, EvCompileEnvInvalidate and — unbounded — EvLogLine{level,len,text}, with the server's MGLOG and deferred diagnostics replayed 'in stream order into the client log stream'. The plan never says what the server does when that ring is full. It also never says the client drains it while WAITING, only 'at each entry point where it could observe them (…eglSwapBuffers)'. Concrete deadlock: the client is inside eglSwapBuffers waiting on presentsSent - presentAckSerial >= 2; the server's mgl-srv-apply thread emits log lines and EvGpuWritten while applying; SEG_EVENT fills; the apply thread blocks producing; presentAckSerial never advances; the client never leaves eglSwapBuffers, so it never drains. Both halves are stuck. This is precisely the 'client blocked on credit while server blocked on the client' shape, and the plan's risk table (R1-R13) does not contain it.
    • 修法:State an explicit policy: (1) the client MUST drain SEG_EVENT inside every wait loop (present credit, kNeedsAck, ring escalation), not only on entry-point boundaries; (2) EvLogLine is lossy — overwrite-oldest with a dropped-count field, since losing a log line must never stall rendering; (3) semantically load-bearing events (EvGpuWritten, EvReadbackDone, EvFenceSignaled, EvBufferWriteback, EvGlError) are non-lossy, and when the ring cannot take one the server sets an eventRingFull flag in RingControl and stops APPLYING rather than blocking mid-record, so the state is recoverable; (4) add a fault-injection test that fills SEG_EVENT while the client is credit-blocked, alongside P8's SIGKILL test.
  • [major] Frames of lag compose: present credit 2 sits on top of the backend's own 2-3, giving 4-5 frames end to end
    • 问题:§9 sets the present credit to 2 and argues it 'mirrors the existing budget' (MagmaFramesInFlight=3 clamped to [2, maxImageCount], Espryt's 4-deep fence ring at DirectGLES.cpp:10071-10074) and therefore 'introduces no new stall class'. The stall CLASS is indeed not new, but the LATENCY composes and the plan never adds it up. The server's own Present already blocks 2-3 frames deep before it returns: VulkanRenderer::Present ends by calling FrameContext::WaitAndAcquireNextImage, whose first statement is vkWaitForFences(device, 1, &frame.imageInFlightFence, VK_TRUE, timeout) (FrameContext.cpp:288-290). presentAckSerial can therefore only advance once that wait completes. A client allowed 2 outstanding presents ahead of a server that is itself 2-3 GPU frames ahead is 4-5 frames of end-to-end latency — 66-83 ms at 60 Hz — for a first-person game. None of the acceptance gates detects this: SSIM goldens are frame-content comparisons and bench.sh measures FPS, not input-to-photon. The risk register's R11 worries about Magma's present mode but not about the composition.
    • 修法:Default MOBILEGL_IPC_PRESENT_CREDIT to 1, not 2, and document the composition explicitly (client credit + server FIF + driver depth). Add an input-latency measurement to the P3 and P9 gates — the codebase already has GetGpuTimestampNs and the trace-replay --benchmark per-frame JSON to build a timestamp-to-present histogram — and only raise the credit if a measured throughput win pays for a measured latency cost.
  • [major] Total CPU work per draw increases and there is no core-placement plan on a big.LITTLE phone
    • 问题:§5.1 says outright that the reconciler 'is the PrepareForDraw reachability walk with sync replaced by emit — not a metaphor: the same set, the same order, the same gating'. That means the walk runs TWICE per draw: once in WireMirror on the client, once in the unchanged PrepareForDraw on the server (DirectGLES.cpp:2916-2975), plus encode and decode. Some of that walk is not cheap: CurrentUnitBindingsEpoch (DirectGLES.cpp:1421-1438) falls through to a full owner-equality walk over every touched texture unit whenever GetTextureBindGeneration() moved, and the code's own note says that happens on redundant re-binds ('26.2 re-binds the unit's own sampler around every texture-unit switch'). The split's entire performance case therefore rests on those two halves landing on two different cores that are both fast. But grep -rn 'sched_setaffinity\|cpu_set_t\|affinity' MobileGL/ --include=*.cpp --include=*.h returns zero hits — the library never sets affinity. The server is a separate process launched by fork/exec (§11), so it does not inherit whatever affinity the launcher applied, and the project's own memory (pojav-bigcore-affinity-trap) records that pojavBigCore=true pinned the entire game JVM and MobileGL workers to one core, invalidating a body of historical measurements. If mgl-srv-apply lands on a 1.55 GHz little core it performs strictly more work than monolith did on a 1.96 GHz big core, and the split is a regression by construction. §15's P3 gate ('split frame time within 10% of monolith') would fail for a reason nobody would attribute correctly.
    • 修法:State the total-CPU-work delta in the plan (client reconcile + encode + decode + server PrepareForDraw vs monolith PrepareForDraw) rather than only the per-side cost. Add explicit affinity: reuse ShaderCompilePool's existing big-core detection (ShaderCompilePool.cpp:73-96 ReadCpuMaxFrequencyKHz / DetectBigCoreCount) to pin mgl-srv-apply to a big core, behind MOBILEGL_IPC_SERVER_AFFINITY, and log the resolved mask. Make P2.5 report per-thread CPU time on both threads, not just wall-clock frame time, so a 'no win' result can be attributed to placement vs to encode cost.
  • [major] A shipping build cannot have both runtime split selection and a zero-overhead monolith; the nm/.text proof only covers the OFF build
    • 问题:§12's three-layer guarantee and decision D8 prove monolith preservation with nm --defined-only plus a stripped .text size diff — but only for MOBILEGL_BUILD_DISAGGREGATED=OFF. Every deployment story in the plan requires ON in the shipped libMobileGL.so: MOBILEGL_TRANSPORT selected via FCL's user-editable env preferences, the plugin APK V2 toggle table, ctest ENVIRONMENT variants, the /data/local/tmp CTS path. And §12 states that in ON builds, inproc mode makes pGLContext a thread-local behind an operator-> shim. That shim sits on the hottest path in the library: grep -rho 'pGLContext->' MobileGL/MG_Impl | wc -l = 1494, plus 124 in DirectGLES and 169 in DirectVulkan. On Android a dlopen'd shared library cannot reliably use initial-exec TLS, so each access becomes a __tls_get_addr call — a function call where there is currently a single load of a global reference (Core.h:564 extern UniquePtr<GLState::GLContext>& pGLContext). The plan's own estimate of the non-arrow sites is also a guess ('~65 places'); the measured count in MG_Impl alone is 4 (GL_Debug.cpp:99 .get(), GL_Program.cpp:1630 == nullptr, plus 2 in header/comment context), with ~20 more in MG_State/MG_Backend (Managers.cpp:3608,3737,3808,4663,7120,7128,7131,8678; DirectGLES.cpp:146; TextureObject.cpp:92; BackendObject_DirectVulkan.cpp:388,788; and ~11 MOBILEGL_ASSERT sites in DirectVulkan.cpp:347-461), so the shim must also supply get(), operator bool and equality — but the count being wrong is minor next to the TLS cost.
    • 修法:Split the option in two: MOBILEGL_BUILD_DISAGGREGATED (spawn/socket only, keeps pGLContext a plain global — one predictable branch in MG_Backend/Init.cpp and nothing on the GL path) and MOBILEGL_BUILD_DISAGGREGATED_INPROC (CI/debug only, adds the TLS shim). Ship the former. Extend the P0 nm/.text gate to run on BOTH the OFF build and the shipping ON build in monolith mode, and make the ON-build check a .text-symbol-level diff of MG_Impl translation units so any accidental indirection on the GL path shows up as a size delta.
  • [minor] Memory doubling is unbudgeted: client segments plus a full replica context plus the server's own three rings
    • 问题:Risk R4 only tracks server-side glslang RSS during P1-P4. The steady-state data-plane and replica footprint is never budgeted. Client side (§6.1): SEG_CMD 8 MiB + SEG_STAGE 32 MiB growing to 256 MiB + SEG_SHADOW allocations at P4.5. Server side: the replica GLContext holds its own PipeResource shadow for every buffer and its own MipmapStorage for every texture level (the client's shadow is separate unless SEG_SHADOW adoption lands), plus the unchanged backend rings — kUboRingInitialBytes/kUboRingMaxBytes 4→64 MiB, kUnpackRing 4→64 MiB, kUploadRing 4→64 MiB (Managers.cpp:82-96) — plus kMaxPoolBytes = 64 MiB of buffer pool (Managers.cpp:566). That is up to ~450 MiB of new committed memory beyond monolith, on a device where the project already values 'saving ~400MB' as a headline result of the adoption fix and where its own memory notes record blanket-immutable buffers causing LMK kills.
    • 修法:Add an explicit steady-state memory budget to the plan alongside the round-trip budget, and make P1's acceptance record RSS for BOTH processes (it currently only records the server's). Size SEG_STAGE's ceiling from measurement, not 256 MiB by default. Prioritise the P4.5 replica-adopts-client-shadow change (see the copy-accounting fix) since it removes the duplicate shadow, not just a copy.
  • [minor] 'Zero round trip in steady state' is fixture-dependent: BeginConditionalRender always blocks and is not exercised by the chosen gate
    • 问题:§8 lists glBeginConditionalRender among the unavoidable blocking points, citing GL_Query.cpp:705-706, and the source confirms it is unconditional: 'Resolved ONCE, here, and by WAITING even for the _NO_WAIT modes: the spec lets those render instead of stalling, so always waiting is conforming and is the only choice that gives the whole block one deterministic verdict.' But P3's acceptance criterion — 'the round-trip counter reads 0 in steady-state frames of minecraft-1.21.4-main-menu' — picks a fixture that exercises neither conditional render nor occlusion queries, so a green gate proves nothing about a renderer that uses them per frame. The same applies to glGetQueryObject(GL_QUERY_RESULT) on an unfinished query.
    • 修法:Either make the P3 gate assert '0 round trips' across the whole trace-case matrix rather than one menu fixture, or restate the claim as 'zero round trips for the draw/state/upload path' and publish the per-fixture round-trip counts as a table. Consider making conditional render's occlusion resolve a client-side speculative pass-through with a server-side correction, since the spec permits the _NO_WAIT modes to render rather than stall.
  • [minor] retiredTail starves in present-less loops and the stated mitigation does not exist on DirectGLES
    • 问题:Risk R12 says the server 'also advances that watermark from its own TryDrainFrameTransients / RefreshCompletedSubmits, and publishes it on a timer'. That is true for DirectVulkan but has no DirectGLES counterpart: g_completedFrameSerial is advanced in exactly two places — inside Present() by polling the frame-fence ring after eglSwapBuffers (DirectGLES.cpp:10626-10643), and inside WaitForFrameSerialCompleted (DirectGLES.cpp:10583-10607) which itself requires a live ring fence at or past the target and returns false when the slot was recycled. In a present-less workload — glcts (tools/cts run_cts_local.py), readback loops, MG_IntegrationTest scenarios that never swap — no fence is ever inserted, so retiredTail never advances, SEG_STAGE fills, and §6.5's escalation runs to the hard drain on every case. That converts a CTS run into a sequence of 50 ms spins plus full drains, and could be misread as a conformance regression.
    • 修法:Give the DirectGLES server an explicit non-present fence tick: insert a glFenceSync and poll the ring on a timer or every N applied records when no Present has occurred for a threshold, reusing the g_frameFenceRing machinery. Log ring-occupancy and escalation counts (the P0 Tracy counters) so a starved watermark is visible as a metric rather than as an unexplained stall, and add a present-less split-mode case to the P2 gate.

已验证的优点:

  • The replica-GLContext decision is correct and the cited evidence holds. IsBufferDrawClean opens with a raw-pointer identity compare before any version check (Managers.cpp:1435-1436, 'Identity first: a respecify path can hand the frontend a NEW resource'), and CurrentUnitBindingsEpoch (DirectGLES.cpp:1421-1438) resolves its epoch by an owner-equality walk over the live binding slots precisely because the bind generation moves on redundant re-binds. Neither has a wire-field analogue. Rewriting the backends to consume deltas would require re-deriving this invalidation model, which is what sank Feat/CS-Delta-IPC.
  • D3 — not shipping version counters and letting the replica bump them through mutator replay — is sound and avoids the failure mode of the prior branch. The counters that gate re-sync really are wrapping Uint16 paired with pointer identity, and replaying mutations makes both sides run the same wrap logic instead of maintaining monotonicity on the wire. It also avoids adding Install* setters, which is how Feat/CS-Delta-IPC's b50f3348 leaked RenderState's private members to public.
  • The claim that unpack pixel-store never crosses the boundary is verified: all six backend reads pass false (PACK) — DirectGLES.cpp:6129, 7614, 9101, 9480; Utils.cpp:2301; VulkanRenderer.cpp:10622. Confining PixelStoreBlob to the PACK direction is correct and removes a delta kind.
  • Using FlatBuffers structs as fixed-layout records inside an SPSC ring, with tables reserved for the rare/variable control-plane messages, is the right call: structs have no vtable, no offset indirection and need only a bounds check rather than a verifier walk. The per-kind static_assert in Records.def is a genuine fix for the exact bug Feat/CS-Delta-IPC hit (its single assert on the first union member could not catch mid-list insertion).
  • The observation that glReadPixels into a pack PBO can become fire-and-forget and thereby beat the monolith is correct: today DirectGLES maps the whole PBO back and writes it into the frontend shadow inside the call (DirectGLES.cpp:9189-9205), so there is no asynchronous PBO readback path at all. Same for deferring glEndTransformFeedback's unconditional infinite ClientWaitSync (GL_Drawing.cpp:1326-1337). Both are real wins and both are correctly identified as standalone monolith improvements worth landing on dev first.
  • Keeping Present strictly 1:1 with the application's eglSwapBuffers is correct and well-justified: DirectGLES.cpp:10646-10649 retires the UBO/unpack/upload rings and trims the buffer pool only there, and the Magma side does all four OnFrameBoundary agings plus the BeginFrame calls inside Present. Batching frames would starve those drains.
  • Porting the ring reclamation discipline from the existing PersistentRing is well-grounded: the RingFrameMark {frameSerial, headAtPresent} structure and the monotonic head/tail with 'in-flight bytes = head - tail must stay <= size' invariant are exactly as described (Managers.cpp:659-706), as is the grow → bounded-wait → hard-drain-plus-generation-bump escalation.
  • P0's demand to remove the uncommitted per-draw instrumentation is necessary and verified: Managers.cpp:875-877 contains a live std::fprintf(stderr, "[BUFTX] FlushPendingRangesNow res=%p serial=%llu' + NL + '", ...) inside the pendingMutex critical section on the buffer flush path, with a literal ' + NL + ' in the format string. Measuring anything before removing it would repeat the prior branch's mistake.
  • The refutation of Feat/CS-Delta-IPC's BFA C-ABI, UtilRuntime C-ABI-isation and share-group-sessioning-first ordering is well-founded, and the replacement ordering (thinnest end-to-end path first, device render at day 13, P2.5 as an early falsification gate at week 5) is the right risk sequencing. Making SCM_RIGHTS a P0 deliverable rather than a deferred 'P6' item directly fixes the defect that left the prior branch's data plane inoperable on Linux and Android.
  • The dead-code cleanups are real and verifiable wins for the monolith independent of the split: GetInteger64i_v and GetProgramiv have no MG_Impl callers, and routing glDispatchCompute's three per-dispatch GetIntegeri_v validation queries to the already-captured CompileEnv limits removes a genuine per-dispatch cost.

可行性/平台/交付(refuted=False12 条)

  • [fatal] Shadow-backed persistent (COHERENT) maps are never published to the server — app writes are silently lost
    • 问题:BufferObject::SyncPersistentMappedRange() (MobileGL/MG_State/GLState/BufferState/BufferObject.cpp:238-250) is the ONLY publisher of writes an application makes through a persistent, non-FLUSH_EXPLICIT, non-adopted map: it emits NotifySubData(m_mappedRange). Every one of its call sites lives inside MG_Backend/ (verified by grep: DirectGLES.cpp:262,4412,4666,4667,4768,4769; Managers.cpp:1547; MultiDraw.cpp:498; DirectVulkan.cpp:290,481,895; UniformManager.cpp:2022; VkBufferManager.cpp:573,620; VulkanRenderer.cpp:3432,3511,3826,7070,12015,12016). There is ZERO caller in MG_Impl or MG_State. Plan §6.8 makes tier T2 (AcquirePersistentMap returns nullptr) the default for phases 1-6. AcquireMemoryRange (BufferObject.cpp:459-475) then falls back to the shadow and hands the app m_resource.Bytes() + range.start. The app writes into the CLIENT's shadow and makes no further GL call — that is the entire point of a coherent persistent map. In split mode the backend runs against the replica, so it calls SyncPersistentMappedRange() on the REPLICA's BufferObject, which is not mapped by anything. The client's emit-ops table is never invoked, no delta is produced, and the server draws from whatever the shadow held at map time. The plan's §6.8 rationale explicitly reasons only about FLUSH_EXPLICIT ('FLUSH_EXPLICIT 恰是跨进程的好情况') and concludes the coherent case is covered by declining adoption. It is not: declining adoption is precisely what routes into the unpublished path. MOBILEGL_COHERENT_AS_FLUSH defaults to false (Config.h:174), so an app that itself passes GL_MAP_COHERENT_BIT — the modern streaming idiom, and the reason Config.h:168-174 exists at all — lands here unconditionally. No listed gate before P7 covers this. OpenRA (the P1 gate) does not use persistent maps.
    • 修法:Make the client the publisher. WireMirror must call SyncPersistentMappedRange() on every currently-mapped buffer reachable from the operation at each emit point, mirroring the backend's 13 call sites (VAO attribute buffers, index buffer, indirect/parameter buffers, UBO/SSBO/atomic binding points, XFB capture targets) BEFORE it samples GetChangeSerial(). Keep a client-side ska::flat_hash_set of live persistent-mapped buffers so the walk is O(mapped) not O(all). Add a P1 acceptance scenario (PersistentCoherentMapScenario) that maps PERSISTENT|WRITE|COHERENT, writes with no further GL call, draws, and reads back — and require it green before P1 is declared done, not at P7.
  • [fatal] The applier replays GLFunctionsTable, not MG_Impl — MG_State mutations MG_Impl performs around table calls never reach the replica
    • 问题:Plan §3, §5.2 and risk row R1 all rest on 'applier = mutator replay, so the replica's versions bump exactly when the client's did' and on R1's claim that divergence would require 'the client's ENTRY POINT doing something the applier did not replay, and that is a bounded, auditable surface (the 91 MG_Impl call sites into GLFunctionsTable)'. That surface is exactly where the bugs are, it is not bounded by anything the plan gates, and I found two concrete, shipped instances: (a) glGenerateMipmap. GLImpl::GenerateMipmap (MG_Impl/GLImpl/Texture/GL_Texture.cpp:6681-6691) runs EnsureGeneratedMipmapStorageAllocated(*mipmapTexture) BEFORE GenerateMipmap_Backend. That helper (GL_Texture.cpp:501-545) calls AllocateStorage for levels 1..N, MarkStorageDirty(...,false) (:528), TruncateMipmapLevels (:533) and BumpContentVersion() (:537). The comment at :534-537 states why the version bump exists: without it 'a cached sampled VkImageView built for the pre-generate level range would otherwise stay stale and clamp LOD>0 sampling to mip 0'. An applier that only calls the table reproduces that exact known bug on the replica. Same for GenerateTextureMipmap (:6705-6711) and MaybeAutoGenerateMipmap (:1625-1635). (b) Transform feedback CPU accounting. AccountTransformFeedbackPrimitives (MG_Impl/GLImpl/Drawing/GL_Drawing.cpp:172-236) mutates six GLContext counters on every captured draw: AddTransformFeedbackPausedPrimitives (:177), AddTransformFeedbackInputPrimitives (:184), AddTransformFeedbackGeometryCaptureDraw (:214), AddTransformFeedbackPrimitives (:231), AddTransformFeedbackCapturedVertices (:232), AddTransformFeedbackAccountedCaptureDraw (:237). DirectGLES reads GetTransformFeedbackCapturedVertices() at DirectGLES.cpp:900 to size the scattered capture, and DirectVulkan reads GetTransformFeedbackPausedPrimitiveCounter() at DirectVulkan.cpp:1384 and folds the frontend delta into the query result at :1337. On the replica every one of these stays 0: scattered XFB captures nothing and PRIMITIVES_WRITTEN/PRIMITIVES_GENERATED are wrong. None of these counters has a version counter; none appears in the plan's §5.3 trigger table or its §5.1 reconcile walk. They are additionally saved/restored per XFB object on bind (Core.cpp:1273,1296; Core.h:313-357), so a naive 'ship the scalar' patch must follow the object swap. The §5.9 coverage generator cannot catch this class: it scans MG_Backend/** for READS and maps them to delta kinds, so a backend read of GetTransformFeedbackCapturedVertices would be classified and pass, while the PRODUCER half in MG_Impl is never audited.
    • 修法:Add a second generated inventory to §5.9: every pGLContext-> MUTATOR call in MG_Impl that occurs in a function which also calls gBackendFunctionsTable.GL.* or pActiveBackendObject->. Each entry must be marked replayed-by-applier, shipped-as-delta, or explicitly client-only, with a #error on unmapped — the same compile-time gate the read side gets. Concretely: (1) factor AccountTransformFeedbackPrimitives and EnsureGeneratedMipmapStorageAllocated into shared helpers the applier also runs, or ship them as explicit RecXfbAccounting / RecGenerateMipmapLevels deltas; (2) move the P8 Xfb* scenario gate earlier, into P2, so this class of divergence surfaces before four more phases are built on the assumption.
  • [major] Read-after-GPU-write is gated by a flag the client can never set in time — glMapBufferRange(READ) returns stale bytes with no round trip
    • 问题:BufferObject::SyncGpuWrites() (BufferObject.cpp:265-274) early-returns unless m_gpuWritePending, and that flag is set only by MarkGpuWritten() (BufferObject.cpp:260-263), whose only callers are in MG_Backend/ (DirectGLES.cpp:465, 509, 1809; UniformManager.cpp:1073, 1229; VulkanRenderer.cpp:11210) plus the resident-SubData branch in UploadSubData, which cannot fire client-side while adoption is off (§6.8 T2). Every reconciliation point calls it: AcquireMemory (:405), AcquireMemoryRange (:454), UploadSubData, FillSubData (:351), CopyDataFrom (:383), and MG_Impl's glGetBufferSubData/glGetNamedBufferSubData (GL_Buffer.cpp:957, 995). In the split the client's flag is set only if an EvGpuWritten event happens to have been drained already. Plan §6.7 lists 'glGetBufferSubData / glMapBuffer(READ) on gpuWritePending' as a round trip and says it is 'narrowed by EvGpuWritten{ranges}' — but nothing establishes the flag in the first place. An app that dispatches a compute shader writing an SSBO and immediately maps it for read gets the stale shadow, silently, with zero round trip. Same for atomic counters, XFB capture targets, and pack PBOs after the fire-and-forget ReadPixels of §6.7.
    • 修法:The client must own a conservative pending set, mirroring what DirectGLES already does at DirectGLES.cpp:459-467/687/697: at every emitted draw/dispatch, mark every buffer bound to SHADER_STORAGE / ATOMIC_COUNTER / an image-buffer texture unit, every active XFB capture target, and any pack PBO named by a ReadPixels record, recording the emit seq. On any read entry point, if the buffer is in that set: publish, wait for appliedSeq >= recordedSeq, drain events, then read. EvGpuWritten becomes a pure narrowing optimisation (it may cancel or range-limit the wait), never the thing that establishes existence.
  • [major] Sync and query poll loops deadlock: the polling entry points are not Publish triggers
    • 问题:Plan §8 answers GetSyncStatus, ClientWaitSync(timeout==0), IsQueryResultAvailable and GetQueryResult64(wait=false) from a single RingControl acquire load with zero round trips. Plan §7.2's Publish trigger list is: 64 KiB of records, SEG_STAGE below 1/4, any blocking request, Present, eglMakeCurrent, glFlush. None of the poll paths appears. The canonical GL idioms are glFenceSync(); while (glClientWaitSync(s, GL_SYNC_FLUSH_COMMANDS_BIT, 0) == GL_TIMEOUT_EXPIRED) {} and while (!avail) glGetQueryObjectuiv(id, GL_QUERY_RESULT_AVAILABLE, &avail);. With no other GL call in the loop, the FenceSync / EndQuery record sits in the shm ring with no release-store of head and no doorbell; the server never observes it; the watermark never advances; the loop spins forever. This is a hang, not a slowdown. It is also a spec violation the codebase already cares about: GL_Sync.cpp:71-82 validates GL_SYNC_FLUSH_COMMANDS_BIT specifically so a caller cannot 'think it had asked for a flush it never got'. glGetSynciv(GL_SYNC_STATUS) (GL_Sync.cpp:172-181) is the same shape.
    • 修法:Add glClientWaitSync (any timeout), glGetSynciv(GL_SYNC_STATUS), glGetQueryObject*(GL_QUERY_RESULT_AVAILABLE | GL_QUERY_RESULT_NO_WAIT) to the Publish trigger list — publish (release-store + doorbell) without waiting. Make GL_SYNC_FLUSH_COMMANDS_BIT publish unconditionally, since the spec mandates the flush. Add a starvation escape: after N consecutive polls with no watermark movement, promote to one blocking round trip so a server that has stalled cannot spin the client.
  • [major] §5.6's 'the client never clears its texture dirty flags' is provably wrong and makes every texture update ship the whole level
    • 问题:MipmapStorage::MarkDirtyRegion (MG_State/GLState/TextureState/MipmapStorage.cpp:198-235) UNIONS the incoming box into m_dirtyRegions[level] and appends to m_dirtyRects[level] for as long as m_isDirty[level] is true; only MarkDirty(level,false) (MipmapStorage.cpp:171-190) resets them. Plan §5.6 asserts the client never clears ('client 的 dirty flag 从不被清 ... 已发送状态存在 WireMirror 里') while §5.5 rule 3 derives the shipping shape from GetStorageDirtyRegion/GetStorageDirtyRects. ShipRecord (§5.1) holds three Uint64 version words — no region can be reconstructed from it. Consequences: after the first sub-image the union box only grows, the rect list saturates at kMaxDirtyRects, and GetDirtyRects returns 0 the moment summedArea*4 >= unionArea*3 (MipmapStorage.cpp:305). Every animated-atlas tick then ships the entire level — the exact opposite of the §5.5 tuning the plan claims to preserve. MarkDirtyRegion's rect-seeding branch (if (!m_isDirty[level]) rects.clear(); else if (rects.empty() ...) rects.push_back(region), :214-221) is written for a consumer that clears; never clearing changes its behaviour too. Secondary factual error in the same paragraph: the frontend does clear dirty flags itself, at five sites — GL_Texture.cpp:528, 701, 5547, 5621, 5691. The good news I verified: MG_Impl contains no IsStorageDirty(/GetDirtyRects(/GetDirtyRegion( call site at all, so clear-on-emit is safe for the frontend.
    • 修法:Have the client clear on emit — MarkStorageDirty(uploadTarget, level, false) immediately after appending the texture record. That reinstates the ack question §5.6 claims to have dissolved; close it by (a) making ResyncSnapshot always ship whole levels from the intact shadow (it can — the shadow is never dropped), and (b) deferring the clear until the record is past a drain-safe watermark, or accepting resync-on-drain. Rewrite §5.6's dirty-flag paragraph accordingly; it is currently the load-bearing justification for a design decision that does not hold.
  • [major] inproc mode cannot work as specified: the backend function table, the active backend object and the default-FBO info are single process globals
    • 问题:§12 hooks the split by replacing MG_Backend::gBackendFunctionsTable and MG_Backend::pActiveBackendObject — both assigned once, process-wide, at MG_Backend/Init.cpp:43-44 and :53-61. In inproc both roles live in one process, so once the client installs the emit table there is no path by which the applier reaches the real DirectGLES/DirectVulkan table, and no path by which server-side MG_Impl code reaches it either. Server-side MG_Impl code exists and reads that global: GenerateMipmap_Backend (GL_Texture.cpp:1621), the GetTexImage fallback chain (GL_Texture.cpp:6713-6725), FixupGsStripCaptureOrder. Worse, MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo is a second process global (defined GL_Framebuffer.cpp:3344) read by the client at GL_Framebuffer.cpp:495, 1827, 1837, 1897, 1905, 1913, 1927, 1936, 2549, 2590, 2598, 2608, 2611 and by the server-side backend at DirectGLES.cpp:1917, 2838, 2867, 9675 and SwapchainObject.cpp:276. One process cannot hold both a client default-FBO description and a server one; SwapchainObject writes the server's view straight into it. §12 addresses only pGLContext (correctly noting the 65 non-arrow uses — I counted exactly 65). This is not a corner: §12 calls inproc a product deliverable and P2.5 makes it the plan's EARLIEST falsification gate, so a broken inproc removes the week-5 go/no-go entirely.
    • 修法:Extend the role-scoping mechanism chosen for pGLContext to gBackendFunctionsTable, pActiveBackendObject and pDefaultFramebufferInfo — thread-local pointer plus an operator->/get()/operator bool shim, all under #if MOBILEGL_BUILD_DISAGGREGATED. Re-scope the D2/§12 claim of 'one hook point in MG_Backend/Init.cpp:47-70': it is four globals, and add the cost to P0's estimate. Alternatively drop inproc to a test-only mode with the applier holding an explicitly-passed table pointer and no MG_Impl on the server side — but then P2.5 no longer measures the monolith render-thread deliverable it is supposed to.
  • [major] Late glGetError breaks the standard allocation-probe idiom for the backend's GL_OUT_OF_MEMORY sites
    • 问题:§5.6 routes backend RecordError (DirectGLES.cpp:6319; Managers.cpp:8679; DirectVulkan.cpp:816; VulkanRenderer.cpp:1242, 1246, 1302) through an EvGlError event that is explicitly allowed to be observed 'a batch late', with MOBILEGL_IPC_STRICT_ERRORS reserved for the CTS lane. Those sites are GL_OUT_OF_MEMORY on renderbuffer and texture allocation. The universal application idiom is glRenderbufferStorage(...); if (glGetError() == GL_OUT_OF_MEMORY) { fall back to a smaller target; }. Late delivery makes the app take the success branch and then render into storage the server never allocated — a divergence that shows up as corrupt output or a later server-side failure, far from the cause. The plan is right that glGetError must stay client-local for the hot path (GL_Getter.cpp:2811-2817; the GL-thread-owned invariant at Core.cpp:48-49). The error is treating all backend errors as one class.
    • 修法:Split the class. Mark only the allocation-class entry points as kNeedsAckglRenderbufferStorage*, glTexImage*/glTexStorage*/glCopyTexImage* where the backend can fail, glBufferStorage. They are rare and already expensive, so the ack is nearly free, and it makes the OOM probe exact. Everything else keeps late delivery. Drop the global MOBILEGL_IPC_STRICT_ERRORS from the CTS-only ghetto; with this split it should not be needed.
  • [minor] P4's 'move glCopyTexSubImage wholly to the server' contradicts the existing implementation and references an event the protocol does not define
    • 问题:glCopyTexSubImage* is already an entirely frontend operation: CopyTexSubImage{1,2,3}D_State (GL_Texture.cpp:3955, 3980) call CopyReadFramebufferIntoMipmapRegion, which borrows a backend ReadPixels into CPU scratch, memcpys into the mipmap shadow, and calls MarkStorageDirty(uploadTarget, level, true) at GL_Texture.cpp:1095. Left alone in the split it costs exactly one blocking ReadPixels round trip and the resulting dirty region ships as an ordinary texture delta — correct, and it needs no new command. P4 instead proposes moving it server-side plus an EvTexWriteback event to update the client shadow. That event does not appear in §7.4's event list (which has EvBufferWriteback but no texture equivalent), it still costs a round trip (the client shadow must be current for glGetTexImage), and it adds a command with no counterpart in GLFunctionsTable. glClearTexImage (GL_Texture.cpp:985-1006) has the same frontend-only shape.
    • 修法:Leave glCopyTexSubImage* and glClearTexImage frontend-side; delete the P4 item and the undefined EvTexWriteback. Keep the per-level serverAuthoritative bit only for the two cases whose shadow writes genuinely happen in the backend: generated mip levels (DirectGLES.cpp:6270-6271, 6861) and the CopyImageSubData destination mirror (DirectGLES.cpp:7144).
  • [minor] Client-side vertex-array bounding can scan a stale index buffer
    • 问题:§6.10 correctly identifies that the index scan (TryComputeMaxIndexFromHostBytes, VulkanRenderer.cpp:3406-3470) must run client-side. But the monolith runs indexBuffer->SyncGpuWrites() immediately before every such scan — DirectGLES.cpp:4413, MultiDraw.cpp:499, VulkanRenderer.cpp:3431, 4159 — precisely because the EBO may have been written by a compute shader or XFB. On the client that scan reads the client shadow, and per the pending-flag flaw the reconciliation will not fire, so the computed maxIndex is derived from stale bytes and the vertex array is under-copied: missing or garbage geometry, or an out-of-range read of the app's array. The same stale-shadow exposure applies to the primitive-restart rewrite (DirectGLES.cpp:4412-4414) and to the *IndirectCount parameter-buffer read (DirectGLES.cpp:4666-4693, 4768-4793).
    • 修法:Fold into the conservative pending-set fix: ClientArrayBounds and the restart/indirect-count readers must force the readback (publish + wait + drain) before touching the shadow, exactly where the monolith calls SyncGpuWrites(). Add a ClientArrayAfterComputeWriteScenario to the P2 gate.
  • [minor] SEG_STAGE has no cursors in RingControl, and P4.5 shadow arena blocks have no retirement rule
    • 问题:Two data-plane bookkeeping gaps. (1) §6.2's RingControl defines one head/appliedTail/retiredTail triple, but §6.1 gives SEG_STAGE its own 32-256 MiB ring and §7.2 makes 'SEG_STAGE 余量 < 1/4' a Publish trigger. Occupancy of a second ring is not computable from the first ring's cursors, and stage slots borrowed by PendingResidentWrite (§P6) retire on retiredSeq, not appliedSeq, so they need their own pair. (2) §6.4's 64 KiB block send-watermark covers overwriting a LIVE shadow, but says nothing about freeing one: glDeleteBuffers or a glBufferData respecify releases or reallocates the SEG_SHADOW arena block while records carrying {segId, offset, size} into it may still be unapplied — the server then reads another object's bytes.
    • 修法:Give SEG_STAGE its own {head, appliedTail, retiredTail} triple in RingControl (there is room in the 4 KiB page). Retire shadow-arena blocks through the same watermark as ring slots — a freed block goes on a pending list and is only returned to the arena once appliedSeq (or retiredSeq for borrowed slots) has passed the last record that referenced it — rather than being released at object destruction.
  • [minor] A lifetimeId mismatch on create is repaired by a destructive re-create, which GL forbids for a still-referenced object
    • 问题:§5.4 says the server keys replica objects by (kind, name) and that 'if a create's lifetimeId does not match the record, destroy first then create'. On the replica that object may still be legally referenced by FBO attachments, binding slots, texture views (GetViewStorageOwner) or XFB capture targets, all of which hold SharedPtrs; GL keeps such an object alive until the last reference drops. A forced destroy either leaves dangling replica references or silently detaches them, and it converts a protocol bug into a rendering bug that will be attributed to the backend. The surrounding design is sound — §5.4's 'identity plus counter, never the counter alone' is the right lesson from the packed_pixels postmortem (DirectGLES.cpp:2823-2831) — it is only the repair action that is wrong.
    • 修法:Make the mismatch Fatal{IdentityDivergence} (or a forced ResyncSnapshot under MOBILEGL_IPC_RESPAWN). It cannot occur if the protocol is correct, so a loud stop is strictly better than a silent destructive repair; the debug cost of an unexplained missing attachment far exceeds the cost of a crash with a named reason.
  • [minor] RenderbufferObject::GetLifetimeId is listed as a 3-line P0 addition but has no version counter either
    • 问题:§5.4 and §14 correctly flag that RenderbufferObject lacks GetLifetimeId() while Buffer (BufferObject.h:208), Framebuffer (:158), Program (ProgramObject.h:1620), VAO (VertexArrayObject.h:120), Sampler (SamplerObject.h:141) and Texture (TextureObject.h:83,161) have one. But the §5.3 trigger table also has no row for renderbuffer state at all: BackendRenderbufferObject::SyncToBackend (Managers.cpp:~8620-8700) caches {internalFormat, width, height, samples} and there is no accessor in the plan's walk that would tell the client a glRenderbufferStorageMultisample happened. It is reachable only transitively through FboAttach, which is gated on GetAllFramebufferAttachmentVersions() — a re-storage of an already-attached renderbuffer need not bump that.
    • 修法:Add both GetLifetimeId() and a GetVersion() to RenderbufferObject in P0 (same shape as SamplerObject::GetVersion), add a RecRenderbufferStorage row to §5.3, and add renderbuffer re-storage to the §5.1 reconcile walk step 6 (per-attachment). Regenerate BackendStateSurface.inc after adding the accessor so the §5.9 gate covers it.

已验证的优点:

  • The replica-GLContext decision is correct and the evidence for it is stronger than the plan states. I confirmed the backend memos written into frontend objects are only 4 sites and DirectVulkan-only (ProgramFactory.cpp:3448; VertexInputStateFactory.cpp:60, 78, 83), and that VertexInputStateFactory.cpp:78 really does store a raw pointer into the backend's own heap. Under the replica model these are free; under any delta-apply rewrite they are a redesign. DirectGLES has zero such sites.
  • RenderStateBlob as a single whole-struct delta is well chosen. I verified RenderStateParameters (RenderState.h:222-370) genuinely carries PatchVertices (:242), PatchDefaultOuter/InnerLevel (:248-249), ClampReadColor (:313), ProvokingVertexModeSetting (:300), PrimitiveRestartIndex (:322), PolygonMode front/back, the 16-viewport arrays and ScissorBoxWrittenMask. So one blob really does subsume the ~40 individual fixed-function accessors plus the patch-parameter reads at DirectGLES.cpp:2807-2814 and Managers.cpp:7120-7132.
  • Deleting GetInteger64i_v and GetProgramiv from the wire is right. I confirmed no MG_Impl call site reaches those table entries: glGetInteger64i_v answers locally and delegates leftovers to the 32-bit form (GL_Getter.cpp:1240, :1302-1307) and glGetProgramiv routes to GetProgramiv_State (GL_Program.cpp:2478-2479), which answers from ProgramObject.
  • tableSlotMask is a necessary addition the prior branch lacked. BeginOcclusionQuery != nullptr really is used as a capability probe at GL_Query.cpp:471, 545 and 768 (COUNTER_BITS answers 32/1/0 off it), so a remote client must reproduce which slots the far side actually registered.
  • The plan's read of GetQueryResult64's contract is accurate and load-bearing: GL_Query.cpp:292-311 reads 0, does NOT cache, and keeps the backend handle when the backend cannot produce a result yet. That is genuinely deferred-reply-friendly and makes watermark-predicted answers conformant.
  • §5.7's identification of the composite-pipeline hazard is correct and non-obvious. GLContext::GetProgramForDraw (Core.cpp:612-660) joins every stage, computes a signature, and on a cache miss does MakeShared<ProgramObject>(0u) and links an anonymous composite; RefreshCompositeUniforms/MirrorUniformValues then mutate it per draw. A publish-mode server with no sources genuinely cannot do this, so client-side resolution plus SetReplicaResolvedDrawProgram is the right fix.
  • Declining AcquirePersistentMap really is tolerated by the frontend at all three request sites — TryAdoptLargeStorage (BufferObject.cpp:173-176), EnsureGpuResidentStorage (:436-443) and AcquireMemoryRange (:470-473) all handle a null return — so §6.8's tier T2 is a safe default from the frontend's point of view (the failure is elsewhere, see the persistent-map finding).
  • MOBILEGL_COHERENT_AS_FLUSH defaults to false (Config.h:174, ConfigLoader.cpp:185), so §6.8's prohibition costs nothing on the default configuration and cannot regress the Create/Flywheel fixtures by itself.
  • The frontend never reads its own texture dirty state — zero IsStorageDirty/GetDirtyRects/GetDirtyRegion call sites in MG_Impl — which is what makes the clear-on-emit fix to §5.6 safe. The plan reached the wrong conclusion from the right underlying fact.
  • The §12 note about pGLContext is precise: it is extern UniquePtr<GLState::GLContext>& (Core.h:564) and I counted exactly 65 non-arrow uses across MG_Impl/MG_State/MG_Backend, matching the plan's '约 65 处'. The shim requirements it lists (operator->, get(), operator bool, equality) are the right set.
  • The critique of Feat/CS-Delta-IPC is accurate on the points I spot-checked: it really did leave POSIX fd passing unimplemented, its ServerHost really does not compile, and its bfa.h really does hand FlatBuffers table pointers across a nominal C ABI. Making SCM_RIGHTS a P0 deliverable with its own test is the correct inversion.
  • Keeping glFinish/glFlush free (Definitions.cpp:111-112) and glGetError client-local (GL_Getter.cpp:2811-2817, invariant at Core.cpp:48-49) is right, and the plan is correct that turning them into round trips would be a self-inflicted regression.

3. 修订记录(综合稿 → 定稿)

  • FATAL persistent-map: verified SyncPersistentMappedRange (BufferObject.cpp:238-250) has zero MG_Impl/MG_State callers (all 19 production call sites are in MG_Backend/). Added new section 5.10: RecBufferMap/RecBufferUnmap records + client-side 64KiB-block push from PublishImplicitState + PersistentCoherentMapScenario as a P1a gate. Also fixes the IsBufferDrawClean IsMapped() gate (Managers.cpp:1447) that the replica would otherwise get wrong.
  • FATAL MarkGpuWritten: verified all 6 callers are backend-only. New section 5.6b makes the CLIENT set the flag conservatively at every draw/dispatch emit point (mirroring DirectGLES.cpp:459-467/509/1809), records emitSeq, and forces publish+wait+drain at every read entry; EvGpuWritten demoted to a narrowing hint. Section 7.4 drain points extended with glMapBuffer*/glGetBufferSubData/glGetNamedBufferSubData/glCopyBufferSubData.
  • FATAL poll livelock: section 7.2 rewritten. glClientWaitSync/glGetSynciv(SYNC_STATUS)/glGetQueryObject*(AVAILABLE|NO_WAIT) are now Publish triggers, GL_SYNC_FLUSH_COMMANDS_BIT publishes unconditionally (cites DirectVulkan.cpp:1158-1160), plus a starvation escalation (MOBILEGL_IPC_POLL_ESCALATE). Dedicated P3 gate added.
  • FATAL fence granularity: added a subsection to section 8 requiring real per-fence server-side polling + EvFenceSignaled instead of a present-granular watermark, citing DirectVulkan.cpp:1120-1128 and the magma-mc1215-fence-oom history. Section 9.3 adds a non-present fence tick for DirectGLES.
  • Applier-replays-table gap (new boundary face (g) in section 2): verified EnsureGeneratedMipmapStorageAllocated (GL_Texture.cpp:501-541, incl. BumpContentVersion at :538 with its stale-VkImageView rationale) and AccountTransformFeedbackPrimitives (GL_Drawing.cpp:172-236, 6 counters read by DirectGLES.cpp:900 and DirectVulkan.cpp:1337/1384). Added section 5.9b: a SECOND generated inventory (gen_impl_mutation_surface.py + MutationCoverage.def) making unmapped MG_Impl mutations a #error, plus RecGenerateMipmapLevels/RecXfbAccounting records and MG_Remote/Shared/ helpers.
  • Texture dirty flags: verified MipmapStorage::MarkDirtyRegion unions forever unless MarkDirty(level,false) runs, and that MG_Impl has ZERO IsStorageDirty/GetDirtyRects/GetDirtyRegion readers. Section 5.6a now requires clear-on-emit and closes the ack question via intact-shadow resync + re-send of un-applied texture records after a hard drain.
  • inproc globals: verified pDefaultFramebufferInfo is a second process global (22 refs; client MG_Impl reads 13, server backend reads 4 + SwapchainObject writes 1) and that gBackendFunctionsTable is read by server-side MG_Impl too. Section 12 split into 12.1/12.2/12.3: two CMake options (shipping spawn build keeps all four globals plain, no TLS on the GL hot path), full shim requirement list, and an explicit P0 go/no-go on isolating vs downgrading inproc.
  • Non-arrow pGLContext count corrected from '~65' to the measured 133 (2 in MG_Impl, the bulk in MG_Backend incl. ~90 DirectVulkan asserts), with the lifecycle sites (Core.cpp:20/1487, Core.h:564, Init.cpp:63) added to the shim requirements.
  • Publish policy: deleted the 64KiB byte threshold (it was a full MC frame and pre-killed the P2.5 hypothesis). Now release-store cmdHead every record (or every 8-16), doorbell only when consumerParked.
  • Added the symmetric producer-side doorbell (new section 6.2a: producerParked + reverse byte / condvar) so present-credit, kNeedsAck and ring-full waits block instead of cross-process spinning on a phone big core.
  • Added SEG_EVENT overflow policy (section 7.4): drain inside every wait loop, lossy EvLogLine with eventDropped counter, non-lossy semantic events with an eventRingFull stop-applying flag, plus a P4 fault-injection gate for the credit-blocked deadlock.
  • RingControl gained an independent {head, appliedTail, retiredTail} triple for SEG_STAGE (section 6.2), since the 'stage below 1/4' publish trigger cannot be computed from the cmd cursors and stage slots retire on retiredSeq.
  • Added SEG_SHADOW block retirement rule (section 6.1): freed/reallocated arena blocks go on a pending list gated by appliedSeq/retiredSeq, not released at object destruction.
  • Copy accounting table (6.4) corrected: monolith is 2 (not 1), P1-4 is 4 (not 2), P4.5 is 3 (not 1); added rows for map+unmap and for the new persistent-map push. Added optional plan B (replica adopts client SEG_SHADOW read-only as a third PipeResource mode) as a P6 candidate, and required TracyPlot counters on BOTH sides of the wire.
  • Errors: section 5.6c splits the class - only allocation-class entry points (glRenderbufferStorage*, some glTexImage*/glTexStorage*/glCopyTexImage*, glBufferStorage) become kNeedsAck so the GL_OUT_OF_MEMORY probe idiom stays exact; MOBILEGL_IPC_STRICT_ERRORS demoted from CTS-required to a diagnostic switch. P4 gains an OOM-probe gate.
  • Present credit default lowered from 2 to 1 with the latency composition spelled out (client credit + server FIF + driver depth; FrameContext.cpp:288-290 shows Present itself already waits), and input-latency histogram gates added to P3 and P9.
  • Added a core-placement plan (section 10): total-CPU-work delta must be stated, mgl-srv-apply pinned to a big core reusing ShaderCompilePool.cpp:73-96 detection via MOBILEGL_IPC_SERVER_AFFINITY, and P2.5/P3 must report per-thread CPU time.
  • Added a non-present fence tick for DirectGLES (9.3) so retiredTail does not starve in glcts/readback loops, plus a present-less split case in P2.
  • lifetimeId mismatch on create changed from destructive re-create to Fatal{IdentityDivergence} (section 5.4), because the replica object may still be legally referenced by attachments/views/binding slots.
  • RenderbufferObject now gets BOTH GetLifetimeId() and GetVersion() in P0, with a RecRenderbufferStorage row in 5.3 and per-attachment version reads in the 5.1 walk (a re-storage of an already-attached RBO need not bump the FBO attachment versions).
  • glCopyTexSubImage*/glClearTexImage kept frontend-side (6.6): verified CopyReadFramebufferIntoMipmapRegion (GL_Texture.cpp:1044-1097) is already pure-frontend borrowing one ReadPixels. Dropped the P4 'move to server' item and the undefined EvTexWriteback; serverAuthoritative bit narrowed to generated mips and the CopyImageSubData mirror.
  • Client index scans / restart rewrite / IndirectCount parameter reads must go through the pending-set force-readback at exactly the sites where the monolith calls SyncGpuWrites() (6.10), with a new ClientArrayAfterComputeWriteScenario in P2.
  • Added runtime bounds discipline for ring records (6.3): the same X-macro generates size >= sizeof(T) && size <= remainingRingBytes && (size%8)==0 preconditions, Fatal{ProtocolCorruption} on violation.
  • MOBILEGL_COHERENT_AS_FLUSH ban REMOVED (5.10/6.8): with client-side persistent-map push, both rewritten and app-native coherent maps are correct, so the two Create fixtures run the same buffer path in split and monolith and the P2 name-for-name comparison is honest.
  • Android delivery chain moved into P0 as spike A (server .so packaging verified through AGP, posix_spawn from the app's own untrusted_app process rather than run-as, generic --es mobilegl_env passthrough across the five trace files). External-memory feasibility became spike B so P7's schedule is known in week 1.
  • P1 split into P1a (client + inproc applier, Linux gate) and P1b (spawn transport, Linux gate); device OpenRA retrace moved to the P2 exit criterion. Total re-estimated 74 -> 77 person-days with milestones at weeks 3/5/6.
  • Spawned server must scrub MOBILEGL_TRANSPORT/MOBILEGL_IPC_* from its envp AND force Transport=Monolith before MG_Backend::Init (11.1), with a P1b process-tree count gate - otherwise an unbounded fork chain on first GL call.
  • HeadlessGL fork pre-flight orphan-server issue addressed (11.3): immediate EOF exit, bounded readiness retry, pgrep gate in P1b; cites HeadlessGL.cpp:344-368 and its own :585-589 'leaked exclusive device' note.
  • Server discovery reworked (11.1): MOBILEGL_IPC_SERVER_PATH primary with dladdr fallback, RUNTIME_OUTPUT_DIRECTORY aligned to the MobileGL library dir, env injected into every new ctest ENVIRONMENT - verified the itest links MobileGL_s statically (CMakeLists.txt:28-35) and retrace passes an explicit -DMOBILEGL_LIBRARY.
  • mobilegl_server_main declared extern "C" with explicit default visibility plus an nm -D assertion in P0 (11.2), because CMakeLists.txt:497-510 sets hidden visibility on every non-Debug build and RelWithDebInfo is what ships.
  • FlatBuffers: add_subdirectory(3rdparty/flatbuffers) removed from the default build path entirely (7.1/13) - the prior branch's flatc block IS the NDK trap - plus a CMake guard that forces the option OFF with a warning when 3rdparty/flatbuffers/include is absent.
  • Windows handle pair spelled out (11.5): GUID-named CreateNamedPipeW + CreateFileW both with FILE_FLAG_OVERLAPPED and the server end inherited, because asio's windows::stream_handle IOCP service needs an overlapped handle and CreatePipe does not give one.
  • trace-replay SPLIT plumbing detailed (13): test name gains a SPLIT suffix (current name MobileGLTraceReplay.CASE.BACKEND would collide) and -DTRACE_TRANSPORT= must be threaded through run_trace_case.cmake; both files listed as P2 deliverables.
  • Added a steady-state memory budget requirement (R14) covering client segments + full replica context + the server's three 4->64MiB rings + the 64MiB buffer pool (~450MiB), with P1a recording RSS for BOTH processes and SEG_STAGE's ceiling set by measurement.
  • P3's 'zero round trips' gate reworded to cover the whole trace-case matrix with per-fixture round-trip counts published, rather than resting on minecraft-1.21.4-main-menu which exercises neither conditional render nor occlusion queries.
  • The nm/.text monolith preservation gate is now a phase-exit criterion for every phase P0-P9, and the P4.5 allocator change is explicitly required to be #if MOBILEGL_BUILD_DISAGGREGATED-wrapped (PipeResource/MipmapStorage live in MG_State, so an unguarded allocator swap would turn the gate red).
  • Added a schedule-risk row (R15) calibrated against Feat/CS-Delta-IPC's 6668 lines / zero frames, with P2.5 named as its annealer.
  • Section 14 REUSE/CHANGE/DROP updated: the prior branch's Protocol/CMakeLists.txt flatc block moved from REUSE to CHANGE-with-deletion, HandleSessionGeneration.md gains the RBO GetVersion and Fatal-on-mismatch edits, and gen_impl_mutation_surface.py noted as having no counterpart there.

4. 被驳回的审查意见

  • 'inproc is a category error because the server must not hold MG_State' - not a flaw in this plan: the replica model deliberately links MG_State into the server, and the verified evidence (UniformManager.cpp:1418-1497 constructing real TextureObjects, VulkanRenderer.cpp:4211-4356 driving ShaderObject::Compile/ProgramObject::Link) shows a thin server is impossible regardless.
  • 'The 167 handle-ify hits mean a huge conversion surface' - already handled: the plan's own section 14 notes those counts include GLFunctionsTable declarations at BackendObject.h:158-186 and the static global at DirectGLES.cpp:55, and the replica model means no SharedPtr-keyed twin registry needs converting at all.
  • 'MarkStorageDirty(...,true) at Managers.cpp:2813 (RequireImageBindableStorage re-dirty) needs a client-visible ack protocol' - it is purely server-initiated by a server-side re-mint, is unpredictable by the client by construction, and the re-upload happens entirely on the replica; no wire traffic is needed (documented as such in the section 5.6 table).
  • 'BeginConditionalRender should become a client-side speculative pass-through' - the spec latitude is real but GL_Query.cpp:705-706 documents the always-wait choice as the only one giving the whole block one deterministic verdict; changing it is an independent monolith behaviour change, not a split concern. Kept as a listed blocking point instead.