Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pkgimages for coverage & malloc tracking by ignoring native code in tracked packages #52123

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ Language changes
allows users to safely tear down background state (such as closing Timers and sending
disconnect notifications to heartbeat tasks) and cleanup other resources when the program
wants to begin exiting.
* Code coverage and malloc tracking is no longer generated during the package precompilation stage.
Further, during these modes pkgimage caches are now used for packages that are not being tracked.
Meaning that coverage testing (the default for `julia-actions/julia-runtest`) will by default use
pkgimage caches for all other packages than the package being tested, likely meaning faster test
execution. ([#52123])

Compiler/Runtime improvements
-----------------------------
Expand Down
52 changes: 48 additions & 4 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,21 @@ const TIMING_IMPORTS = Threads.Atomic{Int}(0)
# these return either the array of modules loaded from the path / content given
# or an Exception that describes why it couldn't be loaded
# and it reconnects the Base.Docs.META
function _include_from_serialized(pkg::PkgId, path::String, ocachepath::Union{Nothing, String}, depmods::Vector{Any})
function _include_from_serialized(pkg::PkgId, path::String, ocachepath::Union{Nothing, String}, depmods::Vector{Any}, ignore_native::Union{Nothing,Bool}=nothing)
if isnothing(ignore_native)
if JLOptions().code_coverage == 0 && JLOptions().malloc_log == 0
ignore_native = false
else
io = open(path, "r")
try
iszero(isvalid_cache_header(io)) && return ArgumentError("Invalid header in cache file $path.")
_, (includes, _, _), _, _, _, _, _, _ = parse_cache_header(io, path)
ignore_native = pkg_tracked(includes)
finally
close(io)
end
end
end
assert_havelock(require_lock)
timing_imports = TIMING_IMPORTS[] > 0
try
Expand All @@ -1056,7 +1070,7 @@ function _include_from_serialized(pkg::PkgId, path::String, ocachepath::Union{No

if ocachepath !== nothing
@debug "Loading object cache file $ocachepath for $pkg"
sv = ccall(:jl_restore_package_image_from_file, Any, (Cstring, Any, Cint, Cstring), ocachepath, depmods, false, pkg.name)
sv = ccall(:jl_restore_package_image_from_file, Any, (Cstring, Any, Cint, Cstring, Cint), ocachepath, depmods, false, pkg.name, ignore_native)
else
@debug "Loading cache file $path for $pkg"
sv = ccall(:jl_restore_incremental, Any, (Cstring, Any, Cint, Cstring), path, depmods, false, pkg.name)
Expand Down Expand Up @@ -1495,15 +1509,45 @@ function _tryrequire_from_serialized(modkey::PkgId, path::String, ocachepath::Un
return loaded
end

# returns whether the package is tracked in coverage or malloc tracking based on
# JLOptions and includes
function pkg_tracked(includes)
if JLOptions().code_coverage == 0 && JLOptions().malloc_log == 0
return false
elseif JLOptions().code_coverage == 1 || JLOptions().malloc_log == 1 # user
# Just say true. Pkgimages aren't in Base
return true
elseif JLOptions().code_coverage == 2 || JLOptions().malloc_log == 2 # all
return true
elseif JLOptions().code_coverage == 3 || JLOptions().malloc_log == 3 # tracked path
if JLOptions().tracked_path == C_NULL
return false
else
tracked_path = unsafe_string(JLOptions().tracked_path)
if isempty(tracked_path)
return false
else
return any(includes) do inc
startswith(inc.filename, tracked_path)
end
end
end
end
end

# loads a precompile cache file, ignoring stale_cachefile tests
# load the best available (non-stale) version of all dependent modules first
function _tryrequire_from_serialized(pkg::PkgId, path::String, ocachepath::Union{Nothing, String})
assert_havelock(require_lock)
local depmodnames
io = open(path, "r")
ignore_native = false
try
iszero(isvalid_cache_header(io)) && return ArgumentError("Invalid header in cache file $path.")
_, _, depmodnames, _, _, _, clone_targets, _ = parse_cache_header(io, path)
_, (includes, _, _), depmodnames, _, _, _, clone_targets, _ = parse_cache_header(io, path)

ignore_native = pkg_tracked(includes)

pkgimage = !isempty(clone_targets)
if pkgimage
ocachepath !== nothing || return ArgumentError("Expected ocachepath to be provided")
Expand All @@ -1529,7 +1573,7 @@ function _tryrequire_from_serialized(pkg::PkgId, path::String, ocachepath::Union
depmods[i] = dep
end
# then load the file
return _include_from_serialized(pkg, path, ocachepath, depmods)
return _include_from_serialized(pkg, path, ocachepath, depmods, ignore_native)
end

# returns `nothing` if require found a precompile cache for this sourcepath, but couldn't load it
Expand Down
3 changes: 0 additions & 3 deletions base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,6 @@ function julia_cmd(julia=joinpath(Sys.BINDIR, julia_exename()); cpu_target::Unio
end
if opts.use_pkgimages == 0
push!(addflags, "--pkgimages=no")
else
# If pkgimage is set, malloc_log and code_coverage should not
@assert opts.malloc_log == 0 && opts.code_coverage == 0
Comment on lines -245 to -247
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should check that malloc_log/code_coverage don't reach the generating-ouput process?

end
return `$julia -C$cpu_target -J$image_file $addflags`
end
Expand Down
2 changes: 0 additions & 2 deletions pkgimage.mk
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,10 @@ $$(BUILDDIR)/stdlib/$1.debug.image: export JULIA_CPU_TARGET=$(JULIA_CPU_TARGET)

$$(BUILDDIR)/stdlib/$1.release.image: $$($1_SRCS) $$(addsuffix .release.image,$$(addprefix $$(BUILDDIR)/stdlib/,$2)) $(build_private_libdir)/sys.$(SHLIB_EXT)
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no --check-bounds=yes -e 'Base.compilecache(Base.identify_package("$1"))')
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no --pkgimages=no -e 'Base.compilecache(Base.identify_package("$1"))')
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no -e 'Base.compilecache(Base.identify_package("$1"))')
touch $$@
$$(BUILDDIR)/stdlib/$1.debug.image: $$($1_SRCS) $$(addsuffix .debug.image,$$(addprefix $$(BUILDDIR)/stdlib/,$2)) $(build_private_libdir)/sys-debug.$(SHLIB_EXT)
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no --check-bounds=yes -e 'Base.compilecache(Base.identify_package("$1"))')
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no --pkgimages=no -e 'Base.compilecache(Base.identify_package("$1"))')
@$$(call PRINT_JULIA, $$(call spawn,$$(JULIA_EXECUTABLE)) --startup-file=no -e 'Base.compilecache(Base.identify_package("$1"))')
touch $$@
else
Expand Down
12 changes: 8 additions & 4 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8450,15 +8450,19 @@ static jl_llvm_functions_t
cursor = -1;
};

// If a pkgimage or sysimage is being generated, disable tracking.
// This means sysimage build or pkgimage precompilation workloads aren't tracked.
auto do_coverage = [&] (bool in_user_code, bool is_tracked) {
return (coverage_mode == JL_LOG_ALL ||
return (jl_generating_output() == 0 &&
(coverage_mode == JL_LOG_ALL ||
(in_user_code && coverage_mode == JL_LOG_USER) ||
(is_tracked && coverage_mode == JL_LOG_PATH));
(is_tracked && coverage_mode == JL_LOG_PATH)));
};
auto do_malloc_log = [&] (bool in_user_code, bool is_tracked) {
return (malloc_log_mode == JL_LOG_ALL ||
return (jl_generating_output() == 0 &&
(malloc_log_mode == JL_LOG_ALL ||
(in_user_code && malloc_log_mode == JL_LOG_USER) ||
(is_tracked && malloc_log_mode == JL_LOG_PATH));
(is_tracked && malloc_log_mode == JL_LOG_PATH)));
Comment on lines +8453 to +8465
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to avoid baking tracking-enabled native code into pkgimages, which is likely undesirable (though arguably reasonable in CI environments)

Copy link
Member Author

@IanButterworth IanButterworth Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it could be allowed if --check-bounds=yes is also forced, as it is by Pkg.test, given that is unlikely in non-test sessions.

And in that case we wouldn't need to zero out the fptrs from the pkgimage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @Keno has an idea on how to handle --check-bounds=yes.

IIUC the problem is that --check-bounds doesn't just change the LLVM code, but actually the behaviour of inference and co. See #50239 (comment)

So right now I don't see away around us needing at least those two copies.

};
SmallVector<unsigned, 0> current_lineinfo, new_lineinfo;
auto coverageVisitStmt = [&] (size_t dbg) {
Expand Down
5 changes: 0 additions & 5 deletions src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -814,11 +814,6 @@ JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)
#endif
#endif

if ((jl_options.outputo || jl_options.outputbc || jl_options.outputasm) &&
(jl_options.code_coverage || jl_options.malloc_log)) {
jl_error("cannot generate code-coverage or track allocation information while generating a .o, .bc, or .s output file");
}

jl_init_rand();
jl_init_runtime_ccall();
jl_init_tasks();
Expand Down
9 changes: 0 additions & 9 deletions src/jloptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
const char **cmds = NULL;
int codecov = JL_LOG_NONE;
int malloclog = JL_LOG_NONE;
int pkgimage_explicit = 0;
int argc = *argcp;
char **argv = *argvp;
char *endptr;
Expand Down Expand Up @@ -469,7 +468,6 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
jl_errorf("julia: invalid argument to --compiled-modules={yes|no|existing} (%s)", optarg);
break;
case opt_pkgimages:
pkgimage_explicit = 1;
if (!strcmp(optarg,"yes"))
jl_options.use_pkgimages = JL_OPTIONS_USE_PKGIMAGES_YES;
else if (!strcmp(optarg,"no"))
Expand Down Expand Up @@ -860,13 +858,6 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
"This is a bug, please report it.", c);
}
}
if (codecov || malloclog) {
if (pkgimage_explicit && jl_options.use_pkgimages) {
jl_errorf("julia: Can't use --pkgimages=yes together "
"with --track-allocation or --code-coverage.");
}
jl_options.use_pkgimages = 0;
}
jl_options.code_coverage = codecov;
jl_options.malloc_log = malloclog;
int proc_args = *argcp < optind ? *argcp : optind;
Expand Down
6 changes: 5 additions & 1 deletion src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -3675,7 +3675,7 @@ JL_DLLEXPORT void jl_restore_system_image_data(const char *buf, size_t len)
JL_SIGATOMIC_END();
}

