Streaming without holes: texture levels survive a device loss in the world cache and yield to the pages (#745) - #777
Merged
Merged
Conversation
… device loss, yielding first to the pages kept (#745)
… order, one room rule, one cook per store, reads tracked by the session (#745)
…ce, and a level landing between sessions takes no page's place (#745)
… yield notice, a session with no world holds its levels in its own page cache (#745)
…ght hold their room, the reader names its cook, the level bytes metric says the CPU total (#745)
…ge cache, and the session's reader reads them there (#745)
…e same in 200 lines each (#745)
…s joined, under 200 lines (#745)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #745
What changed
owned by the world's page cache (
PageCache.levels,texture/levelStore.ts). A lost device'ssession is reopened on that cache, so its tiles are cut again from the levels held and no level
is fetched again. The store keeps one cook's levels (
key): another scene's leave as its readeris made, and a read landing after a scene switch or after the world is gone keeps nothing (the
reader names its cook,
read.key). A session with no world holds its levels in the page cacheits streamer reads through (
streamer.pageCache), within its own CPU total, cleared as thestreamer is disposed.
LEVEL_CACHE_BYTES) is gone. The cap is threequarters of the pages' share of
world.budget.cpu(textureLevelShare, published assplit.textureLevels, 192 MiB at the default total) and follows the total live. The levels countagainst the same total (
reservedBytes). They get the room the session's reservations, the keptproxy and the pages a frame keeps leave them (
PageCache.levelRoom).streaming/cache.tsyieldBeside). When the pages a frame keepsor reads do not fit, the levels yield first, least recently read first (
page-cache-levels-yielded),then the kept proxy. Pages are walked once per
evict, and only when something is held besidethem.
shedTostops at the first entry that fits (through the sharedevictOldest).bytes (
requestedLevelBytes), less the reads in flight. The room is weighed once per frame. Alevel landing without room is not held. Its tile is
refused, notwaiting, so the framesettles; it stays on its coarser level until room comes back.
docs/SDK.md(the CPU split; the lost-device limit no longer says baked texture levels areread again),
docs/ENGINE.md, andtextureLevelCacheBytes(no longer "under a fixed budget",English and the 13 API reference translations).
Proof
Behaviour tests (
packages/sdk-browser/src/texture/levelStore.test.ts). They fail ondevelop,which has no level store in the page cache and no
split.textureLevels:after a device loss the texture levels are rebuilt with no level read again: 0 texturerefetch; a read landing after another cook opens keeps nothing.
a level read by both sides of a device loss is counted once.a small CPU total: no page a frame keeps is refused for a texture level, the levels yield first:every page stays, 0 evictions, 0 admission blocks, the levels yield and the proxy stays. The
level that cannot fit is not read again over the following frames, and its request is refused.
the texture levels cap follows world.budget.cpu live.worldBudget.test.ts: the default split publishestextureLevels.backendsTextureLevels.test.ts:the baked levels are held in the cache the session's pages are read through(a session with no world).Gates run in the worktree after merging
origin/develop(reviewer):pnpm run check:changed(2608pass, 0 fail, 1 skipped),
pnpm run test:changed(2608 pass, 0 fail),pnpm run validate --group quick(pass),pnpm run validate --group typescript(pass),node scripts/check-pr-size.ts(416hand-written lines, limit 600). No Chrome, browser
proof,
test:gpu, perf or bench was run.Local review before push
Simplification pass: (coder) The real
simplifyskill ran with 4 agents. What it found and what was fixed:shedTore-implementedevictOldest; it now uses it.bitmapBytes.getkept a deadframeparameter; removed.PageCache.levelRoom.#keysuffix); the store now holds one cook and ids carry no key.refusedgrew across scenes; pending and refused reads are now tracked by the session.evictwalked the pages even with nothing beside them; it now skips the walk.take/resizeshed only to the share; they now shed to the room left beside the pages.Skipped: a generic list of what is held beside the pages (medium refactor), a running counter of the pages kept, removing a no-world session's own store, and an explicit context field for the store.
Correctness review: (coder) The real
code-review --fixskill found 5 issues; 2 fixed, 3 left:takecounted a level twice (and leaked its bitmap) when both sides of a device loss read it. It now keeps the held one and closes the copy (new test).get; the page walk behindroom()on a landing (both minor).Simplification pass: (reviewer) The real
simplifyskill ran with 4 agents (reuse, simplification, efficiency, altitude). Fixed:takenow owns every level it is handed: it closes a stale cook's, a duplicate and one that cannot fit; the tile layer no longer closes levels or guards the key itself.dropis private; the two yield notices share oneyieldedhelper.streamer.pageCache), so they count in its total and yield to its pages (new test inbackendsTextureLevels.test.ts).Skipped:
levelRoomwritten frombudgetBytes(differs when the budget clamps at 0); a generic ordered list of what is held beside the pages (medium refactor); the streaming layer importing the texture share; a running counter of held page bytes and the second page walk on each level landed (minor);get's Map reinsertion; passing the store beside the reader instead of on it.Correctness review: (reviewer) The real
code-review --fixskill found 7 issues; 3 fixed, and the reviewer fixed one more doc finding:waitingforever, so the frame never settled (texturesPendingevery frame).requestnow says so andsources.tsreturnsrefused(assertion added).key.textureLevelCacheBytesstill said "under a fixed budget", in English and 13 translations.evictdoes not shed levels before pages (no path callskeepor lands a page then); an own cache's levels cap is 3/4 of its enlarged total (about 198 MiB, not 192 MiB);split.textureLevelsand the page cache compute the share separately;textureLevelsDecodedcounts a read discarded on landing.Auditor list: (reviewer)
Closes #745; both To-do items of the body delivered, with tests that fail ondevelop(notexture/levelStore.ts, nosplit.textureLevels, nostoreon the reader), on theservedPagesfixture, waiting onsettled()and the requests, never a fixed delay. Labelsbug,geometry,🔴 critical,in review. Docs and translations follow. No format changed. Streaming without holes: rules and objectives for geometry, memory and shadows #483 rules 1, 4 and 5 held; rule 9's second scene and the browser run are the measurer's. Open: the CTO's third To-do (texture tiles' ms ceiling) is not delivered and must be moved off Texture levels survive a device loss and yield to the pages a frame keeps #745 by the CTO before this body saysCloses #745.#483 checklist
without reloading, now for texture levels too) are held, with tests that fail on
develop.Rule 4 (one memory budget): the levels' fixed 192 MiB is deleted and they count in
world.budget.cpu. Rules 2, 3, 6, 7, 8 and 9 are not moved by this change, and rule 10 is nottouched (nothing is rebuilt per frame; the room is weighed at most once a frame, only while a
level waits). Rule 7, main-thread share: not measured here (see below).
Closes #745: the two To-do items of the body are delivered. The CTO's To-do (texture tiles'own per-frame ms ceiling) is left out; see below.
LEVEL_CACHE_BYTESand the tile layer's own LRU (makeRoom) are deleted.The levels shed through the shared
evictOldestand yield in the page cache's single evictionpath.
origin/develop(47bb4ef); CI is the lead's to check.issue gets
to measureat merge.Not proven / left out
(
webgpu/tile/streamer.ts,budgetMs) should use the one session integration budget(Streaming without holes: rules and objectives for geometry, memory and shadows #483 rule 4). That budget is created by Streaming without holes 11/12: a large world loads its object table by distance, without crashing the tab #404's branch
404-audit-remainders, which is not ondevelop, so the item waits for that merge.memory rows of Streaming without holes: rules and objectives for geometry, memory and shadows #483 ("Peak GPU and JS memory never above the declared budget") are not
measured here.
at its coarser level (not counted in
textureTilesRefused, which counts the pool's refusals), andonly
page-cache-levels-yieldedreports the levels that were shed. A metric would need the 13API reference translations.
reach that level (Streaming without holes: rules and objectives for geometry, memory and shadows #483 rule 5: out of memory is one level coarser); it does once room comes back.
Lead verification
Read by the geometry lead on head
6f3bead94against #745 (split from #726). Its third To-do, the texture tiles' ms ceiling, was moved to #751 by the CTO, as the body of #745 says. The measurer proved it before this pull request opened (CTO rule of 26 Sept.).packages/sdk-browser/src/texture/levelStore.ts:17(createTextureLevelStore, held by the world's page cache asPageCache.levelsand keyed by the cook key, withkeepOnly/close). The session's reader reads through it (world/session/backends.ts). Proved bylevelStore.test.ts"after a device loss the texture levels are rebuilt with no level read again" and "a level read by both sides of a device loss is counted once" (fail on develop). The measurer forced a device loss on marble-bust: 0 requests after it, first frame 54 ms after the loss, the image complete.world.budget.cpu: delivered instreaming/cache.ts:50(yieldBeside: the levels first, then the proxy) andresidency/memoryBudget.ts(textureLevelShare, published assplit.textureLevels). The fixed 192 MiB is deleted. A level that cannot fit is refused, and its tile stays on its coarser level (webgpu/tile/sources.ts:119). Proved by "a small CPU total: no page a frame keeps is refused for a texture level, the levels yield first", "the texture levels cap follows world.budget.cpu live" andbackendsTextureLevels.test.ts"the baked levels are held in the cache the session's pages are read through". The measurer ran a CPU total just above its floor:admissionBlocked0,textureTilesRefused0, tiles coarser, none missing, no hole.check:changedandtest:changed2608 pass, 0 fail;validatequick and typescript pass (reviewer,6f3bead94).b02f09ff5): 0 px at full budget on marble-bust (A/A 0); compressed-textures varies against itself on both sides.test:gpu60/66, the 6 failures develop's known ones. The JS heap exceeds the CPU total by the same amount on base and branch (74.2 and 74.6 MB), because the page and runtime weight is outside what the total covers; this is not the branch's doing.levelStore.ts, nosplit.textureLevels). They useservedPagesand wait on settled requests.docs/SDK.md.LEVEL_CACHE_BYTESare deleted, and the yield shares one helper with the proxy. A session with no world uses its own page cache's levels, not a second store.docs/SDK.md(the CPU split; the lost-device limit no longer says levels are read again),docs/ENGINE.md, andtextureLevelCacheBytesinsdk-core/src/texture/metricsContracts.tswith the 13 API reference translations. The body closes Texture levels survive a device loss and yield to the pages a frame keeps #745.in reviewis removed andto measureset at hand-over.c1d7fce56,66609b1a3,ec4a1021a, develop merged):textureLevels), not its page cache, soPageCacheandHolderstay internal (public-types-audit.test.ts:115);world/session/backends.tsis trimmed to 194 lines: its two preparation diagnostics share one context;backend/types.tsis left to Post-processing chain: bloom, depth of field, motion blur, outline, LUT and custom passes after the resolve #349 by the CTO's word.There is no behaviour change. The lead re-read it as the short re-review.
validate --group native3377 pass, 0 fail;test:changed2644 pass (coder).backend/types.tstrimmed to 200 lines at the CTO's word (8d13decee): only comments are joined, with no behaviour change, re-read by the lead.