Skip to content

Commit

Permalink
Skip some stock GC-specific tests when using MMTk (#57104)
Browse files Browse the repository at this point in the history
As noted in #57103, there are a
few tests that are specific to the stock GC and will fail for MMTk. For
now, this PR simply skips those tests when not using the stock GC until
we agree on how to approach the incompatibilities described in the
issue.
  • Loading branch information
udesou authored Jan 24, 2025
1 parent bcddc65 commit 899d2f5
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 33 deletions.
2 changes: 2 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ include("sysinfo.jl")
include("libc.jl")
using .Libc: getpid, gethostname, time, memcpy, memset, memmove, memcmp

const USING_STOCK_GC = occursin("stock", GC.gc_active_impl())

# These used to be in build_h.jl and are retained for backwards compatibility.
# NOTE: keep in sync with `libblastrampoline_jll.libblastrampoline`.
const libblas_name = "libblastrampoline" * (Sys.iswindows() ? "-5" : "")
Expand Down
11 changes: 11 additions & 0 deletions base/gcutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,15 @@ function logging_enabled()
ccall(:jl_is_gc_logging_enabled, Cint, ()) != 0
end

"""
GC.gc_active_impl()
Return a string stating which GC implementation is being used and possibly
its version according to the list of supported GCs
"""
function gc_active_impl()
unsafe_string(ccall(:jl_gc_active_impl, Ptr{UInt8}, ()))
end


end # module GC
6 changes: 2 additions & 4 deletions base/timing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,8 @@ function gc_page_utilization_data()
return Base.unsafe_wrap(Array, page_utilization_raw, JL_GC_N_MAX_POOLS, own=false)
end


const USING_STOCK_GC = occursin("stock", unsafe_string(ccall(:jl_gc_active_impl, Ptr{UInt8}, ())))
# Full sweep reasons are currently only available for the stock GC
@static if USING_STOCK_GC
@static if Base.USING_STOCK_GC
# must be kept in sync with `src/gc-stock.h``
const FULL_SWEEP_REASONS = [:FULL_SWEEP_REASON_SWEEP_ALWAYS_FULL, :FULL_SWEEP_REASON_FORCED_FULL_SWEEP,
:FULL_SWEEP_REASON_USER_MAX_EXCEEDED, :FULL_SWEEP_REASON_LARGE_PROMOTION_RATE]
Expand All @@ -135,7 +133,7 @@ function full_sweep_reasons()
d = Dict{Symbol, Int64}()
# populate the dictionary according to the reasons above for the stock GC
# otherwise return an empty dictionary for now
@static if USING_STOCK_GC
@static if Base.USING_STOCK_GC
reason = cglobal(:jl_full_sweep_reasons, UInt64)
reasons_as_array = Base.unsafe_wrap(Vector{UInt64}, reason, length(FULL_SWEEP_REASONS), own=false)
for (i, r) in enumerate(FULL_SWEEP_REASONS)
Expand Down
13 changes: 11 additions & 2 deletions stdlib/Profile/test/allocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,14 @@ end
@test length(first_alloc.stacktrace) > 0
@test length(string(first_alloc.type)) > 0

@testset for type in (Task, Vector{Float64},)
@test length(filter(a->a.type <: type, profile.allocs)) >= NUM_TASKS
# Issue #57103: This test does not work with MMTk because of fastpath
# allocation which never calls the allocation profiler.
# TODO: We should port these observability tools (e.g. allocation
# profiler and heap snapshot) to MMTk
@static if Base.USING_STOCK_GC
@testset for type in (Task, Vector{Float64},)
@test length(filter(a->a.type <: type, profile.allocs)) >= NUM_TASKS
end
end

# TODO: it would be nice to assert that these tasks
Expand Down Expand Up @@ -143,6 +149,8 @@ end
@test length([a for a in prof.allocs if a.type == String]) >= 1
end

# FIXME: Issue #57103 disabling test for MMTk.
@static if Base.USING_STOCK_GC
@testset "alloc profiler catches allocs from codegen" begin
@eval begin
struct MyType x::Int; y::Int end
Expand All @@ -162,6 +170,7 @@ end
@test length(prof.allocs) >= 1
@test length([a for a in prof.allocs if a.type == MyType]) >= 1
end
end

@testset "alloc profiler catches allocs from buffer resize" begin
f(a) = for _ in 1:100; push!(a, 1); end
Expand Down
3 changes: 3 additions & 0 deletions stdlib/Profile/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@ end
@test only(node.down).first == lidict[8]
end

# FIXME: Issue #57103: heap snapshots are currently not supported in MMTk
@static if Base.USING_STOCK_GC
@testset "HeapSnapshot" begin
tmpdir = mktempdir()

Expand Down Expand Up @@ -374,6 +376,7 @@ end
rm(fname)
rm(tmpdir, force = true, recursive = true)
end
end

@testset "PageProfile" begin
fname = "$(getpid())_$(time_ns())"
Expand Down
8 changes: 6 additions & 2 deletions test/checked.jl
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,12 @@ end
@test checked_pow(BigInt(2), 2) == BigInt(4)
@test checked_pow(BigInt(2), 100) == BigInt(1267650600228229401496703205376)

# Perf test: Make sure BigInts allocs don't scale with the power:
@test @allocations(checked_pow(BigInt(2), 2)) @allocations(checked_pow(BigInt(2), 10000)) rtol=0.9
# FIXME: Issue #57103: the following test may fail because
# allocation may not be logged via MMTk's fastpath allocation
@static if Base.USING_STOCK_GC
# Perf test: Make sure BigInts allocs don't scale with the power:
@test @allocations(checked_pow(BigInt(2), 2)) @allocations(checked_pow(BigInt(2), 10000)) rtol=0.9
end
end

@testset "Additional tests" begin
Expand Down
49 changes: 29 additions & 20 deletions test/cmdlineargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -383,29 +383,33 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
@test p.exitcode == 1 && p.termsignal == 0
end

# --gcthreads
code = "print(Threads.ngcthreads())"
cpu_threads = ccall(:jl_effective_threads, Int32, ())
@test string(cpu_threads) ==
read(`$exename --threads auto -e $code`, String) ==
read(`$exename --threads=auto -e $code`, String) ==
read(`$exename -tauto -e $code`, String) ==
read(`$exename -t auto -e $code`, String)
for nt in (nothing, "1")
withenv("JULIA_NUM_GC_THREADS" => nt) do
@test read(`$exename --gcthreads=2 -e $code`, String) == "2"
end
withenv("JULIA_NUM_GC_THREADS" => nt) do
@test read(`$exename --gcthreads=2,1 -e $code`, String) == "3"
# FIXME: Issue #57103 --gcthreads does not have the same semantics
# for Stock GC and MMTk, so the tests below are specific to the Stock GC
@static if Base.USING_STOCK_GC
# --gcthreads
code = "print(Threads.ngcthreads())"
cpu_threads = ccall(:jl_effective_threads, Int32, ())
@test string(cpu_threads) ==
read(`$exename --threads auto -e $code`, String) ==
read(`$exename --threads=auto -e $code`, String) ==
read(`$exename -tauto -e $code`, String) ==
read(`$exename -t auto -e $code`, String)
for nt in (nothing, "1")
withenv("JULIA_NUM_GC_THREADS" => nt) do
@test read(`$exename --gcthreads=2 -e $code`, String) == "2"
end
withenv("JULIA_NUM_GC_THREADS" => nt) do
@test read(`$exename --gcthreads=2,1 -e $code`, String) == "3"
end
end
end

withenv("JULIA_NUM_GC_THREADS" => 2) do
@test read(`$exename -e $code`, String) == "2"
end
withenv("JULIA_NUM_GC_THREADS" => 2) do
@test read(`$exename -e $code`, String) == "2"
end

withenv("JULIA_NUM_GC_THREADS" => "2,1") do
@test read(`$exename -e $code`, String) == "3"
withenv("JULIA_NUM_GC_THREADS" => "2,1") do
@test read(`$exename -e $code`, String) == "3"
end
end

# --machine-file
Expand Down Expand Up @@ -1182,6 +1186,10 @@ end
end
end

# FIXME: Issue #57103: MMTK currently does not use --heap-size-hint since it only
# supports setting up a hard limit unlike the Stock GC
# which takes it as a soft limit. For now, we skip the tests below for MMTk
@static if Base.USING_STOCK_GC
@testset "heap size hint" begin
#heap-size-hint, we reserve 250 MB for non GC memory (llvm, etc.)
@test readchomp(`$(Base.julia_cmd()) --startup-file=no --heap-size-hint=500M -e "println(@ccall jl_gc_get_max_memory()::UInt64)"`) == "$((500-250)*1024*1024)"
Expand All @@ -1201,6 +1209,7 @@ end

@test readchomp(`$(Base.julia_cmd()) --startup-file=no --heap-size-hint=10M -e "println(@ccall jl_gc_get_max_memory()::UInt64)"`) == "$(1*1024*1024)"
end
end

## `Main.main` entrypoint

Expand Down
12 changes: 8 additions & 4 deletions test/gc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ end
run_gctest("gc/chunks.jl")
end

#FIXME: Issue #57103 disabling tests for MMTk, since
# they rely on information that is specific to the stock GC.
@static if Base.USING_STOCK_GC
@testset "GC page metrics" begin
run_nonzero_page_utilization_test()
run_pg_size_test()
Expand All @@ -76,13 +79,14 @@ end
issue_54275_test()
end

@testset "Base.GC docstrings" begin
@test isempty(Docs.undocumented_names(GC))
end

@testset "Full GC reasons" begin
full_sweep_reasons_test()
end
end

@testset "Base.GC docstrings" begin
@test isempty(Docs.undocumented_names(GC))
end

#testset doesn't work here because this needs to run in top level
#Check that we ensure objects in toplevel exprs are rooted
Expand Down
4 changes: 3 additions & 1 deletion test/misc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1453,7 +1453,8 @@ end
@test_throws ErrorException finalizer(x->nothing, 1)
@test_throws ErrorException finalizer(C_NULL, 1)


# FIXME: Issue #57103 Test is specific to Stock GC
@static if Base.USING_STOCK_GC
@testset "GC utilities" begin
GC.gc()
GC.gc(true); GC.gc(false)
Expand All @@ -1473,6 +1474,7 @@ end
@test occursin("GC: pause", read(tmppath, String))
end
end
end

@testset "fieldtypes Module" begin
@test fieldtypes(Module) === ()
Expand Down

0 comments on commit 899d2f5

Please sign in to comment.