fix(memory): lazy VirtualAlloc commit on Windows + mmap overcommit on macOS/Linux - #728
Merged
Merged
Conversation
… macOS/Linux Windows: CreateSubArena no longer calls Allocate(size) which triggered VirtualAlloc(MEM_COMMIT) for the full sub-arena budget upfront. With ImportPipeline (3.5 GB) + AssetManager (1 GB) + VulkanDevice (1 GB) committed before UIContext, pagefile quota was exhausted and the UIContext VirtualAlloc returned NULL. Sub-arenas now start with m_committed_size = 0 and commit pages lazily in ArenaAllocateRaw. Sub-arena m_memory is aligned to m_mem_page_size (4 KB) so commit boundaries never cross into adjacent sub-arenas' pages. macOS/Linux: Replace mmap(PROT_NONE) + per-allocation mprotect with mmap(PROT_READ|PROT_WRITE) + m_committed_size = size. The OS uses overcommit — physical pages are only backed on first write. This eliminates every mprotect syscall from the allocation hot path. Sub-arenas inherit m_committed_size = size (parent mmap covers their range). The mprotect branches in ArenaAllocateRaw and Resize are removed entirely for macOS/Linux. Tests: updated ArenaSubArenaLifecycle to assert platform-specific m_committed_size; added ArenaSubArenaMultipleLargeSubArenas (regression for the UIContext Windows bug) and ArenaSubArenaPageAlignedOnWindows.
2 tasks
JeanPhilippeKernel
added a commit
that referenced
this pull request
Sep 3, 2026
…mpleted/ Verified every doc's completion claim against the actual codebase — file existence, key symbols, checklist items — rather than trusting the Status line alone. Two categories of finding: Checklist-hygiene gaps (real implementation, boxes just never ticked): render-resource-manager.md, vfs-ticket2/4/5. Ticked every item after confirming the referenced file/symbol/test exists. render-resource-manager.md also got a naming-divergence note — the doc's proposed GPUResource.h/ GPUBuffer/GPUImage shipped as GpuAllocator.h's BufferView/BufferImage instead; functionally identical, different names. Genuine correctness gap: fly-camera-redesign.md claimed 'Implemented' with every one of its own checklist items unchecked, and describes an entirely different architecture (FlyCameraInput/FlyCameraState/EditorCameraController) than what's actually in FlyCameraController.h (CamState enum, SetViewportRect self-gating). Corrected the status to flag this and pointed at the real design so a future reader isn't misled. memory-allocator-audit.md — added a scope note: its 13 bugs (#497/#531) are genuinely all fixed, but later, unrelated allocator bugs were found and fixed independently this cycle (#680-683, #697, #728, #731) — pointed to the wiki's Memory Management page for the current picture. Moved to ZEngine/docs/completed/ (verified, no open items): asset-manager.md, memory-allocator-audit.md, vfs-design.md, vfs-ticket2/3/4/5/6, gpu-allocator-rearchitecture.md, render-resource-manager.md, system-scheduler.md, ui-system.md. Left in place — genuinely partial: tlsf-allocator-integration.md (Phase 3 blocked), logging-policy.md (real outstanding verification/benchmark tasks), fly-camera-redesign.md (needs a content rewrite, not just a status fix), and everything already marked Design/Planning/In Progress/Partially implemented. Cross-checked every backtick-quoted reference to the 12 moved filenames across the rest of the docs tree — all are informal textual mentions, not markdown hyperlinks, so nothing broke.
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
1 participant
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.
Summary
CreateSubArenacalledAllocate(size)which triggeredVirtualAlloc(MEM_COMMIT)for the full sub-arena budget upfront. By the time UIContext was allocated, earlier sub-arenas (ImportPipeline 3.5 GB + AssetManager 1 GB + VulkanDevice 1 GB) had already consumed pagefile quota and the UIContext commit returned NULL.m_committed_size = 0and commit pages lazily inArenaAllocateRaw. Sub-arenam_memoryis page-aligned (m_mem_page_size) soVirtualAlloc(MEM_COMMIT)boundaries never cross into adjacent sub-arenas' pages.mmap(PROT_NONE)+ per-allocationmprotectwithmmap(PROT_READ|PROT_WRITE)+m_committed_size = size. The OS uses overcommit — physical pages are backed on first write only. Eliminates everymprotectsyscall from the allocation hot path. Themprotectbranches inArenaAllocateRawandResizeare removed entirely.Test plan
ArenaSubArenaLifecycle— assertsm_committed_size = 0on Windows,= sizeon macOS/Linux; allocation into sub-arena still worksArenaSubArenaMultipleLargeSubArenas— regression: 3 x 1 MB sub-arenas from a 4 MB parent all succeed and have non-overlapping rangesArenaSubArenaPageAlignedOnWindows— Windows only: sub-arenam_memoryis a multiple ofdwPageSize