From ce41e6552d3c3069fab7e8f0a34e3624ab003aa9 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 13:27:26 -0400 Subject: [PATCH 1/9] Compile each variable file once per test process Coverage's sys.monitoring core (COVERAGE_CORE=sysmon on CI) keeps every code object it has seen alive. policyengine-core compiles a system's variable files again for every new system, so each Simulation left about 2,500 code objects behind and the test shards grew to the runner's memory. The conftest now returns the code object compiled the first time for each variable file, so every system runs the same code with its own modules, classes and functions. Fixes #2330 Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 40 ++++++ .../tests/test_variable_code_sharing.py | 120 ++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 changelog.d/2330.fixed.md create mode 100644 policyengine_uk/tests/test_variable_code_sharing.py diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md new file mode 100644 index 000000000..54046df6d --- /dev/null +++ b/changelog.d/2330.fixed.md @@ -0,0 +1 @@ +- Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 1ec459c81..d4d018d77 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -5,6 +5,7 @@ import re import sys import weakref +from importlib.machinery import SourceFileLoader import pytest from policyengine_core.taxbenefitsystems import TaxBenefitSystem @@ -234,3 +235,42 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: def _release_variable_modules_of_dead_systems(): yield release_dead_variable_modules() + + +# Coverage on CI is measured with sys.monitoring (COVERAGE_CORE=sysmon). That +# core keeps every code object it has seen alive until the process exits, so +# that it can key its records by id(code) (SysMonitor.code_objects in +# coverage/sysmon.py). policyengine-core compiles a system's variable files +# again for every new system, into new code objects, and every Simulation +# builds a new system, so each Simulation left about 2,500 code objects and +# 4 MB that were never freed. On CI the four workers of a shard grew to the +# whole runner's memory (15.3 of 15.6 GiB). +# +# A file's code object does not depend on the system that runs it, so each +# process compiles each variable file once and every later system runs that +# code object. Systems still get their own modules, classes and functions; +# only the immutable code is shared. Remove this once policyengine-core +# shares the code itself. +_variable_code = {} +_get_code = SourceFileLoader.get_code + + +def _get_shared_variable_code(loader, fullname): + """``SourceFileLoader.get_code``, compiling each variable file only once.""" + if not _VARIABLE_MODULE_NAME.match(fullname): + return _get_code(loader, fullname) + path = loader.get_filename(fullname) + stat = os.stat(path) + key = (path, stat.st_mtime_ns, stat.st_size) + code = _variable_code.get(key) + if code is None: + code = _variable_code[key] = _get_code(loader, fullname) + return code + + +def pytest_sessionstart(session): + SourceFileLoader.get_code = _get_shared_variable_code + + +def pytest_unconfigure(config): + SourceFileLoader.get_code = _get_code diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py new file mode 100644 index 000000000..f6364a3b6 --- /dev/null +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -0,0 +1,120 @@ +"""The conftest compiles each variable file once and shares the code. + +policyengine-core compiles a system's variable files again for every new +system. Coverage's sys.monitoring core, which CI uses, keeps every code object +it has seen alive, so each Simulation used to leave its code objects behind. +The conftest makes every system run the code object compiled the first time. + +Systems must still get their own modules, classes and functions, run the code +of the file's current source, and compute the same results. If +policyengine-core starts sharing the code itself, the upstream check skips +with a note to remove the workaround. +""" + +import sys +from importlib.machinery import SourceFileLoader +from pathlib import Path + +import pytest + +from policyengine_uk import Simulation + +SITUATION = { + "people": {"adult": {"age": {2026: 40}, "employment_income": {2026: 30_000}}}, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": {"household": {"members": ["adult"]}}, +} + + +def _conftest(request): + path = Path(__file__).with_name("conftest.py") + for plugin in request.config.pluginmanager.get_plugins(): + if getattr(plugin, "__file__", None) and Path(plugin.__file__) == path: + return plugin + raise AssertionError(f"{path} is not loaded as a plugin") + + +def _income_tax_formula(simulation): + variable = simulation.tax_benefit_system.variables["income_tax"] + return variable.get_formula() + + +def _coverage_code_objects(): + """Code objects coverage's sys.monitoring core holds, or None if inactive.""" + try: + import coverage + except ImportError: + return None + cov = coverage.Coverage.current() + collector = getattr(cov, "_collector", None) + if collector is None: + return None + lists = [getattr(tracer, "code_objects", None) for tracer in collector.tracers] + lists = [objects for objects in lists if objects is not None] + return sum(len(objects) for objects in lists) if lists else None + + +def test_systems_share_code_but_not_modules_classes_or_functions(): + first = Simulation(situation=SITUATION) + second = Simulation(situation=SITUATION) + first_class = type(first.tax_benefit_system.variables["income_tax"]) + second_class = type(second.tax_benefit_system.variables["income_tax"]) + first_formula = _income_tax_formula(first) + second_formula = _income_tax_formula(second) + + assert first_class is not second_class + assert ( + sys.modules[first_class.__module__] + is not (sys.modules[second_class.__module__]) + ) + assert first_formula is not second_formula + assert first_formula.__globals__ is not second_formula.__globals__ + assert first_formula.__code__ is second_formula.__code__ + assert first.calculate("income_tax", 2026) == second.calculate("income_tax", 2026) + + +def test_shared_code_is_compiled_from_the_current_source(request): + Simulation(situation=SITUATION) + shared = _conftest(request)._variable_code + variables_dir = Path(sys.modules["policyengine_uk"].__file__).parent / "variables" + # Core reads a directory's __init__.py for metadata and runs every other file. + files = { + str(path) for path in variables_dir.rglob("*.py") if path.name != "__init__.py" + } + + assert shared, "no variable code was shared" + for (path, mtime_ns, size), code in shared.items(): + stat = Path(path).stat() + assert (stat.st_mtime_ns, stat.st_size) == (mtime_ns, size), path + source = Path(path).read_text(encoding="utf-8") + assert code == compile(source, path, "exec", dont_inherit=True), path + assert {path for path, _, _ in shared} >= files + + +def test_policyengine_core_still_compiles_each_system_again(request, monkeypatch): + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + first = _income_tax_formula(Simulation(situation=SITUATION)) + second = _income_tax_formula(Simulation(situation=SITUATION)) + if first.__code__ is second.__code__: + pytest.skip( + "policyengine-core now shares variable code between systems; " + "remove _get_shared_variable_code from conftest.py and this file" + ) + assert first.__code__ == second.__code__ + + +def test_a_simulation_leaves_coverage_no_new_code_objects(request, monkeypatch): + if _coverage_code_objects() is None: + pytest.skip("coverage is not measuring with sys.monitoring") + Simulation(situation=SITUATION).calculate("income_tax", 2026) + + before = _coverage_code_objects() + Simulation(situation=SITUATION).calculate("income_tax", 2026) + shared = _coverage_code_objects() - before + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + Simulation(situation=SITUATION).calculate("income_tax", 2026) + unshared = _coverage_code_objects() - before - shared + + # Without sharing, a Simulation leaves about 2,500 code objects. + assert unshared > 1_000, unshared + assert shared < 100, shared From e41150e20bf6c3eb6c04ed10d1b844f4e1bded04 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:37:40 -0400 Subject: [PATCH 2/9] Release dead systems' variable modules as each new system loads A property test on CI had 165 systems' variable modules loaded at once, because dead systems' modules were released only after each test, and Hypothesis builds hundreds of systems while it shrinks a failing example. The loader hook now also runs release_dead_variable_modules() when a new system starts loading its variables, so at most max_owners dead systems' modules are held. Also record the CI before/after numbers in the comment. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 31 ++++++++++++++----- .../tests/test_variable_module_release.py | 12 +++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 54046df6d..8cffd45d4 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1 +1,2 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). +- Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index d4d018d77..c84345ad5 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -242,23 +242,38 @@ def _release_variable_modules_of_dead_systems(): # that it can key its records by id(code) (SysMonitor.code_objects in # coverage/sysmon.py). policyengine-core compiles a system's variable files # again for every new system, into new code objects, and every Simulation -# builds a new system, so each Simulation left about 2,500 code objects and -# 4 MB that were never freed. On CI the four workers of a shard grew to the -# whole runner's memory (15.3 of 15.6 GiB). +# builds a new system, so each Simulation left about 2,500 code objects that +# were never freed. By the end of a CI shard each pytest-xdist worker's +# coverage held 0.8 to 1.5 million of them, and the shards peaked at 14.4 to +# 15.3 GiB of the runner's 15.6, using up to 1.8 GiB of swap (run 38159894687). # # A file's code object does not depend on the system that runs it, so each # process compiles each variable file once and every later system runs that # code object. Systems still get their own modules, classes and functions; -# only the immutable code is shared. Remove this once policyengine-core -# shares the code itself. +# only the immutable code is shared. In the same run this lowered the shards' +# peaks to 10.2 to 12.2 GiB with no swap, cut their pytest time by 15 to 21%, +# and left every test's outcome unchanged. Remove this once policyengine-core +# shares the code itself (PolicyEngine/policyengine-core#608). +# +# The same hook also releases dead systems' variable modules whenever a new +# system starts loading its variables, not only after each test: in that run +# one property test had 165 systems' modules loaded at once, and Hypothesis +# builds hundreds of systems while it shrinks a failing example. _variable_code = {} _get_code = SourceFileLoader.get_code +_loading_owner = None -def _get_shared_variable_code(loader, fullname): +def _get_variable_code(loader, fullname): """``SourceFileLoader.get_code``, compiling each variable file only once.""" - if not _VARIABLE_MODULE_NAME.match(fullname): + global _loading_owner + match = _VARIABLE_MODULE_NAME.match(fullname) + if not match: return _get_code(loader, fullname) + if match.group(1) != _loading_owner: + # A system loads all its variable files in turn, so this is a new one. + _loading_owner = match.group(1) + release_dead_variable_modules() path = loader.get_filename(fullname) stat = os.stat(path) key = (path, stat.st_mtime_ns, stat.st_size) @@ -269,7 +284,7 @@ def _get_shared_variable_code(loader, fullname): def pytest_sessionstart(session): - SourceFileLoader.get_code = _get_shared_variable_code + SourceFileLoader.get_code = _get_variable_code def pytest_unconfigure(config): diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index a0300c453..57f6d4ec7 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -118,3 +118,15 @@ def test_no_system_dies_before_the_freeze(request): "Simulation or CountryTaxBenefitSystem at import: build it in a " "fixture or on first use instead." ) + + +def test_dead_systems_modules_are_released_while_a_test_runs(request): + variable_module_owners = _conftest(request)._variable_module_owners + max_owners = 8 + owners = set() + for _ in range(3 * max_owners): + Simulation(situation=SITUATION).calculate("income_tax", 2026) + owners.add(len(variable_module_owners())) + # The release runs as each new system starts loading, and keeps at most + # max_owners systems' modules plus the new system's. + assert max(owners) <= max_owners + 1, sorted(owners) From a1774f0a27dfe8dbd87f775f9b6fe31b0530b4df Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:49:06 -0400 Subject: [PATCH 3/9] Return freed memory after each test file and drop the latest dataset On CI the worker that ran the microsimulation tests kept about 4 GiB of RSS after their simulations were gone, though locally only the 0.74 GiB cached dataset stayed referenced. The conftest now calls glibc's malloc_trim(0) once a file's fixtures are torn down, and the latest-data smoke test removes its unpinned dataset from the simulation cache when its module ends, since no other test uses it. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 21 +++++++++ .../tests/test_latest_data_smoke.py | 7 ++- policyengine_uk/tests/test_malloc_trim.py | 43 +++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 policyengine_uk/tests/test_malloc_trim.py diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 8cffd45d4..5706ee5f6 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1,2 +1,3 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). - Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. +- Hand freed memory back to the system after each test file (glibc malloc_trim), and stop keeping the unpinned latest dataset cached after its smoke test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index c84345ad5..e13e5038b 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -1,4 +1,5 @@ import argparse +import ctypes import gc import hashlib import os @@ -289,3 +290,23 @@ def pytest_sessionstart(session): def pytest_unconfigure(config): SourceFileLoader.get_code = _get_code + + +# Freed memory stays in the worker: on CI the worker that ran the +# microsimulation tests kept about 4 GiB of RSS after their simulations were +# gone (run 38159894687), though locally only the 0.74 GiB cached dataset was +# still referenced once they were deleted (tracemalloc). After each test +# file's fixtures are torn down, glibc's malloc_trim(0) hands free heap memory +# back to the system. Elsewhere this does nothing. +try: + _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim +except (OSError, AttributeError): + _malloc_trim = None + + +@pytest.hookimpl(trylast=True) +def pytest_runtest_teardown(item, nextitem): + # trylast: after pytest's own teardown, which finalizes the file's fixtures + # when the next test is in another file. + if _malloc_trim is not None and (nextitem is None or nextitem.path != item.path): + _malloc_trim(0) diff --git a/policyengine_uk/tests/test_latest_data_smoke.py b/policyengine_uk/tests/test_latest_data_smoke.py index aa9639cab..4e32f537e 100644 --- a/policyengine_uk/tests/test_latest_data_smoke.py +++ b/policyengine_uk/tests/test_latest_data_smoke.py @@ -20,6 +20,7 @@ import pytest from policyengine_uk import Microsimulation +from policyengine_uk.simulation import _url_dataset_cache LATEST_DATASET_URL = ( @@ -39,7 +40,11 @@ def sim() -> Microsimulation: """ with pytest.MonkeyPatch.context() as monkeypatch: monkeypatch.setenv("POLICYENGINE_UK_DEFAULT_DATASET", LATEST_DATASET_URL) - return Microsimulation() + simulation = Microsimulation() + yield simulation + # Only this file uses the unpinned dataset, so don't keep it cached (about + # 0.7 GiB) beside the pinned one for the rest of the test process. + _url_dataset_cache.pop(LATEST_DATASET_URL, None) def _weighted(sim: Microsimulation, variable: str, period: int = YEAR) -> float: diff --git a/policyengine_uk/tests/test_malloc_trim.py b/policyengine_uk/tests/test_malloc_trim.py new file mode 100644 index 000000000..bcd9f8800 --- /dev/null +++ b/policyengine_uk/tests/test_malloc_trim.py @@ -0,0 +1,43 @@ +"""The conftest hands freed memory back to the system after each test file. + +It calls glibc's malloc_trim(0) once a file's last test has been torn down, +and only then. On Linux it must find the function, or the release would +silently do nothing. +""" + +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + + +def _conftest(request): + path = Path(__file__).with_name("conftest.py") + for plugin in request.config.pluginmanager.get_plugins(): + if getattr(plugin, "__file__", None) and Path(plugin.__file__) == path: + return plugin + raise AssertionError(f"{path} is not loaded as a plugin") + + +def test_malloc_trim_is_found_on_linux(request): + if not sys.platform.startswith("linux"): + pytest.skip("malloc_trim is part of glibc") + assert _conftest(request)._malloc_trim is not None + + +@pytest.mark.parametrize( + "next_file, trims", + [("test_a.py", False), ("test_b.py", True), (None, True)], + ids=["same file", "another file", "last test"], +) +def test_trims_only_after_a_files_last_test(request, monkeypatch, next_file, trims): + conftest = _conftest(request) + calls = [] + monkeypatch.setattr(conftest, "_malloc_trim", calls.append) + item = SimpleNamespace(path=Path("test_a.py")) + nextitem = None if next_file is None else SimpleNamespace(path=Path(next_file)) + + conftest.pytest_runtest_teardown(item, nextitem) + + assert calls == ([0] if trims else []) From 5051db8d17968d21ff7f54831cff3c517496e0c2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:36:59 -0400 Subject: [PATCH 4/9] Install the variable code hook in pytest_configure pytest_sessionstart never reaches a conftest that pytest finds only while collecting, as with a bare `pytest` from the repository root, so the hook was not installed there and the sharing and release tests failed. pytest_configure is also called for conftests registered later. The loader is restored in the existing pytest_unconfigure (a second definition would replace it). Also read coverage's tracers with getattr in the test helper. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 12 ++++-------- policyengine_uk/tests/test_variable_code_sharing.py | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index e13e5038b..e6b0a8cf2 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -154,6 +154,9 @@ def pytest_configure(config): _owners_at_start.update(_variable_module_owners()) _threshold_before = gc.get_threshold() gc.set_threshold(GC_FIRST_GENERATION_THRESHOLD, *_threshold_before[1:]) + # Below. pytest_configure, unlike pytest_sessionstart, also reaches this + # conftest when pytest finds it only while collecting (bare `pytest`). + SourceFileLoader.get_code = _get_variable_code @pytest.hookimpl(trylast=True) @@ -184,6 +187,7 @@ def pytest_unconfigure(config): _owners_at_start.clear() if _threshold_before is not None: gc.set_threshold(*_threshold_before) + SourceFileLoader.get_code = _get_code # policyengine-core loads every variable file of each new tax-benefit system as @@ -284,14 +288,6 @@ def _get_variable_code(loader, fullname): return code -def pytest_sessionstart(session): - SourceFileLoader.get_code = _get_variable_code - - -def pytest_unconfigure(config): - SourceFileLoader.get_code = _get_code - - # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py index f6364a3b6..dddb869be 100644 --- a/policyengine_uk/tests/test_variable_code_sharing.py +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -49,7 +49,8 @@ def _coverage_code_objects(): collector = getattr(cov, "_collector", None) if collector is None: return None - lists = [getattr(tracer, "code_objects", None) for tracer in collector.tracers] + tracers = getattr(collector, "tracers", ()) + lists = [getattr(tracer, "code_objects", None) for tracer in tracers] lists = [objects for objects in lists if objects is not None] return sum(len(objects) for objects in lists) if lists else None From 7e96e72c4c5fe1caeca0b966ae7f241c04ce238f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:17:52 -0400 Subject: [PATCH 5/9] Cite the CI measurement of the module release and malloc_trim Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index e6b0a8cf2..7a6930f55 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -263,7 +263,8 @@ def _release_variable_modules_of_dead_systems(): # The same hook also releases dead systems' variable modules whenever a new # system starts loading its variables, not only after each test: in that run # one property test had 165 systems' modules loaded at once, and Hypothesis -# builds hundreds of systems while it shrinks a failing example. +# builds hundreds of systems while it shrinks a failing example. With this the +# shards peaked at 10.0 to 10.4 GiB (run 38165445604). _variable_code = {} _get_code = SourceFileLoader.get_code _loading_owner = None @@ -291,9 +292,12 @@ def _get_variable_code(loader, fullname): # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was -# still referenced once they were deleted (tracemalloc). After each test -# file's fixtures are torn down, glibc's malloc_trim(0) hands free heap memory -# back to the system. Elsewhere this does nothing. +# still referenced once they were deleted (tracemalloc). So after each test +# file's fixtures are torn down, ask glibc to return free heap memory to the +# system (malloc_trim(0)); elsewhere this does nothing. With this, and +# test_latest_data_smoke.py no longer keeping its dataset cached, the shards +# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in no more time, and two +# shards would peak at 8.1 (run 38165445604). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim except (OSError, AttributeError): From 4a88e23a84e6b22b0515bd10d6a0ef0d3ac45ccc Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 16:32:33 -0400 Subject: [PATCH 6/9] Trigger the module release from add_variables_from_file and amortize it Opus review of 7e96e72c4: - The release ran from the SourceFileLoader hook, which policyengine-core#608 bypasses, so it would have stopped without notice. It now runs from TaxBenefitSystem.add_variables_from_file as each new system starts adding its variables. - With more than eight systems alive (an lru_cache of systems in a test module), every new system and every test scanned the heap and released nothing; a worker's later tests took 432 s instead of 219 s. The release now scans only once eight more systems own modules than were alive at the last scan, after tests as well as builds. - The release test assumed at most eight live systems and failed in a serial run; it now measures the live systems first. - Sharing tests skip, with a note, once policyengine-core shares the code; the note names the right function. - The malloc_trim test checks for glibc rather than Linux, malloc_trim gets its C signature, the loader restore leaves no copy on the subclass, the latest-data fixture keeps the cache when the latest URL is the pinned one, and comments say "about the same time" and "can build many more". Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 2 +- policyengine_uk/tests/conftest.py | 94 ++++++++++++++----- .../tests/test_latest_data_smoke.py | 13 ++- policyengine_uk/tests/test_malloc_trim.py | 8 +- .../tests/test_variable_code_sharing.py | 47 +++++++--- .../tests/test_variable_module_release.py | 23 +++-- 6 files changed, 130 insertions(+), 57 deletions(-) diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 5706ee5f6..a84eac6a6 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1,3 +1,3 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). -- Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. +- Release dead tax-benefit systems' variable modules in the Python tests once eight more systems own modules than were alive at the last release, checked as each new system starts adding its variables as well as after each test. - Hand freed memory back to the system after each test file (glibc malloc_trim), and stop keeping the unpinned latest dataset cached after its smoke test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 7a6930f55..2c4d6f20d 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -154,9 +154,10 @@ def pytest_configure(config): _owners_at_start.update(_variable_module_owners()) _threshold_before = gc.get_threshold() gc.set_threshold(GC_FIRST_GENERATION_THRESHOLD, *_threshold_before[1:]) - # Below. pytest_configure, unlike pytest_sessionstart, also reaches this - # conftest when pytest finds it only while collecting (bare `pytest`). - SourceFileLoader.get_code = _get_variable_code + # The variable code and module hooks defined below. pytest_configure, unlike + # pytest_sessionstart, also reaches a conftest that pytest finds only while + # collecting (a bare `pytest` from the repository root). + _install_variable_hooks() @pytest.hookimpl(trylast=True) @@ -187,7 +188,7 @@ def pytest_unconfigure(config): _owners_at_start.clear() if _threshold_before is not None: gc.set_threshold(*_threshold_before) - SourceFileLoader.get_code = _get_code + _remove_variable_hooks() # policyengine-core loads every variable file of each new tax-benefit system as @@ -236,10 +237,28 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: return released +# A test or a new system triggers the release only once ``max_new`` systems +# have been built since the last release found the live ones, so a process +# that keeps more than eight systems alive (an lru_cache of systems in a test +# module) doesn't scan every tracked object after every test and every build. +_live_owners = 0 + + +def release_after_new_systems(max_new: int = 8) -> int: + """Release dead systems' variable modules once ``max_new`` more systems own + modules than were alive at the last release. Returns the number removed.""" + global _live_owners + if len(_variable_module_owners()) <= _live_owners + max_new: + return 0 + released = release_dead_variable_modules(max_owners=0) + _live_owners = len(_variable_module_owners()) + return released + + @pytest.fixture(autouse=True) def _release_variable_modules_of_dead_systems(): yield - release_dead_variable_modules() + release_after_new_systems() # Coverage on CI is measured with sys.monitoring (COVERAGE_CORE=sysmon). That @@ -257,29 +276,17 @@ def _release_variable_modules_of_dead_systems(): # code object. Systems still get their own modules, classes and functions; # only the immutable code is shared. In the same run this lowered the shards' # peaks to 10.2 to 12.2 GiB with no swap, cut their pytest time by 15 to 21%, -# and left every test's outcome unchanged. Remove this once policyengine-core -# shares the code itself (PolicyEngine/policyengine-core#608). -# -# The same hook also releases dead systems' variable modules whenever a new -# system starts loading its variables, not only after each test: in that run -# one property test had 165 systems' modules loaded at once, and Hypothesis -# builds hundreds of systems while it shrinks a failing example. With this the -# shards peaked at 10.0 to 10.4 GiB (run 38165445604). +# and left every test's outcome unchanged. Remove _get_variable_code once +# policyengine-core shares the code itself (PolicyEngine/policyengine-core#608). _variable_code = {} _get_code = SourceFileLoader.get_code -_loading_owner = None +_get_code_was_own = "get_code" in SourceFileLoader.__dict__ def _get_variable_code(loader, fullname): """``SourceFileLoader.get_code``, compiling each variable file only once.""" - global _loading_owner - match = _VARIABLE_MODULE_NAME.match(fullname) - if not match: + if not _VARIABLE_MODULE_NAME.match(fullname): return _get_code(loader, fullname) - if match.group(1) != _loading_owner: - # A system loads all its variable files in turn, so this is a new one. - _loading_owner = match.group(1) - release_dead_variable_modules() path = loader.get_filename(fullname) stat = os.stat(path) key = (path, stat.st_mtime_ns, stat.st_size) @@ -289,17 +296,56 @@ def _get_variable_code(loader, fullname): return code +# Dead systems' variable modules are also released as each new system starts +# adding its variables, not only after each test: in that run one property +# test had 165 systems' modules loaded at once, and Hypothesis can build many +# more while it shrinks a failing example. With this the shards peaked at 10.0 +# to 10.4 GiB (run 38165445604). Keep this until policyengine-core stops +# leaking the modules (PolicyEngine/policyengine-core#520). +_add_variables_from_file = TaxBenefitSystem.add_variables_from_file +_adding_system = None + + +def _add_variables_from_file_releasing(system, file_path): + """``TaxBenefitSystem.add_variables_from_file``, releasing dead systems' + modules when a new system starts adding its variables.""" + global _adding_system + if id(system) != _adding_system: + # A system adds all its variable files in turn, so this is a new one. + _adding_system = id(system) + release_after_new_systems() + return _add_variables_from_file(system, file_path) + + +def _install_variable_hooks(): + SourceFileLoader.get_code = _get_variable_code + TaxBenefitSystem.add_variables_from_file = _add_variables_from_file_releasing + + +def _remove_variable_hooks(): + if _get_code_was_own: + SourceFileLoader.get_code = _get_code + elif "get_code" in SourceFileLoader.__dict__: + # Inherited from SourceLoader: leave no copy on the subclass. + del SourceFileLoader.get_code + TaxBenefitSystem.add_variables_from_file = _add_variables_from_file + + # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was # still referenced once they were deleted (tracemalloc). So after each test # file's fixtures are torn down, ask glibc to return free heap memory to the -# system (malloc_trim(0)); elsewhere this does nothing. With this, and +# system (malloc_trim(0)); memory still awaiting the cycle collector goes back +# at a later file. Elsewhere this does nothing. With this, and # test_latest_data_smoke.py no longer keeping its dataset cached, the shards -# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in no more time, and two -# shards would peak at 8.1 (run 38165445604). +# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in about the same time +# (pytest wall 776 / 952 / 637 s against 788 / 902 / 581), and two shards +# would peak at 8.1 (run 38165445604). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim + _malloc_trim.argtypes = [ctypes.c_size_t] + _malloc_trim.restype = ctypes.c_int except (OSError, AttributeError): _malloc_trim = None diff --git a/policyengine_uk/tests/test_latest_data_smoke.py b/policyengine_uk/tests/test_latest_data_smoke.py index 4e32f537e..51076d019 100644 --- a/policyengine_uk/tests/test_latest_data_smoke.py +++ b/policyengine_uk/tests/test_latest_data_smoke.py @@ -16,6 +16,9 @@ from __future__ import annotations +import os +from collections.abc import Iterator + import numpy as np import pytest @@ -30,7 +33,7 @@ @pytest.fixture(scope="module") -def sim() -> Microsimulation: +def sim() -> Iterator[Microsimulation]: """Simulation built against the unpinned latest dataset. Overrides any pinned-version dataset set in conftest.py so the test @@ -42,9 +45,11 @@ def sim() -> Microsimulation: monkeypatch.setenv("POLICYENGINE_UK_DEFAULT_DATASET", LATEST_DATASET_URL) simulation = Microsimulation() yield simulation - # Only this file uses the unpinned dataset, so don't keep it cached (about - # 0.7 GiB) beside the pinned one for the rest of the test process. - _url_dataset_cache.pop(LATEST_DATASET_URL, None) + # Only this file uses the unpinned dataset, so don't keep it cached beside + # the pinned one for the rest of the test process (a cached copy of the + # pinned 2023-24 dataset held 0.74 GiB, measured locally). + if os.environ.get("POLICYENGINE_UK_DEFAULT_DATASET") != LATEST_DATASET_URL: + _url_dataset_cache.pop(LATEST_DATASET_URL, None) def _weighted(sim: Microsimulation, variable: str, period: int = YEAR) -> float: diff --git a/policyengine_uk/tests/test_malloc_trim.py b/policyengine_uk/tests/test_malloc_trim.py index bcd9f8800..df4d2b2ca 100644 --- a/policyengine_uk/tests/test_malloc_trim.py +++ b/policyengine_uk/tests/test_malloc_trim.py @@ -1,11 +1,11 @@ """The conftest hands freed memory back to the system after each test file. It calls glibc's malloc_trim(0) once a file's last test has been torn down, -and only then. On Linux it must find the function, or the release would +and only then. With glibc it must find the function, or the release would silently do nothing. """ -import sys +import platform from pathlib import Path from types import SimpleNamespace @@ -20,8 +20,8 @@ def _conftest(request): raise AssertionError(f"{path} is not loaded as a plugin") -def test_malloc_trim_is_found_on_linux(request): - if not sys.platform.startswith("linux"): +def test_malloc_trim_is_found_with_glibc(request): + if platform.libc_ver()[0] != "glibc": pytest.skip("malloc_trim is part of glibc") assert _conftest(request)._malloc_trim is not None diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py index dddb869be..7445f4ab9 100644 --- a/policyengine_uk/tests/test_variable_code_sharing.py +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -6,9 +6,9 @@ The conftest makes every system run the code object compiled the first time. Systems must still get their own modules, classes and functions, run the code -of the file's current source, and compute the same results. If -policyengine-core starts sharing the code itself, the upstream check skips -with a note to remove the workaround. +of the file's current source, and compute the same results. Once +policyengine-core shares the code itself (PolicyEngine/policyengine-core#608), +the tests of the workaround skip with a note to remove it. """ import sys @@ -34,6 +34,27 @@ def _conftest(request): raise AssertionError(f"{path} is not loaded as a plugin") +@pytest.fixture(scope="module") +def core_shares_code(request): + """Whether policyengine-core itself gives two systems the same code.""" + from policyengine_uk.tax_benefit_system import CountryTaxBenefitSystem + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + first, second = CountryTaxBenefitSystem(), CountryTaxBenefitSystem() + first = first.variables["income_tax"].get_formula().__code__ + second = second.variables["income_tax"].get_formula().__code__ + return first is second + + +def _skip_if_core_shares(core_shares_code): + if core_shares_code: + pytest.skip( + "policyengine-core now shares variable code between systems; " + "remove _get_variable_code from conftest.py and the tests of it" + ) + + def _income_tax_formula(simulation): variable = simulation.tax_benefit_system.variables["income_tax"] return variable.get_formula() @@ -74,7 +95,8 @@ def test_systems_share_code_but_not_modules_classes_or_functions(): assert first.calculate("income_tax", 2026) == second.calculate("income_tax", 2026) -def test_shared_code_is_compiled_from_the_current_source(request): +def test_shared_code_is_compiled_from_the_current_source(request, core_shares_code): + _skip_if_core_shares(core_shares_code) Simulation(situation=SITUATION) shared = _conftest(request)._variable_code variables_dir = Path(sys.modules["policyengine_uk"].__file__).parent / "variables" @@ -92,19 +114,14 @@ def test_shared_code_is_compiled_from_the_current_source(request): assert {path for path, _, _ in shared} >= files -def test_policyengine_core_still_compiles_each_system_again(request, monkeypatch): - monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) - first = _income_tax_formula(Simulation(situation=SITUATION)) - second = _income_tax_formula(Simulation(situation=SITUATION)) - if first.__code__ is second.__code__: - pytest.skip( - "policyengine-core now shares variable code between systems; " - "remove _get_shared_variable_code from conftest.py and this file" - ) - assert first.__code__ == second.__code__ +def test_policyengine_core_still_compiles_each_system_again(core_shares_code): + _skip_if_core_shares(core_shares_code) -def test_a_simulation_leaves_coverage_no_new_code_objects(request, monkeypatch): +def test_a_simulation_leaves_coverage_no_new_code_objects( + request, monkeypatch, core_shares_code +): + _skip_if_core_shares(core_shares_code) if _coverage_code_objects() is None: pytest.skip("coverage is not measuring with sys.monitoring") Simulation(situation=SITUATION).calculate("income_tax", 2026) diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index 57f6d4ec7..acf96d995 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -120,13 +120,18 @@ def test_no_system_dies_before_the_freeze(request): ) -def test_dead_systems_modules_are_released_while_a_test_runs(request): - variable_module_owners = _conftest(request)._variable_module_owners - max_owners = 8 - owners = set() - for _ in range(3 * max_owners): +def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatch): + conftest = _conftest(request) + max_new = 8 + # Find the systems alive now; other test modules may keep many (caches). + monkeypatch.setattr(conftest, "_live_owners", 0) + conftest.release_after_new_systems(max_new=0) + live = len(conftest._variable_module_owners()) + owners = [] + for _ in range(max_new + 3): Simulation(situation=SITUATION).calculate("income_tax", 2026) - owners.add(len(variable_module_owners())) - # The release runs as each new system starts loading, and keeps at most - # max_owners systems' modules plus the new system's. - assert max(owners) <= max_owners + 1, sorted(owners) + owners.append(len(conftest._variable_module_owners())) + # Released as each new system starts adding its variables, so at most + # max_new dead systems' modules beyond the live ones, plus the newest + # system's. Without the release the last count would be live + 11. + assert max(owners) <= live + max_new + 1, (live, owners) From 591f90fd26b1fa7597881606b0c84edb8047532d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 16:41:43 -0400 Subject: [PATCH 7/9] Test the release's bookkeeping and that it runs inside a test Opus delta review of 4a88e23a8 (approved) left three nits: the comment was off by one (a scan needs more than max_new owners beyond the live ones); the release test could pass if builds reused dead systems' ids, so it now also asserts a release ran; and nothing pinned the amortization, so a Hypothesis test models builds, deaths and releases with faked owner counts and checks that a scan happens exactly when more than max_new owners are beyond the live ones and that at most max_new are left after. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 9 +-- .../tests/test_variable_module_release.py | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 2c4d6f20d..91329219e 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -237,16 +237,17 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: return released -# A test or a new system triggers the release only once ``max_new`` systems -# have been built since the last release found the live ones, so a process +# A test or a new system triggers the release only once more than ``max_new`` +# systems own modules beyond those alive at the last release, so a process # that keeps more than eight systems alive (an lru_cache of systems in a test # module) doesn't scan every tracked object after every test and every build. _live_owners = 0 def release_after_new_systems(max_new: int = 8) -> int: - """Release dead systems' variable modules once ``max_new`` more systems own - modules than were alive at the last release. Returns the number removed.""" + """Release dead systems' variable modules once more than ``max_new`` systems + own modules beyond those alive at the last release. Returns the number + removed.""" global _live_owners if len(_variable_module_owners()) <= _live_owners + max_new: return 0 diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index acf96d995..9af463d13 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -22,6 +22,8 @@ from pathlib import Path import pytest +from hypothesis import given, settings +from hypothesis import strategies as st from policyengine_uk import Simulation @@ -127,6 +129,13 @@ def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatc monkeypatch.setattr(conftest, "_live_owners", 0) conftest.release_after_new_systems(max_new=0) live = len(conftest._variable_module_owners()) + scans = [] + release = conftest.release_dead_variable_modules + monkeypatch.setattr( + conftest, + "release_dead_variable_modules", + lambda **kwargs: scans.append(kwargs) or release(**kwargs), + ) owners = [] for _ in range(max_new + 3): Simulation(situation=SITUATION).calculate("income_tax", 2026) @@ -134,4 +143,58 @@ def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatc # Released as each new system starts adding its variables, so at most # max_new dead systems' modules beyond the live ones, plus the newest # system's. Without the release the last count would be live + 11. + assert scans, "no release ran while the systems were built" assert max(owners) <= live + max_new + 1, (live, owners) + + +# Model of the release's bookkeeping, without building systems: each step +# builds a system that stays alive or dies at once, kills live systems, or +# runs the release. After every release, at most max_new owners are beyond +# those alive at the last scan, and a scan happens exactly when more are. +_steps = st.lists( + st.one_of( + st.tuples(st.just("build"), st.booleans()), + st.tuples(st.just("die"), st.integers(0, 5)), + st.tuples(st.just("release"), st.just(0)), + ), + max_size=60, +) + + +@settings(max_examples=200, deadline=None) +@given(steps=_steps, max_new=st.integers(0, 10)) +def test_release_scans_only_past_max_new_new_owners(request, steps, max_new): + conftest = _conftest(request) + live, dead, scans = 1, 0, [] + + def owners(): + return {str(i): [] for i in range(live + dead)} + + def release_dead(max_owners): + nonlocal dead + released, dead = dead, 0 + scans.append(max_owners) + return released + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr(conftest, "_variable_module_owners", owners) + monkeypatch.setattr(conftest, "release_dead_variable_modules", release_dead) + monkeypatch.setattr(conftest, "_live_owners", 0) + for action, value in steps: + if action == "build": + live, dead = (live + 1, dead) if value else (live, dead + 1) + elif action == "die": + moved = min(value, live) + live, dead = live - moved, dead + moved + else: + last_live = conftest._live_owners + due = live + dead > last_live + max_new + before = len(scans) + released = conftest.release_after_new_systems(max_new=max_new) + assert (len(scans) > before) == due + if due: + assert scans[-1] == 0 and dead == 0 and released >= 0 + assert conftest._live_owners == live + else: + assert released == 0 + assert live + dead <= conftest._live_owners + max_new From 4ee6936edf187d2efb19ffc843970c6064cc474c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 17:41:05 -0400 Subject: [PATCH 8/9] Cite the final CI measurement in conftest.py Run 38173323996, at 591f90fd2 with all of PR #2331's changes, peaked at 7.6 to 8.7 GiB; the release comment no longer attributes run 38165445604's figure to the amortized trigger, which that run did not use. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 91329219e..62b57652a 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -300,9 +300,11 @@ def _get_variable_code(loader, fullname): # Dead systems' variable modules are also released as each new system starts # adding its variables, not only after each test: in that run one property # test had 165 systems' modules loaded at once, and Hypothesis can build many -# more while it shrinks a failing example. With this the shards peaked at 10.0 -# to 10.4 GiB (run 38165445604). Keep this until policyengine-core stops -# leaking the modules (PolicyEngine/policyengine-core#520). +# more while it shrinks a failing example. In run 38165445604 an earlier form +# of this release, which scanned at every new system once more than eight +# systems owned modules, brought the shards' peaks from 10.2-12.2 GiB to +# 10.0-10.4. Keep this until policyengine-core stops leaking the modules +# (PolicyEngine/policyengine-core#520). _add_variables_from_file = TaxBenefitSystem.add_variables_from_file _adding_system = None @@ -342,7 +344,8 @@ def _remove_variable_hooks(): # test_latest_data_smoke.py no longer keeping its dataset cached, the shards # peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in about the same time # (pytest wall 776 / 952 / 637 s against 788 / 902 / 581), and two shards -# would peak at 8.1 (run 38165445604). +# would peak at 8.1 (run 38165445604). With everything in this file the three +# shards peaked at 7.6 to 8.7 GiB (run 38173323996). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim _malloc_trim.argtypes = [ctypes.c_size_t] From 4d472d9c58bfd9b5ea9e1823a5a3d14092fa54a1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 17:43:23 -0400 Subject: [PATCH 9/9] Cite each run for its own figure in the release comment Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 62b57652a..dfab30455 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -300,10 +300,11 @@ def _get_variable_code(loader, fullname): # Dead systems' variable modules are also released as each new system starts # adding its variables, not only after each test: in that run one property # test had 165 systems' modules loaded at once, and Hypothesis can build many -# more while it shrinks a failing example. In run 38165445604 an earlier form -# of this release, which scanned at every new system once more than eight -# systems owned modules, brought the shards' peaks from 10.2-12.2 GiB to -# 10.0-10.4. Keep this until policyengine-core stops leaking the modules +# more while it shrinks a failing example. With an earlier form of this +# release, which scanned at every new system once more than eight systems +# owned modules, the shards peaked at 10.0-10.4 GiB (run 38165445604), against +# 10.2-12.2 with code sharing alone (run 38159894687). Keep this until +# policyengine-core stops leaking the modules # (PolicyEngine/policyengine-core#520). _add_variables_from_file = TaxBenefitSystem.add_variables_from_file _adding_system = None