JL_DLLEXPORT jl_value_t *jl_restore_package_image_from_file(const char *fname, jl_array_t *depmods, int completeinfo, const char *pkgname)
JL_DLLEXPORT jl_value_t *jl_restore_package_image_from_file(const char *fname, jl_array_t *depmods, int completeinfo, const char *pkgname, int ignore_native)
{
void *pkgimg_handle = jl_dlopen(fname, JL_RTLD_LAZY);
if (!pkgimg_handle) {
Expand All @@ -3696,6 +3696,10 @@ JL_DLLEXPORT jl_value_t *jl_restore_package_image_from_file(const char *fname, j

jl_image_t pkgimage = jl_init_processor_pkgimg(pkgimg_handle);

if (ignore_native){
memset(&pkgimage.fptrs, 0, sizeof(pkgimage.fptrs));
}
IanButterworth marked this conversation as resolved.
Show resolved Hide resolved

jl_value_t* mod = jl_restore_incremental_from_buf(pkgimg_handle, pkgimg_data, &pkgimage, *plen, depmods, completeinfo, pkgname, false);

return mod;
Expand Down
12 changes: 12 additions & 0 deletions stdlib/Test/docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,3 +491,15 @@ Using `Test.jl`, more complicated tests can be added for packages but this shoul
```@meta
DocTestSetup = nothing
```

### Code Coverage

Code coverage tracking during tests can be enabled using the `pkg> test --coverage` flag (or at a lower level using the
[`--code-coverage`](@ref command-line-interface) julia arg). This is on by default in the
[julia-runtest](https://github.com/julia-actions/julia-runtest) GitHub action.

To evaluate coverage either manually inspect the `.cov` files that are generated beside the source files locally,
or in CI use the [julia-processcoverage](https://github.com/julia-actions/julia-processcoverage) GitHub action.

!!! compat "Julia 1.11"
Since Julia 1.11, coverage is not collected during the package precompilation phase.
13 changes: 12 additions & 1 deletion test/cmdlineargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,19 @@ end

@testset "julia_cmd" begin
julia_basic = Base.julia_cmd()
function get_julia_cmd(arg)
io = Base.BufferStream()
cmd = `$julia_basic $arg -e 'print(repr(Base.julia_cmd()))'`
try
run(pipeline(cmd, stdout=io, stderr=io))
catch
@error "cmd failed" cmd read(io, String)
rethrow()
end
return read(io, String)
end

opts = Base.JLOptions()
get_julia_cmd(arg) = strip(read(`$julia_basic $arg -e 'print(repr(Base.julia_cmd()))'`, String), ['`'])

for (arg, default) in (
("-C$(unsafe_string(opts.cpu_target))", false),
Expand Down
44 changes: 44 additions & 0 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1333,3 +1333,47 @@ end
@test filesize(cache_path) != cache_size
end
end

@testset "code coverage disabled during precompilation" begin
mktempdir() do depot
cov_test_dir = joinpath(@__DIR__, "project", "deps", "CovTest.jl")
cov_cache_dir = joinpath(depot, "compiled", "v$(VERSION.major).$(VERSION.minor)", "CovTest")
function rm_cov_files()
for cov_file in filter(endswith(".cov"), readdir(joinpath(cov_test_dir, "src"), join=true))
rm(cov_file)
end
@test !cov_exists()
end
cov_exists() = !isempty(filter(endswith(".cov"), readdir(joinpath(cov_test_dir, "src"))))

rm_cov_files() # clear out any coverage files first
@test !cov_exists()

cd(cov_test_dir) do
# In our depot, precompile CovTest.jl with coverage on
@test success(addenv(
`$(Base.julia_cmd()) --startup-file=no --pkgimage=yes --code-coverage=@ --project -e 'using CovTest; exit(0)'`,
"JULIA_DEPOT_PATH" => depot,
))
@test !isempty(filter(!endswith(".ji"), readdir(cov_cache_dir))) # check that object cache file(s) exists
@test !cov_exists()
rm_cov_files()

# same again but call foo(), which is in the pkgimage, and should generate coverage
@test success(addenv(
`$(Base.julia_cmd()) --startup-file=no --pkgimage=yes --code-coverage=@ --project -e 'using CovTest; foo(); exit(0)'`,
"JULIA_DEPOT_PATH" => depot,
))
@test cov_exists()
rm_cov_files()

# same again but call bar(), which is NOT in the pkgimage, and should generate coverage
@test success(addenv(
`$(Base.julia_cmd()) --startup-file=no --pkgimage=yes --code-coverage=@ --project -e 'using CovTest; bar(); exit(0)'`,
"JULIA_DEPOT_PATH" => depot,
))
@test cov_exists()
rm_cov_files()
end
end
end
2 changes: 1 addition & 1 deletion test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,7 @@ precompile_test_harness("PkgCacheInspector") do load_path
end

if ocachefile !== nothing
sv = ccall(:jl_restore_package_image_from_file, Any, (Cstring, Any, Cint, Cstring), ocachefile, depmods, true, "PCI")
sv = ccall(:jl_restore_package_image_from_file, Any, (Cstring, Any, Cint, Cstring, Cint), ocachefile, depmods, true, "PCI", false)
else
sv = ccall(:jl_restore_incremental, Any, (Cstring, Any, Cint, Cstring), cachefile, depmods, true, "PCI")
end
Expand Down
3 changes: 3 additions & 0 deletions test/project/deps/CovTest.jl/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name = "CovTest"
uuid = "f1f4390d-b815-473a-b5dd-5af6e1d717cb"
version = "0.1.0"
26 changes: 26 additions & 0 deletions test/project/deps/CovTest.jl/src/CovTest.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

module CovTest

function foo()
x = 1
y = 2
z = x * y
return z
end

function bar()
x = 1
y = 2
z = x * y
return z
end

if Base.generating_output()
# precompile foo but not bar
foo()
end

export foo, bar

end #module