Skip to content

Commit

Permalink
Don't perform extra inference during incremental image creation (#48054)
Browse files Browse the repository at this point in the history
As noted in #48047, we're currently attempting to infer extra
methods during incremental image saving, which causes us to miss
edges in the image. In particular, in the case of #48047, Cthulhu
had the `compile=min` option set, which caused the code instance
for `do_typeinf!` to not be infered. However, later it was
nevertheless queued for precompilation, causing inference to
occur at an inopportune time.

This PR simply prevents code instances that don't explicitly have
the `->precompile` flag set (e.g. the guard instance created for
the interpreter) from being enqueued for precompilation. It is
not clear that this is necessarily the correct behavior - we may
in fact want to infer these method instances, just before we set
up the serializer state, but for now this fixes #48047 for me.

I also included an appropriate test and a warning message if
we attempt to enter inference when this is not legal, so any
revisit of what should be happening here can hopefully make
use of those.

(cherry picked from commit 80aeebe)
  • Loading branch information
Keno authored and KristofferC committed Jan 10, 2023
1 parent 46cd080 commit a064027
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 2 deletions.
4 changes: 4 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ for m in methods(include)
delete_method(m)
end

# This method is here only to be overwritten during the test suite to test
# various sysimg related invalidation scenarios.
a_method_to_overwrite_in_test() = inferencebarrier(1)

# These functions are duplicated in client.jl/include(::String) for
# nicer stacktraces. Modifications here have to be backported there
include(mod::Module, _path::AbstractString) = _include(identity, mod, _path)
Expand Down
7 changes: 7 additions & 0 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@ jl_code_info_t *jl_type_infer(jl_method_instance_t *mi, size_t world, int force)
if (jl_typeinf_func == NULL)
return NULL;
jl_task_t *ct = jl_current_task;
if (ct->reentrant_inference == (uint16_t)-1) {
// TODO: We should avoid attempting to re-inter inference here at all
// and turn on this warning, but that requires further refactoring
// of the precompile code, so for now just catch that case here.
//jl_printf(JL_STDERR, "ERROR: Attempted to enter inference while writing out image.");
return NULL;
}
if (ct->reentrant_inference > 2)
return NULL;

Expand Down
11 changes: 9 additions & 2 deletions src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -2620,7 +2620,12 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
} else {
checksumpos_ff = checksumpos;
}
jl_gc_enable_finalizers(ct, 0); // make sure we don't run any Julia code concurrently after this point
{
// make sure we don't run any Julia code concurrently after this point
jl_gc_enable_finalizers(ct, 0);
assert(ct->reentrant_inference == 0);
ct->reentrant_inference = (uint16_t)-1;
}
jl_prepare_serialization_data(mod_array, newly_inferred, jl_worklist_key(worklist), &extext_methods, &new_specializations, &method_roots_list, &ext_targets, &edges);

// Generate _native_data`
Expand All @@ -2644,7 +2649,9 @@ JL_DLLEXPORT void jl_create_system_image(void **_native_data, jl_array_t *workli
jl_save_system_image_to_stream(ff, worklist, extext_methods, new_specializations, method_roots_list, ext_targets, edges);
native_functions = NULL;
if (worklist) {
jl_gc_enable_finalizers(ct, 1); // make sure we don't run any Julia code concurrently before this point
// Re-enable running julia code for postoutput hooks, atexit, etc.
jl_gc_enable_finalizers(ct, 1);
ct->reentrant_inference = 0;
jl_precompile_toplevel_module = NULL;
}

Expand Down
17 changes: 17 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1674,6 +1674,23 @@ precompile_test_harness("DynamicExpressions") do load_path
end
end

precompile_test_harness("BadInvalidations") do load_path
write(joinpath(load_path, "BadInvalidations.jl"),
"""
module BadInvalidations
Base.Experimental.@compiler_options compile=min optimize=1
getval() = Base.a_method_to_overwrite_in_test()
getval()
end # module BadInvalidations
""")
Base.compilecache(Base.PkgId("BadInvalidations"))
(@eval Base a_method_to_overwrite_in_test() = inferencebarrier(2))
(@eval (using BadInvalidations))
Base.invokelatest() do
@test BadInvalidations.getval() === 2
end
end

empty!(Base.DEPOT_PATH)
append!(Base.DEPOT_PATH, original_depot_path)
empty!(Base.LOAD_PATH)
Expand Down

0 comments on commit a064027

Please sign in to comment.