Repository navigation
feat(libs): let relinked Windows games opt into mimalloc - #6067
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
f23a370 to
9a0e5fc
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, the measurement and the fallback when mimalloc.dll is missing are clear. This replaces the allocator of every relinked game on Windows, so it needs more than the packaging test:
WindowsEntryStubBuilder.cpp:122-131and:493-514emit new machine code (aLoadLibraryExWper allocator path, in both the absolute-path and the relative-path branch with thePathCapacitycheck) with no test.core/relinker/elfpatcher/tests/EntryStubTests.cppis the place: pin that the stub referencesmimalloc.dllbefore the first library in both branches, and that an over-long path throws like the library paths do.core/libs/allocator/PeImportFirst.cpphas no test. It rewrites a PE import directory in place; add a registered test (runs on Linux CI) with a small synthetic PE showing the named import moves to first and the others keep their order, and that a missing import, a truncated directory and a non-PE32+ file fail without touching the file.docs/dev/ARCHITECTURE.mdshould describe the new process-wide allocator, the load order in the entry stub, and theRuntimeHeap.cwraps (which calls of mimalloc itself go to the process heap and why), under the section it changes.- The numbers come from a base with extra commits of your fork, one title, one machine. That is fine as motivation, but please also say how the redirection was checked (e.g. that
mallocin a guest-bundled module and in a host library both land in mimalloc, and thatfreeacross the two works), since a pointer mixed between mimalloc and the CRT heap would corrupt silently.
Build and the Linux test suite are fine (the failures I saw were load-related and pass alone). The Windows side still needs the full Build once these are in.
8e5e2ef to
8977d03
Compare
|
Thanks for the review. All four points are in 8977d03 and the description:
Both new tests fail when the behaviour they pin is broken (the allocator loaded after the libraries; an import move that does nothing). |
adriadam10
left a comment
There was a problem hiding this comment.
Re-checked the four points against 8977d03:
entry_stubnow pinsmimalloc.dll's path and itsLoadLibraryExWcall before the first library, for both an absolute and a module-relative run path, and that a path one character past the startup buffer throws.pe_import_firstis split into header/functions/main and has a registered test with a synthetic PE32+ image (named import first, others keep order, missing import / truncated directory / PE32 fail without touching the file).- ARCHITECTURE.md has the "Windows allocator" section.
- The redirection check (checked frees, no errors, guest heap on the guest arena) is in the description.
Built on the merged tree; pe_import_first, entry_stub and release_package pass. Merging once the full Windows Build is green.
|
Picking this up while @adriadam10 is away. The code and tests look good to us, and full CI passes on 8977d03. Since this changes the allocator for every relinked Windows game, two things before we merge:
Thanks for the work on this! |
|
Thanks @oneandonlydean, both points are done. 1. Opt-out switch. de23d16 adds
2. Results on current main.
Allocator checks:
Frame times:
No second GPU vendor here, sorry. |
|
Thanks, the opt-out looks right. Could you post two more runs per arm (main vs this PR, same scene) so the p99 change isn't resting on one sample, ideally one of them on a second GPU vendor? And could you say what the 325 ms warm-up round is? Please also add an |
|
Heads-up: #6078 has merged, so the environment reference check now runs on main. |
|
A Linux data point, in case it helps decide whether this should also cover Linux later. I built mimalloc from this PR's submodule commit (f8401befa6, v3.5.4) as Shader preparation.
System time is under 0.5 s in all of them. The largest single compute shader in my dumps (PPSA31162 Gameplay. On 10-16 I ran MXGP on a test build from that day (5f469824dc, with my #6917 on top), 130 s with pad input and a warm cache, 3 runs each for glibc and mimalloc and 2 for jemalloc. The frame times in the 10 s trace window did not change measurably: p50 225 to 242 ms with glibc, 222 ms with jemalloc, 221 to 295 ms with mimalloc. The GPU was busy about 120 ms per present in every run, so that scene is GPU-bound here. In a cold-cache run of PPSA31162 on 24 host threads, the preparation threads kept up with the game either way, so the allocator showed up as idle CPU rather than more shaders. I only see a clear difference in CPU time. If it would be useful once this is merged, I can do the Linux side: the relinked ELF would load AI-assisted: yes (Claude Code) |
|
Cross-validation on Windows 11 Pro 26220, NVIDIA GeForce RTX 4070 (driver 596.21), Ryzen 7 5700X, WinLibs MinGW-w64 GCC 15.2.0, Release. PR head Tests. Runs. Three titles, three arms, 3 runs each, interleaved (main, PR, PR with
Median (min-max) of 3 runs.
Not checked: your titles (GoW Sons of Sparta, Sonic Origins Plus) aren't available here; AMD/Intel; Linux; sessions longer than 65 s; the full ctest suite. Written with Claude Code assistance (validation by a subagent, reviewed in this session). |
|
Thanks @oneandonlydean.
The 325 ms round was warm-up, not the allocator:
More runs. Sons of Sparta again, same scene and window:
Plain main 5bf2e13 could not complete a traced run on this title:
Still no second GPU vendor here, sorry. |
de23d16 to
8e2c330
Compare
|
Thanks for the honest re-measure, and thanks @screamm for the three-title runs. Putting them together: no Windows run shows a frame-time win (your title is neutral now that the worker-busy mode is accounted for), and screamm's runs show p99 worse on two paced titles (14.96 vs 8.71 ms, 19.0 vs 17.3 ms) and +80-230 MiB private memory, with So we'd rather not swap the allocator for every relinked Windows game by default. Could you invert the switch, so mimalloc loads only when it's asked for (for example |
8e2c330 to
9824c46
Compare
|
Done, thanks both, and thanks @screamm for the three-title cross-check.
|
What
On Windows, a relinked game and its libraries allocate through the CRT, which goes to the Windows heap. On the S2 page of Sonic Origins Plus's main menu, the queue worker spent about 32 samples per frame (APS5 sampler, 15 s) in
RtlAllocateHeap,_malloc_baseand_free_base.This builds mimalloc 3.5.4 (a new submodule) as
mimalloc.dll, with its Windows redirection (MI_OVERRIDE,MI_WIN_REDIRECT).mimalloc.dllandmimalloc-redirect.dllare copied beside the libraries. WithAPS5_MIMALLOCset (to any value), the relinker's Windows entry stub loadsmimalloc.dllbefore every other library, so the redirection replaces the CRT allocator for the whole process. Without the variable the stub skips the load and the CRT heap stays, so nothing changes by default. Withoutmimalloc.dllthe stub's load fails quietly, and the CRT heap stays.pe_import_first, a build tool, makesmimalloc-redirect.dllthe first import of the copiedmimalloc.dll. The redirection needs this, and MinGW's linker does not guarantee the order.RuntimeHeap.csends mimalloc's own CRT calls (malloc,calloc,realloc,free,strdup) to the process heap through linker wraps, so they never reach the redirected allocator.mimalloc.dlllinks statically: GCC's emulated thread-locals (emutls, which hold mimalloc's own thread-locals), MinGW's thread-exit destructor list, and winpthreads.tools/package_release.pyships both DLLs.entry_stub(the relinker's entry stub tests, now linked withrelinker_core) pins the new stub code, for an absolute and for a module-relative run path:APS5_MIMALLOCwithGetEnvironmentVariableWbefore the allocator load, and an unset variable branches past exactly that load;mimalloc.dll's path before the first library's;LoadLibraryExWand the library search flags before that library;mimalloc.dllpath cannot fit the startup buffer fails with the same error as a library path, one character past the longest path that fits.PeImportFirst.cppis split into a header, its functions and the tool'smain.pe_import_first(a test registered on every platform) builds a small PE32+ image:docs/dev/ENVIRONMENT.mdgets theAPS5_MIMALLOCrow.docs/dev/ARCHITECTURE.mdgets a "Windows allocator" section: the two DLLs, the load order in the entry stub, guest versus host allocations, and the wraps.With mimalloc, the Windows heap's share fell from about 32 samples per frame to 0, and
operator newfell from 10.1 to 6.5 (Sonic Origins Plus, on this PR's first base).The allocator is opt-in after review. On current main, Sons of Sparta (two runs per arm) and three titles measured by @screamm show no frame-time win, and mimalloc adds 80-230 MiB of private memory. So it stays off unless
APS5_MIMALLOCasks for it.Tested
Windows 11 (10.0.26300), WinLibs GCC 15.2.0 (UCRT), Release, NVIDIA GeForce RTX 2080 Ti, driver 617.14.
entry_stub,pe_import_firstandrelease_package.python tools/env_reference.py --checkandtools/check_conventions.pypass.APS5_MIMALLOC=1both load,mimalloc: malloc is redirected.and the v3.5.4 banner appear, and there are no mimalloc errors.CMakeLists.txtincludescmake/ZArchive.cmakewhere the mimalloc block goes; both stay.tools/package_release.pyalso packs fix(release): include the enabled Tracy runtime #5729's Tracy runtime, so the Windows block adds the mimalloc DLLs and the Tracy files follow for every platform, and fix(release): include the enabled Tracy runtime #5729's Tracy packaging test gets the mimalloc DLLs in its Windows fixture. On that main, with this change, the full ctest suite passes 1339 of 1342, includingentry_stub,pe_import_first,release_packageandwindows_import_modules.agc_driver_gpu_indirect_arguments_misaligned_writer_aheadis not run: main itself does not compile it with MinGW.select_testsfails in its setup on Windows without the symlink privilege (WinError 1314fromsymlink_to), as on main.agc_driver_store_over_untrackedfailed once in the parallel run; alone it passes 5 of 5.entry_stubfails; with the import move made a no-op,pe_import_firstfails.pe_import_firstare this PR's):MIMALLOC_VERBOSE=1andMIMALLOC_SHOW_ERRORS=1. mimalloc reportedfree mode: checkedandmalloc is redirected.freeof a block mimalloc does not own reports an error. The run reported no error and no warning.RtlAllocateHeap,_malloc_baseand_free_baseappear in no stack, and mimalloc is in about 9% of them.libSceAgcDriverand 1 fromlibc, both entering throughlibstdc++(operator newanddelete).malloconly when there is no arena) and frees them the same way, so no block moves between the guest heap and mimalloc. The new ARCHITECTURE.md section says so.nvoglv64.dllandnvgpucomp64.dlllink private copies of mimalloc 3.1.6, which printed their own reports underMIMALLOC_VERBOSE. They do not take part in the redirection.APS5_TRACE_SECONDS=200:5on the same machine in one session, caches warm. Traces attached.The FPS shown in the title went from 4.99 to 5.70 (median of six readings).
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)Traces and summary lines (baseline 61ac8fd, this change 02d5af5):
mimalloc-traces.zip