Skip to content

fix(ns2): retain pool wrappers through pending GPU work - #1852

Draft
1395490241lhj-code wants to merge 1 commit into
ly05010419:mainfrom
1395490241lhj-code:codex/ns2-pool-retirement-20261106
Draft

1395490241lhj-code wants to merge 1 commit into
ly05010419:mainfrom
1395490241lhj-code:codex/ns2-pool-retirement-20261106

Conversation

@1395490241lhj-code

@1395490241lhj-code 1395490241lhj-code commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

NS2 can finalize a memory pool after its zero-copy buffer has been encoded but before the submission completes. mtl_pool_del currently removes that buffer from the residency set immediately. Route NS2 pool wrappers through the existing fence-based deferred-release queue, while invalidating address lookups and updating pool accounting immediately. Retire unused/completed pools immediately; when no encoder is active, wait only for the last submitted fence, so idle finalization does not require another frame. Switch 1 keeps its existing behavior.

The regression compiles the production retirement, deferred-release and residency helpers, then checks real Metal residency and an unused-pool case and 64 GPU copy/readback cycles per build mode, covering encoded, submitted and already completed work. It fails on main before submitting an invalid residency state and passes with this change (262,144 checked words per mode). The test keeps caller-owned backing mapped and unchanged until actual GPU completion, then reuses it.

Validation: focused tests passed locally on Apple Silicon; complete nvn_mtl.m compiled for NS2/Pokopia and Switch 1 (using the existing -fbracket-depth=1024 requirement addressed separately in #1849). Final focused validation: 8 tests passed using a local temporary directory, including pool retirement, existing retirement races, cloud-save isolation and data-bundle signing. A development Metal4 production probe also passed 66,126 checks with the debug layer enabled.

The broad local run (excluding the known settings-panel baseline covered by #1850) reported 1,364 passed and 169 skipped, with two filesystem-related failures and one stale test/source snapshot failure while the final correction was being applied. The unchanged cloud-save/signing tests failed under Documents (file coordination timeout / disallowed signing metadata) and passed under a local temporary directory; the final pool regression passed there too. This is not a claim of a clean full-suite run. Hosted Source regressions run 42334237802 reached the job's 10-minute limit and was cancelled automatically. Its log reports four test_settings_panel failures (tracked separately in #1850); the new pool regression compiled but both Metal subtests were skipped because the runner had no device with residency-set support. Later sanitizer steps did not run. GitGuardian passed. Hosted CI is therefore not green, and GPU validation here is the local Apple Silicon evidence above.

Scope: this repairs Metal wrapper ownership/residency. It does not extend the lifetime of guest-owned bytes or change host virtual-pool unmap/remap behavior; those still require separate lifetime validation. Registration autorelease behavior is tracked in #1841 and isolated by a local scope in this regression. No game files are included.

Adapted from nxnative ca90b26f (MIT), with attribution in THIRD_PARTY_NOTICES.md. Draft; no full-game acceptance claimed.

Integration validation (2026-11-07): all three jobs passed on the exact combined commit 360180ae85f263e3fa57ffe6bceddfcab3855e63 (fork CI run). This separate validation branch merges the unchanged heads of #1841, #1849, #1850, #1852 and #1856; none of these PR branches was rebased or expanded.

  • Hosted core: 1,544 tests, 11 skips, no failures; settings: all 3 tests passed; final gate and all 8 UBSan checks passed.
  • The hosted runner has no usable Metal residency-set device: the registration test and both pool-retirement modes were skipped. Their real-Metal cases passed locally in the complete combined suite: 1,547 tests, 169 environment-dependent skips, no failures; all 8 UBSan checks also passed locally.
  • This is combined-source evidence, not a green status for each original PR head, a packaged-app test, or gameplay/performance acceptance. The independent diffs remain unchanged; the four runtime/build fixes still need their own heads refreshed and validated after the CI repair in Fix compact settings probe assertions and split source CI #1850 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant