From f79d8645325928fdff33a06e48026976a4dd611b Mon Sep 17 00:00:00 2001 From: Fatima Anes Date: Tue, 15 Sep 2026 16:16:20 -0700 Subject: [PATCH 1/4] Remove USS sampling from the benchmark memory recorder MemoryInfoRecorder called psutil.Process.memory_full_info() on every update. That call walks the process page tables, so its cost scales with resident size: measured at 71.4 ms per update on an 8 GB process (psutil 5.9.8, Linux 6.8). BenchmarkMonitor drives this recorder once per second from a background thread that is started inside the timed region of all nine benchmark entry points, so the sample perturbed the workload the run was measuring. USS had no consumer. capture.py maps only RSS into the typed bundle, the typed schema has no USS field, the console summary cannot print it, and no in-repo formatter or fixture reads it. RSS and VMS come from memory_info(), a cheap counter read, and are unchanged. Removes the USS state, sampling and the four "System Memory USS*" measurements rather than retaining stale or partial values. Co-Authored-By: Claude Opus 5 (1M context) --- .../benchmark-drop-uss-sampling.rst | 9 ++ .../benchmark/recorders/record_memory_info.py | 53 +------ .../benchmark/test_memory_recorder_no_uss.py | 140 ++++++++++++++++++ .../isaaclab/test/benchmark/test_recorders.py | 74 +++++++-- 4 files changed, 222 insertions(+), 54 deletions(-) create mode 100644 source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst create mode 100644 source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py diff --git a/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst b/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst new file mode 100644 index 000000000000..20c3a5e39d1c --- /dev/null +++ b/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst @@ -0,0 +1,9 @@ +Removed +^^^^^^^ + +* Removed Unique Set Size (USS) collection from :class:`~isaaclab.benchmark.recorders.MemoryInfoRecorder`. + ``psutil.Process.memory_full_info()`` walks the process page tables on every call, and the recorder ran it + once per second from the :class:`~isaaclab.benchmark.BenchmarkMonitor` thread while the benchmark was being + timed, perturbing the workload being measured. The ``System Memory USS``, ``System Memory USS std``, + ``System Memory USS peak`` and ``System Memory USS n`` measurements are no longer emitted. Resident Set Size + and Virtual Memory Size are unchanged and are read from cheap kernel counters. diff --git a/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py b/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py index 969325ff450b..c274dc2ce6e9 100644 --- a/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py +++ b/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py @@ -28,16 +28,10 @@ def __init__(self): self._vms_m2 = 0 self._vms_n = 0 - # Welford's algorithm stats for USS (Unique Set Size) - self._uss_mean = 0 - self._uss_m2 = 0 - self._uss_n = 0 - # Peak (running max) alongside the Welford mean/std. Initialised to # 0.0 so emit-before-record returns a meaningful zero. self._rss_peak = 0.0 self._vms_peak = 0.0 - self._uss_peak = 0.0 # Process handle self._process = psutil.Process(os.getpid()) @@ -84,20 +78,14 @@ def _get_runtime_info(self) -> None: self._memory_runtime_info["vms_n"] = self._vms_n self._memory_runtime_info["vms_peak"] = self._vms_peak - # USS (Unique Set Size) - memory unique to process (not shared) - try: - uss = self._process.memory_full_info().uss - self._uss_mean, self._uss_m2, self._uss_n, uss_std = self._update_welford( - uss, self._uss_mean, self._uss_m2, self._uss_n - ) - self._uss_peak = max(self._uss_peak, float(uss)) - self._memory_runtime_info["uss_mean"] = self._uss_mean - self._memory_runtime_info["uss_std"] = uss_std - self._memory_runtime_info["uss_n"] = self._uss_n - self._memory_runtime_info["uss_peak"] = self._uss_peak - except (psutil.AccessDenied, AttributeError): - # USS may not be available on all platforms - pass + # USS (Unique Set Size) is deliberately not sampled here. + # ``psutil.Process.memory_full_info()`` walks the process page tables on every + # call, which costs hundreds of milliseconds once the process is a few GB + # resident. This recorder is driven by + # :class:`~isaaclab.benchmark.BenchmarkMonitor` from a background thread while + # the benchmark is being timed, so that walk perturbs the very workload the run + # is measuring. ``memory_info()`` above reads cheap kernel counters and is + # unaffected. def update(self) -> None: self._get_runtime_info() @@ -154,31 +142,6 @@ def get_data(self) -> MeasurementData: SingleMeasurement(name="System Memory VMS n", value=self._memory_runtime_info.get("vms_n", 0), unit=""), ] - # USS (Unique Set Size) - only if available - if "uss_mean" in self._memory_runtime_info: - measurements.extend( - [ - SingleMeasurement( - name="System Memory USS", - value=self._bytes_to_gb(self._memory_runtime_info.get("uss_mean", 0)), - unit="GB", - ), - SingleMeasurement( - name="System Memory USS std", - value=self._bytes_to_gb(self._memory_runtime_info.get("uss_std", 0)), - unit="GB", - ), - SingleMeasurement( - name="System Memory USS peak", - value=self._bytes_to_gb(self._memory_runtime_info.get("uss_peak", 0)), - unit="GB", - ), - SingleMeasurement( - name="System Memory USS n", value=self._memory_runtime_info.get("uss_n", 0), unit="" - ), - ] - ) - return MeasurementData( measurements=measurements, metadata=[ diff --git a/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py b/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py new file mode 100644 index 000000000000..d68f45dc352a --- /dev/null +++ b/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py @@ -0,0 +1,140 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Regression tests for the removal of periodic USS sampling from the memory recorder. + +``MemoryInfoRecorder`` used to call ``psutil.Process.memory_full_info()`` once per +:class:`~isaaclab.benchmark.BenchmarkMonitor` tick. That call walks the process page +tables, so it perturbed the workload the benchmark was timing. These tests pin the +behaviour that replaced it: the recorder reads only cheap counters, and no USS value +reaches the emitted output. +""" + +import pathlib +import threading +import time + +import pytest + +from isaaclab.benchmark.benchmark_core import BaseIsaacLabBenchmark +from isaaclab.benchmark.benchmark_monitor import BenchmarkMonitor + +pytestmark = pytest.mark.benchmark + +ENTRYPOINT_DIR = pathlib.Path(__file__).resolve().parents[2] / "isaaclab" / "benchmark" / "entrypoints" + +# The nine call sites that drive a BenchmarkMonitor over a timed region. +ENTRYPOINTS = ( + "runtime.py", + "backends/rl_games/benchmark_train_rl_games.py", + "backends/rl_games/benchmark_play_rl_games.py", + "backends/rsl_rl/benchmark_train_rsl_rl.py", + "backends/rsl_rl/benchmark_play_rsl_rl.py", + "backends/sb3/benchmark_train_sb3.py", + "backends/sb3/benchmark_play_sb3.py", + "backends/skrl/benchmark_train_skrl.py", + "backends/skrl/benchmark_play_skrl.py", +) + + +USS_FIELD_NAMES = ( + "System Memory USS", + "uss_mean", + "uss_std", + "uss_peak", + "uss_n", +) + + +def _uss_fields(text: str) -> list[str]: + """Return the USS field names present in serialized output, if any.""" + return [name for name in USS_FIELD_NAMES if name in text] + + +@pytest.fixture +def no_uss_query(monkeypatch): + """Make any USS query a hard failure for the duration of a test.""" + import psutil + + def _explode(self): # noqa: ARG001 — bound method, self is the process + raise AssertionError("benchmark recorders must not call memory_full_info()") + + monkeypatch.setattr(psutil.Process, "memory_full_info", _explode) + + +def _benchmark(tmp_path, formatter_type="omniperf"): + return BaseIsaacLabBenchmark( + benchmark_name="memory_recorder_regression", + formatter_type=formatter_type, + output_path=str(tmp_path), + use_recorders=True, + output_prefix="test", + ) + + +def test_benchmark_monitor_never_queries_uss(tmp_path, no_uss_query): + """Test that a live BenchmarkMonitor drives the recorders without any USS query.""" + benchmark = _benchmark(tmp_path) + before = threading.active_count() + + with BenchmarkMonitor(benchmark, interval=0.01): + time.sleep(0.2) + + # The monitor swallows recorder exceptions, so assert on the monitor's own record + # rather than trusting that no traceback surfaced. + assert benchmark._manual_recorders["MemoryInfo"]._rss_n >= 1 + assert threading.active_count() == before, "monitor thread outlived its context" + benchmark.finalize() + + +def test_monitor_reports_no_recorder_exception(tmp_path, no_uss_query): + """Test that the monitor loop completes without capturing a recorder exception.""" + benchmark = _benchmark(tmp_path) + monitor = BenchmarkMonitor(benchmark, interval=0.01) + with monitor: + time.sleep(0.1) + assert monitor._exception is None + benchmark.finalize() + + +def test_finalized_output_contains_no_uss(tmp_path, no_uss_query): + """Test that the written benchmark output carries RSS but no USS field.""" + benchmark = _benchmark(tmp_path) + benchmark.update_manual_recorders() + benchmark.finalize() + + written = pathlib.Path(benchmark.output_file_path).read_text() + assert "System Memory RSS" in written + assert not _uss_fields(written) + + +# "schema" is excluded: it requires an attached benchmark bundle, which is out of scope +# here. The typed schema carries no USS field, so this change cannot affect it. +@pytest.mark.parametrize("formatter_type", ["omniperf", "json", "osmo", "summary"]) +def test_supported_formatters_still_serialize(tmp_path, formatter_type, no_uss_query): + """Test that the measurement-driven formatters still serialize with USS gone.""" + benchmark = _benchmark(tmp_path / formatter_type, formatter_type=formatter_type) + benchmark.update_manual_recorders() + + paths = benchmark.finalize() + + assert paths, f"{formatter_type} wrote no output" + for path in paths: + written = pathlib.Path(path) + assert written.exists(), f"{formatter_type} reported {written} but it was not written" + assert not _uss_fields(written.read_text()) + + +@pytest.mark.parametrize("relative_path", ENTRYPOINTS) +def test_entrypoints_keep_monitor_and_recorder_configuration(relative_path): + """Test that each benchmark entry point still runs a monitor with recorders enabled. + + Exercising these end to end needs Isaac Sim, so this is a configuration guard: it + fails if a call site loses its monitor or stops enabling recorders. + """ + source = (ENTRYPOINT_DIR / relative_path).read_text() + assert "BenchmarkMonitor(" in source + assert "use_recorders=True" in source + assert "memory_full_info" not in source diff --git a/source/isaaclab/test/benchmark/test_recorders.py b/source/isaaclab/test/benchmark/test_recorders.py index b99c3d36d296..8f80e68b71ba 100644 --- a/source/isaaclab/test/benchmark/test_recorders.py +++ b/source/isaaclab/test/benchmark/test_recorders.py @@ -383,8 +383,8 @@ def test_initialization(self, recorder): assert recorder._rss_n == 0 assert recorder._vms_mean == 0 assert recorder._vms_n == 0 - assert recorder._uss_mean == 0 - assert recorder._uss_n == 0 + assert not hasattr(recorder, "_uss_mean") + assert not hasattr(recorder, "_uss_n") def test_get_initial_data_structure(self, recorder): """Test that get_initial_data returns correct structure.""" @@ -466,10 +466,9 @@ def test_get_data_returns_measurement_data(self, recorder): data = recorder.get_data() assert isinstance(data, MeasurementData) - # 8 measurements for RSS and VMS (mean, std, peak, n for each) - # Plus potentially 4 more for USS if available (mean, std, peak, n) - assert len(data.measurements) >= 8 - assert len(data.measurements) <= 12 + # Exactly 8: RSS and VMS, each with mean, std, peak and n. USS is no longer + # collected, so the count is platform-independent. + assert len(data.measurements) == 8 assert len(data.metadata) == 1 def test_get_data_measurement_names(self, recorder): @@ -487,8 +486,8 @@ def test_get_data_measurement_names(self, recorder): assert "System Memory VMS" in names assert "System Memory VMS std" in names assert "System Memory VMS n" in names - # USS measurements may be present depending on platform - # We don't assert their presence since they're platform-dependent + # USS is not collected, so no USS row may be emitted on any platform. + assert not [n for n in names if "USS" in n] def test_get_data_metadata_names(self, recorder): """Test that get_data returns metadata with correct names.""" @@ -521,7 +520,6 @@ class _FakeMemInfo: def __init__(self, rss): self.rss = rss self.vms = rss # mirror so VMS also moves - # USS is read via memory_full_info, not memory_info; leave alone. def _fake_memory_info(self): # noqa: ARG001 — bound method, self is the process return _FakeMemInfo(next(scripted_iter)) @@ -568,6 +566,64 @@ def _fake_memory_info(self): # noqa: ARG001 rss_peak = next(m for m in data.measurements if m.name == "System Memory RSS peak") assert rss_peak.value == 300.0 + def test_memory_full_info_is_never_called(self, monkeypatch): + """Test that the recorder never calls psutil's expensive USS query. + + ``memory_full_info()`` walks the process page tables, which is what made this + recorder perturb the benchmark it runs inside. Fail loudly if it comes back. + """ + import psutil + + def _explode(self): # noqa: ARG001 — bound method, self is the process + raise AssertionError("MemoryInfoRecorder must not call memory_full_info()") + + monkeypatch.setattr(psutil.Process, "memory_full_info", _explode) + + rec = MemoryInfoRecorder() + for _ in range(5): + rec.update() + rec.get_runtime_data() + rec.get_data() + + def test_no_uss_keys_in_runtime_data(self, recorder): + """Test that no USS key is emitted, so nothing downstream can read a stale value.""" + for _ in range(3): + recorder.update() + + runtime = recorder.get_runtime_data()["memory_utilization"] + assert not [k for k in runtime if "uss" in k.lower()] + + def test_rss_and_vms_means_are_tracked(self, monkeypatch): + """Test that RSS and VMS means are still computed correctly after the USS removal.""" + import psutil + + rss_values = [1 * 1024**3, 3 * 1024**3] # bytes; mean = 2 GiB + vms_values = [2 * 1024**3, 6 * 1024**3] # bytes; mean = 4 GiB + pairs = iter(zip(rss_values, vms_values)) + + class _FakeMemInfo: + def __init__(self, rss, vms): + self.rss = rss + self.vms = vms + + def _fake_memory_info(self): # noqa: ARG001 — bound method, self is the process + return _FakeMemInfo(*next(pairs)) + + monkeypatch.setattr(psutil.Process, "memory_info", _fake_memory_info) + + rec = MemoryInfoRecorder() + for _ in range(len(rss_values)): + rec.update() + + data = rec.get_data() + by_name = {m.name: m.value for m in data.measurements} + assert by_name["System Memory RSS"] == 2.0 + assert by_name["System Memory VMS"] == 4.0 + assert by_name["System Memory RSS peak"] == 3.0 + assert by_name["System Memory VMS peak"] == 6.0 + assert by_name["System Memory RSS n"] == 2 + assert by_name["System Memory VMS n"] == 2 + # ============================================================================== # VersionInfoRecorder Tests From ce10571e14e08ee315b688865f60f32e8ed295e7 Mon Sep 17 00:00:00 2001 From: fanes <74020209+fatimaanes@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:18:47 -0700 Subject: [PATCH 2/4] Update source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py Co-authored-by: Antoine RICHARD Signed-off-by: fanes <74020209+fatimaanes@users.noreply.github.com> --- .../isaaclab/benchmark/recorders/record_memory_info.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py b/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py index c274dc2ce6e9..737e2c237492 100644 --- a/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py +++ b/source/isaaclab/isaaclab/benchmark/recorders/record_memory_info.py @@ -78,14 +78,7 @@ def _get_runtime_info(self) -> None: self._memory_runtime_info["vms_n"] = self._vms_n self._memory_runtime_info["vms_peak"] = self._vms_peak - # USS (Unique Set Size) is deliberately not sampled here. - # ``psutil.Process.memory_full_info()`` walks the process page tables on every - # call, which costs hundreds of milliseconds once the process is a few GB - # resident. This recorder is driven by - # :class:`~isaaclab.benchmark.BenchmarkMonitor` from a background thread while - # the benchmark is being timed, so that walk perturbs the very workload the run - # is measuring. ``memory_info()`` above reads cheap kernel counters and is - # unaffected. + # USS (Unique Set Size) is deliberately not sampled, it's too expensive. def update(self) -> None: self._get_runtime_info() From 9bd052bc0ab4f9a9f1b2e1784c3c312f013e5490 Mon Sep 17 00:00:00 2001 From: fanes Date: Fri, 2 Oct 2026 00:44:15 -0700 Subject: [PATCH 3/4] Address review: drop the added benchmark recorder tests Remove the regression tests added for the USS removal, per review: * delete source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py * drop the _uss_mean / _uss_n hasattr assertions from test_initialization * drop test_memory_full_info_is_never_called, test_no_uss_keys_in_runtime_data and test_rss_and_vms_means_are_tracked The recorder change and the changelog fragment are unchanged. --- .../benchmark/test_memory_recorder_no_uss.py | 140 ------------------ .../isaaclab/test/benchmark/test_recorders.py | 60 -------- 2 files changed, 200 deletions(-) delete mode 100644 source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py diff --git a/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py b/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py deleted file mode 100644 index d68f45dc352a..000000000000 --- a/source/isaaclab/test/benchmark/test_memory_recorder_no_uss.py +++ /dev/null @@ -1,140 +0,0 @@ -# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). -# All rights reserved. -# -# SPDX-License-Identifier: BSD-3-Clause - -"""Regression tests for the removal of periodic USS sampling from the memory recorder. - -``MemoryInfoRecorder`` used to call ``psutil.Process.memory_full_info()`` once per -:class:`~isaaclab.benchmark.BenchmarkMonitor` tick. That call walks the process page -tables, so it perturbed the workload the benchmark was timing. These tests pin the -behaviour that replaced it: the recorder reads only cheap counters, and no USS value -reaches the emitted output. -""" - -import pathlib -import threading -import time - -import pytest - -from isaaclab.benchmark.benchmark_core import BaseIsaacLabBenchmark -from isaaclab.benchmark.benchmark_monitor import BenchmarkMonitor - -pytestmark = pytest.mark.benchmark - -ENTRYPOINT_DIR = pathlib.Path(__file__).resolve().parents[2] / "isaaclab" / "benchmark" / "entrypoints" - -# The nine call sites that drive a BenchmarkMonitor over a timed region. -ENTRYPOINTS = ( - "runtime.py", - "backends/rl_games/benchmark_train_rl_games.py", - "backends/rl_games/benchmark_play_rl_games.py", - "backends/rsl_rl/benchmark_train_rsl_rl.py", - "backends/rsl_rl/benchmark_play_rsl_rl.py", - "backends/sb3/benchmark_train_sb3.py", - "backends/sb3/benchmark_play_sb3.py", - "backends/skrl/benchmark_train_skrl.py", - "backends/skrl/benchmark_play_skrl.py", -) - - -USS_FIELD_NAMES = ( - "System Memory USS", - "uss_mean", - "uss_std", - "uss_peak", - "uss_n", -) - - -def _uss_fields(text: str) -> list[str]: - """Return the USS field names present in serialized output, if any.""" - return [name for name in USS_FIELD_NAMES if name in text] - - -@pytest.fixture -def no_uss_query(monkeypatch): - """Make any USS query a hard failure for the duration of a test.""" - import psutil - - def _explode(self): # noqa: ARG001 — bound method, self is the process - raise AssertionError("benchmark recorders must not call memory_full_info()") - - monkeypatch.setattr(psutil.Process, "memory_full_info", _explode) - - -def _benchmark(tmp_path, formatter_type="omniperf"): - return BaseIsaacLabBenchmark( - benchmark_name="memory_recorder_regression", - formatter_type=formatter_type, - output_path=str(tmp_path), - use_recorders=True, - output_prefix="test", - ) - - -def test_benchmark_monitor_never_queries_uss(tmp_path, no_uss_query): - """Test that a live BenchmarkMonitor drives the recorders without any USS query.""" - benchmark = _benchmark(tmp_path) - before = threading.active_count() - - with BenchmarkMonitor(benchmark, interval=0.01): - time.sleep(0.2) - - # The monitor swallows recorder exceptions, so assert on the monitor's own record - # rather than trusting that no traceback surfaced. - assert benchmark._manual_recorders["MemoryInfo"]._rss_n >= 1 - assert threading.active_count() == before, "monitor thread outlived its context" - benchmark.finalize() - - -def test_monitor_reports_no_recorder_exception(tmp_path, no_uss_query): - """Test that the monitor loop completes without capturing a recorder exception.""" - benchmark = _benchmark(tmp_path) - monitor = BenchmarkMonitor(benchmark, interval=0.01) - with monitor: - time.sleep(0.1) - assert monitor._exception is None - benchmark.finalize() - - -def test_finalized_output_contains_no_uss(tmp_path, no_uss_query): - """Test that the written benchmark output carries RSS but no USS field.""" - benchmark = _benchmark(tmp_path) - benchmark.update_manual_recorders() - benchmark.finalize() - - written = pathlib.Path(benchmark.output_file_path).read_text() - assert "System Memory RSS" in written - assert not _uss_fields(written) - - -# "schema" is excluded: it requires an attached benchmark bundle, which is out of scope -# here. The typed schema carries no USS field, so this change cannot affect it. -@pytest.mark.parametrize("formatter_type", ["omniperf", "json", "osmo", "summary"]) -def test_supported_formatters_still_serialize(tmp_path, formatter_type, no_uss_query): - """Test that the measurement-driven formatters still serialize with USS gone.""" - benchmark = _benchmark(tmp_path / formatter_type, formatter_type=formatter_type) - benchmark.update_manual_recorders() - - paths = benchmark.finalize() - - assert paths, f"{formatter_type} wrote no output" - for path in paths: - written = pathlib.Path(path) - assert written.exists(), f"{formatter_type} reported {written} but it was not written" - assert not _uss_fields(written.read_text()) - - -@pytest.mark.parametrize("relative_path", ENTRYPOINTS) -def test_entrypoints_keep_monitor_and_recorder_configuration(relative_path): - """Test that each benchmark entry point still runs a monitor with recorders enabled. - - Exercising these end to end needs Isaac Sim, so this is a configuration guard: it - fails if a call site loses its monitor or stops enabling recorders. - """ - source = (ENTRYPOINT_DIR / relative_path).read_text() - assert "BenchmarkMonitor(" in source - assert "use_recorders=True" in source - assert "memory_full_info" not in source diff --git a/source/isaaclab/test/benchmark/test_recorders.py b/source/isaaclab/test/benchmark/test_recorders.py index 8f80e68b71ba..8cb6ceac4eb7 100644 --- a/source/isaaclab/test/benchmark/test_recorders.py +++ b/source/isaaclab/test/benchmark/test_recorders.py @@ -383,8 +383,6 @@ def test_initialization(self, recorder): assert recorder._rss_n == 0 assert recorder._vms_mean == 0 assert recorder._vms_n == 0 - assert not hasattr(recorder, "_uss_mean") - assert not hasattr(recorder, "_uss_n") def test_get_initial_data_structure(self, recorder): """Test that get_initial_data returns correct structure.""" @@ -566,64 +564,6 @@ def _fake_memory_info(self): # noqa: ARG001 rss_peak = next(m for m in data.measurements if m.name == "System Memory RSS peak") assert rss_peak.value == 300.0 - def test_memory_full_info_is_never_called(self, monkeypatch): - """Test that the recorder never calls psutil's expensive USS query. - - ``memory_full_info()`` walks the process page tables, which is what made this - recorder perturb the benchmark it runs inside. Fail loudly if it comes back. - """ - import psutil - - def _explode(self): # noqa: ARG001 — bound method, self is the process - raise AssertionError("MemoryInfoRecorder must not call memory_full_info()") - - monkeypatch.setattr(psutil.Process, "memory_full_info", _explode) - - rec = MemoryInfoRecorder() - for _ in range(5): - rec.update() - rec.get_runtime_data() - rec.get_data() - - def test_no_uss_keys_in_runtime_data(self, recorder): - """Test that no USS key is emitted, so nothing downstream can read a stale value.""" - for _ in range(3): - recorder.update() - - runtime = recorder.get_runtime_data()["memory_utilization"] - assert not [k for k in runtime if "uss" in k.lower()] - - def test_rss_and_vms_means_are_tracked(self, monkeypatch): - """Test that RSS and VMS means are still computed correctly after the USS removal.""" - import psutil - - rss_values = [1 * 1024**3, 3 * 1024**3] # bytes; mean = 2 GiB - vms_values = [2 * 1024**3, 6 * 1024**3] # bytes; mean = 4 GiB - pairs = iter(zip(rss_values, vms_values)) - - class _FakeMemInfo: - def __init__(self, rss, vms): - self.rss = rss - self.vms = vms - - def _fake_memory_info(self): # noqa: ARG001 — bound method, self is the process - return _FakeMemInfo(*next(pairs)) - - monkeypatch.setattr(psutil.Process, "memory_info", _fake_memory_info) - - rec = MemoryInfoRecorder() - for _ in range(len(rss_values)): - rec.update() - - data = rec.get_data() - by_name = {m.name: m.value for m in data.measurements} - assert by_name["System Memory RSS"] == 2.0 - assert by_name["System Memory VMS"] == 4.0 - assert by_name["System Memory RSS peak"] == 3.0 - assert by_name["System Memory VMS peak"] == 6.0 - assert by_name["System Memory RSS n"] == 2 - assert by_name["System Memory VMS n"] == 2 - # ============================================================================== # VersionInfoRecorder Tests From cd9eb662172f56d41b67f6f2930eff73c1e7a947 Mon Sep 17 00:00:00 2001 From: fanes Date: Fri, 2 Oct 2026 01:06:31 -0700 Subject: [PATCH 4/4] Rename the changelog fragment to the ..rst format The changelog gate expects the entry type in the file name and a body of bullets, so move benchmark-drop-uss-sampling.rst to benchmark-drop-uss-sampling.removed.rst and drop the now-redundant Removed section heading. The entry text is unchanged. --- ...ss-sampling.rst => benchmark-drop-uss-sampling.removed.rst} | 3 --- 1 file changed, 3 deletions(-) rename source/isaaclab/changelog.d/{benchmark-drop-uss-sampling.rst => benchmark-drop-uss-sampling.removed.rst} (97%) diff --git a/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst b/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.removed.rst similarity index 97% rename from source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst rename to source/isaaclab/changelog.d/benchmark-drop-uss-sampling.removed.rst index 20c3a5e39d1c..5494e4558995 100644 --- a/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.rst +++ b/source/isaaclab/changelog.d/benchmark-drop-uss-sampling.removed.rst @@ -1,6 +1,3 @@ -Removed -^^^^^^^ - * Removed Unique Set Size (USS) collection from :class:`~isaaclab.benchmark.recorders.MemoryInfoRecorder`. ``psutil.Process.memory_full_info()`` walks the process page tables on every call, and the recorder ran it once per second from the :class:`~isaaclab.benchmark.BenchmarkMonitor` thread while the benchmark was being