Repository navigation
mir: reserve 32 MB of address space per context for its code, not 128 (#1215) - #1327
Open
elyfmiller wants to merge 3 commits into
Open
elyfmiller wants to merge 3 commits into
elyfmiller wants to merge 3 commits into
Conversation
…lanl#1215) Since lanl#1208, each MIR context reserves MIR_CODE_RESERVE_SIZE, upstream's 128 MB on 64-bit hosts other than Windows, so that its generated code's direct branches reach each other. It holds no memory, but a cap on the address space (ulimit -v, RLIMIT_AS) counts it: about 60 live MIR-JIT'd models filled 8 GB. The new CMake option BNGSIM_MIR_CODE_RESERVE_MB (default 32) sets it for bngsim_mir. 32 MB holds the largest committed model's code in one piece (fceri_gamma, 27.9 MB; fceri_fyn, 7.4 MB). Code that outgrows it is mapped on its own, as MIR does once a reservation is full. 0 reserves nothing; Windows keeps MIR's own 0. No vendored file changes. Measured on macOS arm64, 200 live MIR Simulators: 128.1 MB of address space each at 128, 32.1 at 32, 0.1 at 0, with RSS the same at all three.
…nl#1215) test_mir_vendoring.py's modules are part of run_tests.sh's walk-up probe (its -k "vendoring"), which allows no skip it cannot excuse. The live-model test skipped on every build without MIR, so test_run_tests_sh failed on all three Python legs. It now lives in test_mir_equivalence.py, which mir.yml runs and which skips as a whole, with its declared reason, without MIR. The static check of the CMake option stays in test_mir_vendoring.py.
40 MB a model is the default 32 plus the 0.1 MB of other mappings measured at 0, with 8 MB to spare, and upstream's 128 fails it by a factor of three.
This branch has not been deployed
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.
Fixes #1215.
Problem
Since #1208 moved the vendored MIR to upstream 439a90a2, each MIR context reserves 128 MB of address space for its generated code at
MIR_init, on 64-bit hosts other than Windows (third_party/mir/mir.c,MIR_CODE_RESERVE_SIZE). That keeps the context's code within its own direct-branch reach (±128 MB on aarch64). It holds no memory, but a cap on the address space (ulimit -v,RLIMIT_AS) counts it. With about 60 live MIR-JIT'd models, a cap of 8 GB is full.Measured on main (aedadd8), MIR build, macOS arm64, 200 live
Simulator(..., codegen=True)ofsimple_decay.netunderBNGSIM_CODEGEN_JIT=mir:Fix
The fix is a CMake cache option,
BNGSIM_MIR_CODE_RESERVE_MB(default 32). It setsMIR_CODE_RESERVE_SIZEforbngsim_miron 64-bit hosts other than Windows. A value that isn't a whole number is refused at configure time.0reserves nothing, and Windows keeps MIR's own 0. No vendored file is edited; the define overrides MIR's#ifndefdefault.Why 32 MB. It holds the largest committed model's code in one piece. That matters for locality only, not correctness. bngsim generates code eagerly (
MIR_linkwithMIR_set_gen_interface). Its calls go through a register on aarch64, and the thunk redirects check their reach on both targets, so code mapped on its own runs the same. Measured withvmmapas the resident size of each context's reservation,BNGSIM_CODEGEN_JIT=mirat the automatic optimization level:simple_decay.netfceri_fyn.netfceri_gamma.net(the largest.netinbenchmarks/models/net/ode)Code that outgrows the reservation is mapped on its own, which is what MIR does once a reservation is full. Before #1208 every context did that.
Results
The same 200 live models, on this branch's MIR build with the option set three ways:
BNGSIM_MIR_CODE_RESERVE_MBTests
Two tests, both in files
mir.ymlalready runs:test_cmake_sizes_the_mir_code_reservation, intest_mir_vendoring.py: CMake sets the option and the define, and the vendored default it overrides is still upstream's 128 MB, so a refresh that moves it is seen.test_a_live_mir_model_holds_far_less_than_upstreams_reservation, intest_mir_equivalence.py: 20 live MIR-JIT'd models in a subprocess, each holding at most 40 MB of address space. That catches a fall back to upstream's size, not the exact setting. It reads/proc/self/statuson Linux andps -o vszelsewhere.test_mir_equivalence.pybecausetest_mir_vendoring.pyis part ofrun_tests.sh's walk-up probe (-k vendoring), which allows no skip. In its first place, it failedtest_run_tests_shon the three Python legs (5bfc698 moves it).test_a_live_mir_model_holds_far_less_than_upstreams_reservationassert 128.0 <= 40.0test_cmake_sizes_the_mir_code_reservationTest audit
Checked against the test-writing rules Bill sent: each expected value's oracle, each tolerance's reason, and each test shown to fail against broken code. Commit 64936e8.
Other checks:
mir.yml's test list, all 35 files, on this branch's MIR build underBNGSIM_CODEGEN_JIT=mirandBNGSIM_SKIP_AUDIT=strict: 1,318 passed, 1 xfailed.test_mir_vendoring.py,test_skip_audit.py,test_ci_run_list_coverage.pyandtest_changelog_fragments.pyunder the strict audit: 116 passed on the MIR build. On a build without MIR, the first two give 44 passed and 1 skipped, and the skip is a declared one.scripts/vendor_mir.py --check: OK (third_party/mirmatchesVENDOR.json).ci/changelog.py checkand ruff 0.14.0: clean.The change is CMake inside
if(BNGSIM_ENABLE_MIR)plus tests and docs, so a default build is untouched. perf_ab doesn't apply: it measures the default build.Review
No second session was available, so a separate agent reviewed the diff, read-only:
Not tested here
ulimit -v. macOS can't setRLIMIT_AS, and Docker wasn't running. The Linux MIR leg ofmir.ymlruns the new test, which measures the same reservation.BNGSIM_MIR_CODE_RESERVE_MBthat isn't a whole number. It runs in CMake, and no test configures the project.