Skip to content

Release the array behind a type annotated global - #100

Open
davidanthoff wants to merge 1 commit into
mainfrom
fix/release-typed-globals
Open

Release the array behind a type annotated global#100
davidanthoff wants to merge 1 commit into
mainfrom
fix/release-typed-globals

Conversation

@davidanthoff

Copy link
Copy Markdown
Member

Ports julia-testitems/TestItemRunner.jl#145TestItemServer.release_module_globals! was
byte-identical to the pre-fix TestItemRunner version, docstring included.

release_module_globals! sets every global of a test item's module to nothing when the
item finishes, because Julia cannot unload the module. A global declared with a type does
not accept nothing:

@testitem "" begin
    buffer::Vector{UInt8} = zeros(UInt8, 100_000_000)
    @test length(buffer) == 100_000_000
end

The assignment throws on convert, the bare catch swallows it, and the array stays
reachable. This matters more here than in TestItemRunner: test processes are pooled, so the
array is held not just for the rest of that run but for every later run on that worker.
Neither existing mitigation helps — gc_between_testitems is off by default for a
single-process run and GC.gc(true) cannot free what a live module global points at, and
memory_threshold is off by default and recycles the whole process when it does fire.

When the binding holds an array — the case where the memory is worth having back — the
teardown now falls back to assigning an empty array of the same type. Everything else is
skipped as before, and nothing needs a VERSION guard: isa Array, similar with a Dims
tuple and ntuple all work on Julia 1.0, which the test process still supports.

Measured on the identical function in TestItemRunner, ten test items binding 100 MB each
behind such a global:

before after
Julia 1.12.7 417 MB → 1303 MB flat
Julia 1.13.0-rc3 344 MB → 1203 MB flat

The docstring also had the const case backwards — it said a const is only skipped
"on Julia before 1.12". In fact from 1.12 on a const cannot be released at all: the
assignment is rejected, and redeclaring with const frees nothing either, because the
previous Core.BindingPartition goes on holding the old value (verified with
gc_live_bytes). Before 1.12 the plain assignment goes through for some values, with a
WARNING: redefinition of constant, and throws for others.

testdata/MemoryPackage probed a single plain global — the case that already worked — so it
now probes all three binding forms, keeping the two-item / two-run shape that
test/test_memory_release.jl drives. That test fails with the extended fixture and the old
teardown, and passes with this change.

Full suite green on the host Julia: 1143/1143 in 18m55s.

🤖 Generated with Claude Code

`release_module_globals!` nulls every global of a test item's module once the item
finishes, so that whatever the item bound can be collected — Julia cannot unload a
module, and test processes are pooled, so anything still reachable is held for every
later item and every later run on that worker. A global declared with a type,
`x::Vector{UInt8} = …`, rejects `nothing`: the assignment throws on `convert`, the bare
`catch` swallows it, and the array stays reachable. When such a binding holds an array an
empty array of the same type is assigned instead, which the declared type does accept.

Measured on the identical function in TestItemRunner, ten test items binding 100 MB each
behind such a global took the run from 417 MB to 1303 MB on Julia 1.12 and from 344 MB to
1203 MB on 1.13; it stays flat now.

The docstring also had the `const` case backwards. From Julia 1.12 on a `const` cannot be
released at all, because the previous `Core.BindingPartition` goes on holding the old
value even after a `const` redeclaration; before 1.12 the plain assignment goes through
for some values and throws for others.

`testdata/MemoryPackage` probed only a plain global, which is the case that already
worked, so it now probes all three binding forms.

Ported from julia-testitems/TestItemRunner.jl#145.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant