From 67526796ff941366cb54a623d06c1a6c6f912474 Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Thu, 24 Sep 2026 13:01:37 -0700 Subject: [PATCH] Measure memory_threshold against the test process's own RSS The memory recycle check compared whole-system memory use, 1 - Sys.free_memory()/Sys.total_memory(), against the threshold. On macOS free memory stays near zero even on an idle machine (0.07-0.19 GB of 7 GB on GitHub arm64 runners), so any threshold recycled the test process after every item, and a process was also blamed for memory other processes used. The threshold is now the fraction of total system memory that one test process's current resident memory may reach. `_current_rss()` reads it from /proc/self/statm on Linux, task_info(MACH_TASK_BASIC_INFO) on macOS and K32GetProcessMemoryInfo on Windows, using plain ccall so it works on every Julia version the test server supports (1.0 on). When the RSS cannot be determined the check never fires. A threshold of 0.0 still recycles after every item. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + shared/testserver_protocol.jl | 5 +- src/state.jl | 2 + src/testitemcontroller.jl | 4 + test/test_worker_lifecycle.jl | 38 ++++++++- .../TestItemServer/src/TestItemServer.jl | 22 +++-- .../TestItemServer/src/process_memory.jl | 84 +++++++++++++++++++ 7 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 testprocess/TestItemServer/src/process_memory.jl diff --git a/CHANGELOG.md b/CHANGELOG.md index 32d7fab..b8812fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- The experimental `memory_threshold` option of `execute_testrun` now measures each test process's own resident memory (RSS; the working set on Windows) as a fraction of total system memory, instead of whole-system memory use. On macOS `Sys.free_memory()` stays near zero even on an idle machine, so any threshold made every test process recycle after every test item; the old measure also recycled a process for memory other processes were using. A threshold tuned for the old meaning will now fire much later; `0.0` still recycles after every item. - Failures a test process can carry on past are now reported instead of being swallowed. The process was given a single error handler, and that handler always ends in `exit(1)`, so anything not worth dying for had to be discarded — which is why clearing coverage data between items had a bare `catch` with a `# TODO Call global error handler` in it, and why losing a setup's captured output showed up as an empty string. `TestItemServer.serve` now also accepts a non-fatal handler, used by those sites and by the watchdog. In the same vein, a test process that dies before it can connect no longer discards the output it produced while starting — the usual explanation for why it died — and the two internal-consistency assertions in the controller now attach an exception so their crash reports carry a backtrace pointing at the code that broke the invariant rather than at the logger. - **Breaking (JSONRPC):** every test item notification — `testItemStarted`, `testItemPassed`, `testItemFailed`, `testItemErrored`, `testItemSkipped` and `appendOutput` — now carries `testEnvId` alongside `testItemId`, and clients must identify an item by the pair. A test item id is scoped to its package, so the same package checked out into two folders mints the same id from both; a client keying on the id alone collapses them, reporting one item's results twice while the other never resolves. The in-process callbacks have always received `test_env_id` and `TestRunState.test_items` is already keyed this way — it was only the JSONRPC layer that dropped it. - Load setup modules via `using` by default ([2b187ad1](https://github.com/julia-vscode/TestItemControllers.jl/commit/2b187ad1)) diff --git a/shared/testserver_protocol.jl b/shared/testserver_protocol.jl index 27d141d..bf9420c 100644 --- a/shared/testserver_protocol.jl +++ b/shared/testserver_protocol.jl @@ -93,8 +93,9 @@ end testSetups::Union{Missing,Vector{TestsetupDetails}} # Run a full `GC.gc()` after each test item. gcBetweenTestitems::Union{Missing,Bool} - # Fraction of system memory (0..1) above which the test process exits cleanly after - # finishing an item, so the controller can recycle it. + # Fraction of total system memory (0..1) that the test process's own resident memory + # may reach; above it the process exits cleanly after finishing an item, so the + # controller can recycle it. memoryThreshold::Union{Missing,Float64} end diff --git a/src/state.jl b/src/state.jl index 89dbe61..3a61bf1 100644 --- a/src/state.jl +++ b/src/state.jl @@ -127,6 +127,8 @@ mutable struct TestRunState # `gc_between_testitems` is resolved from the caller's request in `execute_testrun` # (default: on whenever the run uses more than one process). gc_between_testitems::Bool + # Fraction of total system memory (0..1) that one test process's resident memory may + # reach before the process is recycled after its current item; `nothing` disables it. memory_threshold::Union{Nothing,Float64} # Stop the run at the first failing or errored work unit. Decided on the reactor rather # than by a consumer reacting to a callback: items are handed to a worker as a batch, so diff --git a/src/testitemcontroller.jl b/src/testitemcontroller.jl index 877e3a4..2dfb71f 100644 --- a/src/testitemcontroller.jl +++ b/src/testitemcontroller.jl @@ -3141,6 +3141,10 @@ environments use `"Coverage"` mode) or `nothing`. # Keyword arguments - `coverage_root_uris` — if set, only collect coverage for files under these URI prefixes. +- `memory_threshold` — experimental. A fraction (0..1) of total system memory: once a test + process's own resident memory is above it, the process exits after finishing its current + test item and the controller hands its remaining items to other processes. `nothing` (the + default) disables the check. - `failfast` — stop the run as soon as a work unit fails or errors. The remaining work is reported as skipped and the run finishes normally, exactly as an outside cancellation would. The decision is taken on the reactor, in the same step that records the failure, so diff --git a/test/test_worker_lifecycle.jl b/test/test_worker_lifecycle.jl index a680247..4505ac2 100644 --- a/test/test_worker_lifecycle.jl +++ b/test/test_worker_lifecycle.jl @@ -87,7 +87,7 @@ end end @testitem "A test process over the memory threshold is recycled without losing items" setup=[TestHelpers] begin - # The worker checks system memory after each item and, when over threshold, exits + # The worker checks its resident memory after each item and, when over threshold, exits # cleanly with a distinguished code instead of being killed. The controller has to treat # that as a recycle — redistributing un-started items through the existing termination # path — rather than as a crash. @@ -143,6 +143,42 @@ end @test isempty(filter(e -> e.event in (:failed, :errored), result.events)) end +# `_current_rss` lives in the test process, which the controller test suite never loads. +# The file only needs Base, so include it directly, as test_cpu_target.jl does. +@testmodule ProcessMemoryImpl begin + include(joinpath(@__DIR__, "..", "testprocess", "TestItemServer", "src", "process_memory.jl")) +end + +@testitem "_current_rss reports a plausible resident size" setup=[ProcessMemoryImpl] begin + # CI runs this on Linux, macOS and Windows, so it exercises each of the three OS + # implementations. `nothing` would mean the check silently never fires on that + # platform. The upper bound catches reading the wrong field: the virtual size of a + # Julia process is far larger than physical memory on every platform. + rss = ProcessMemoryImpl._current_rss() + @test rss isa Int + @test 0 < rss < Sys.total_memory() +end + +@testitem "A memory threshold of 1.0 never recycles the test process" setup=[TestHelpers] begin + # The threshold compares this process's own resident memory against total system + # memory, so 1.0 cannot be crossed: the counterpart of the 0.0 tests above, pinning + # that a set threshold does not recycle by itself. + pkg_path = joinpath(TestHelpers.TESTDATA_DIR, "BasicPackage") + discovered = TestHelpers.discover_test_items(pkg_path) + + items = filter(i -> i.label in ("add works", "greet works", "output test"), discovered.items) + @test length(items) == 3 + + result = TestHelpers.run_testrun( + items, discovered.setups, discovered; + memory_threshold=1.0, max_procs=1, timeout=300, + ) + + @test length(filter(e -> e.event == :passed, result.events)) == 3 + @test isempty(filter(e -> e.event in (:failed, :errored), result.events)) + @test length(filter(e -> e.event == :process_created, result.process_events)) == 1 +end + @testitem "Shutdown stops within its grace period when a process never reports termination" setup=[TestHelpers] begin # The reactor leaves `ControllerShuttingDown` only when every tracked process has posted # a `TestProcessTerminatedMsg`. A process whose IO task has wedged never posts one, and diff --git a/testprocess/TestItemServer/src/TestItemServer.jl b/testprocess/TestItemServer/src/TestItemServer.jl index 7c8ff9a..d53cb55 100644 --- a/testprocess/TestItemServer/src/TestItemServer.jl +++ b/testprocess/TestItemServer/src/TestItemServer.jl @@ -44,9 +44,10 @@ include("scratch_env.jl") include("cpu_target_precompile.jl") include("error_location.jl") include("watchdog.jl") +include("process_memory.jl") -# Exit code the test process uses when it stops itself between test items because system -# memory crossed `memoryThreshold`. The controller recognises it and redistributes the +# Exit code the test process uses when it stops itself between test items because its +# resident memory crossed `memoryThreshold`. The controller recognises it and redistributes the # process's un-started items instead of reporting a crash — see `_handle_termination_during_run!`. const MEMORY_RECYCLE_EXIT_CODE = 66 @@ -77,8 +78,9 @@ mutable struct TestProcessState # Run a full `GC.gc()` after every test item. Defaulted by the controller, which turns # it on whenever a run has more than one test process. gc_between_testitems::Bool - # Fraction of system memory (0..1) above which we stop after the current item so the - # controller can recycle us. `nothing` disables the check. + # Fraction of total system memory (0..1) that this process's resident memory may reach + # before we stop after the current item so the controller can recycle us. `nothing` + # disables the check. memory_threshold::Union{Nothing,Float64} testitems_channel::Channel{Vector{TestItemServerProtocol.RunTestItem}} @@ -1434,15 +1436,17 @@ JSONRPC.@message_dispatcher dispatch_msg begin TestItemServerProtocol.testserver_shutdown_request_type => shutdown_request end -# Fraction of system memory currently in use, or `nothing` when the OS numbers are not -# available. Deliberately a whole-system figure rather than this process's RSS: what makes -# recycling worth doing is total pressure on the machine, and several test processes share it. +# Whether this process's current resident memory is above `threshold` as a fraction of +# total system memory. This used to measure whole-system use, but macOS keeps +# `Sys.free_memory()` near zero even when idle, so any threshold fired after every item. function _memory_over_threshold(threshold::Union{Nothing,Float64}) threshold === nothing && return false return try + rss = _current_rss() + rss === nothing && return false total = Sys.total_memory() total == 0 && return false - (1 - Sys.free_memory() / total) > threshold + rss / total > threshold catch err false end @@ -1532,7 +1536,7 @@ function runner_loop(state::TestProcessState) end if _memory_over_threshold(state.memory_threshold) - @info "Stopping this test process: system memory use is above the configured threshold of $(state.memory_threshold). The controller will redistribute the remaining test items." + @info "Stopping this test process: its resident memory is above the configured threshold of $(state.memory_threshold) of system memory. The controller will redistribute the remaining test items." flush(stderr) flush(stdout) # Must precede `exit`: it tears the runtime down from this thread, and diff --git a/testprocess/TestItemServer/src/process_memory.jl b/testprocess/TestItemServer/src/process_memory.jl new file mode 100644 index 0000000..2b17d4a --- /dev/null +++ b/testprocess/TestItemServer/src/process_memory.jl @@ -0,0 +1,84 @@ +# Resident memory of the current process, for the `memoryThreshold` recycle check. +# +# Base has `Sys.maxrss()`, but that is the *peak* resident size, which never goes down, so +# a process that once touched a lot of memory would be recycled after every item from then +# on. This asks the OS for the current figure instead. The test server runs on every Julia +# version from 1.0 on, so this sticks to plain `ccall` and Base APIs that 1.0 has. Only +# Base is used, so the controller's test suite can include this file directly. + +# `struct mach_task_basic_info` from . The header wraps it in +# `#pragma pack(push, 4)`, but every field already sits at an offset that is a multiple +# of its size, so the layout is the same as Julia's: 48 bytes, `resident_size` at 8. +struct _MachTimeValue + seconds::Int32 + microseconds::Int32 +end + +struct _MachTaskBasicInfo + virtual_size::UInt64 + resident_size::UInt64 + resident_size_max::UInt64 + user_time::_MachTimeValue + system_time::_MachTimeValue + policy::Int32 + suspend_count::Int32 +end + +const _MACH_TASK_BASIC_INFO = UInt32(20) +# `MACH_TASK_BASIC_INFO_COUNT`: the struct's size in `natural_t` (32-bit) units, i.e. 12. +const _MACH_TASK_BASIC_INFO_COUNT = UInt32(div(sizeof(_MachTaskBasicInfo), sizeof(UInt32))) + +# `PROCESS_MEMORY_COUNTERS` from . `SIZE_T` is `Csize_t`, so this is also right +# in a 32-bit Julia. +struct _ProcessMemoryCounters + cb::UInt32 + PageFaultCount::UInt32 + PeakWorkingSetSize::Csize_t + WorkingSetSize::Csize_t + QuotaPeakPagedPoolUsage::Csize_t + QuotaPagedPoolUsage::Csize_t + QuotaPeakNonPagedPoolUsage::Csize_t + QuotaNonPagedPoolUsage::Csize_t + PagefileUsage::Csize_t + PeakPagefileUsage::Csize_t +end + +""" + _current_rss() -> Union{Int,Nothing} + +The current resident set size of this process in bytes (the working set on Windows), or +`nothing` when it cannot be determined on this platform. +""" +function _current_rss() + try + if Sys.islinux() + # Second field of /proc/self/statm: resident pages. + fields = split(read("/proc/self/statm", String)) + pages = parse(Int, fields[2]) + return pages * Int(ccall(:getpagesize, Cint, ())) + elseif Sys.isapple() + # `mach_task_self()` is a macro for the global `mach_task_self_`. + task = unsafe_load(cglobal(:mach_task_self_, UInt32)) + info = Ref(_MachTaskBasicInfo(0, 0, 0, _MachTimeValue(0, 0), _MachTimeValue(0, 0), 0, 0)) + count = Ref(_MACH_TASK_BASIC_INFO_COUNT) + kr = ccall(:task_info, Cint, + (UInt32, UInt32, Ptr{_MachTaskBasicInfo}, Ptr{UInt32}), + task, _MACH_TASK_BASIC_INFO, info, count) + kr == 0 || return nothing + return Int(info[].resident_size) + elseif Sys.iswindows() + z = Csize_t(0) + counters = Ref(_ProcessMemoryCounters(UInt32(sizeof(_ProcessMemoryCounters)), 0, z, z, z, z, z, z, z, z)) + handle = ccall((:GetCurrentProcess, "kernel32"), stdcall, Ptr{Cvoid}, ()) + ok = ccall((:K32GetProcessMemoryInfo, "kernel32"), stdcall, Cint, + (Ptr{Cvoid}, Ptr{_ProcessMemoryCounters}, UInt32), + handle, counters, UInt32(sizeof(_ProcessMemoryCounters))) + ok == 0 && return nothing + return Int(counters[].WorkingSetSize) + else + return nothing + end + catch + return nothing + end +end