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.

Sign up for GitHub

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

improve precompilation coverage #33006

Merged
merged 1 commit into from
Aug 26, 2019
Merged

improve precompilation coverage #33006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 10 additions & 15 deletions contrib/generate_precompile.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,6 @@ function generate_precompile_statements()
push!(statements, statement)
end

if have_repl
# Seems like a reasonable number right now, adjust as needed
# comment out if debugging script
@assert length(statements) > 700
end

# Create a staging area where all the loaded packages are available
PrecompileStagingArea = Module()
for (_pkgid, _mod) in Base.loaded_modules
Expand All @@ -157,22 +151,23 @@ function generate_precompile_statements()
end

# Execute the collected precompile statements
n_succeeded = 0
include_time = @elapsed for statement in sort(collect(statements))
# println(statement)
# Workarounds for #28808
occursin("\"YYYY-mm-dd\\THH:MM:SS\"", statement) && continue
statement == "precompile(Tuple{typeof(Base.show), Base.IOContext{Base.TTY}, Type{Vararg{Any, N} where N}})" && continue
# check for `#x##s66` style variable names not in quotes
occursin(r"#\w", statement) &&
count(r"(?:#+\w+)+", statement) !=
count(r"\"(?:#+\w+)+\"", statement) && continue
try
Base.include_string(PrecompileStagingArea, statement)
n_succeeded += 1
catch
@error "Failed to precompile $statement"
rethrow()
# See #28808
# @error "Failed to precompile $statement"
Copy link
Member

Choose a reason for hiding this comment

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

Could be left in at @debug level?

end
end
if have_repl
# Seems like a reasonable number right now, adjust as needed
# comment out if debugging script
@assert n_succeeded > 3500
end

print(" $(length(statements)) generated in ")
tot_time = time() - start_time
Base.time_print(tot_time * 10^9)
Expand Down
38 changes: 32 additions & 6 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,10 @@ const char *name_from_method_instance(jl_method_instance_t *mi)
extern "C"
jl_code_instance_t *jl_compile_linfo(jl_method_instance_t *mi, jl_code_info_t *src, size_t world, const jl_cgparams_t *params)
{
// TODO: Merge with jl_dump_compiles?
static ios_t f_precompile;
static JL_STREAM* s_precompile = NULL;

// N.B.: `src` may have not been rooted by the caller.
JL_TIMING(CODEGEN);
assert(jl_is_method_instance(mi));
Expand Down Expand Up @@ -1266,12 +1270,34 @@ jl_code_instance_t *jl_compile_linfo(jl_method_instance_t *mi, jl_code_info_t *s
// ... unless mi->def isn't defined here meaning the function is a toplevel thunk and
// would have its CodeInfo printed in the stream, which might contain double-quotes that
// would not be properly escaped given the double-quotes added to the stream below.
if (dump_compiles_stream != NULL && jl_is_method(mi->def.method)) {
uint64_t this_time = jl_hrtime();
jl_printf(dump_compiles_stream, "%" PRIu64 "\t\"", this_time - last_time);
jl_static_show(dump_compiles_stream, mi->specTypes);
jl_printf(dump_compiles_stream, "\"\n");
last_time = this_time;
if (jl_is_method(mi->def.method)) {
if (jl_options.trace_compile != NULL) {
if (s_precompile == NULL) {
const char* t = jl_options.trace_compile;
if (!strncmp(t, "stderr", 6))
s_precompile = JL_STDERR;
else {
if (ios_file(&f_precompile, t, 1, 1, 1, 1) == NULL)
jl_errorf("cannot open precompile statement file \"%s\" for writing", t);
s_precompile = (JL_STREAM*) &f_precompile;
}
}
if (!jl_has_free_typevars(mi->specTypes)) {
jl_printf(s_precompile, "precompile(");
jl_static_show(s_precompile, mi->specTypes);
jl_printf(s_precompile, ")\n");

if (s_precompile != JL_STDERR)
ios_flush(&f_precompile);
}
}
if (dump_compiles_stream != NULL) {
uint64_t this_time = jl_hrtime();
jl_printf(dump_compiles_stream, "%" PRIu64 "\t\"", this_time - last_time);
jl_static_show(dump_compiles_stream, mi->specTypes);
jl_printf(dump_compiles_stream, "\"\n");
last_time = this_time;
}
}
JL_GC_POP();
return codeinst;
Expand Down
24 changes: 0 additions & 24 deletions src/gf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,10 +1079,6 @@ static jl_typemap_entry_t *jl_typemap_morespecific_by_type(jl_typemap_entry_t *f

static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype_t *tt, int mt_cache, size_t world)
{
// TODO: Merge with jl_dump_compiles?
static ios_t f_precompile;
static JL_STREAM* s_precompile = NULL;

// caller must hold the mt->writelock
jl_typemap_entry_t *entry = NULL;
entry = jl_typemap_assoc_by_type(mt->cache, (jl_value_t*)tt, NULL, /*subtype*/1, jl_cachearg_offset(mt), world, /*max_world_mask*/0);
Expand All @@ -1099,26 +1095,6 @@ static jl_method_instance_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_datatype
entry = jl_typemap_morespecific_by_type(entry, (jl_value_t*)tt, &env, world);
if (entry != NULL) {
jl_method_t *m = entry->func.method;
if (jl_options.trace_compile != NULL) {
if (s_precompile == NULL) {
const char* t = jl_options.trace_compile;
if (!strncmp(t, "stderr", 6))
s_precompile = JL_STDERR;
else {
if (ios_file(&f_precompile, t, 1, 1, 1, 1) == NULL)
jl_errorf("cannot open precompile statement file \"%s\" for writing", t);
s_precompile = (JL_STREAM*) &f_precompile;
}
}
if (!jl_has_free_typevars((jl_value_t*)tt)) {
jl_printf(s_precompile, "precompile(");
jl_static_show(s_precompile, (jl_value_t*)tt);
jl_printf(s_precompile, ")\n");

if (s_precompile != JL_STDERR)
ios_flush(&f_precompile);
}
}
if (!mt_cache) {
intptr_t nspec = (mt == jl_type_type_mt ? m->nargs + 1 : mt->max_args + 2);
jl_compilation_sig(tt, env, m, nspec, &newparams);
Expand Down
10 changes: 8 additions & 2 deletions stdlib/REPL/src/REPL.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,15 +765,21 @@ setup_interface(
repl::LineEditREPL;
# those keyword arguments may be deprecated eventually in favor of the Options mechanism
hascolor::Bool = repl.options.hascolor,
extra_repl_keymap::Union{Dict,Vector{<:Dict}} = repl.options.extra_keymap
extra_repl_keymap::Any = repl.options.extra_keymap
Copy link
Member

Choose a reason for hiding this comment

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

@nospecialize? Just asserting that it's an Any doesn't prevent the compiler from trying to specialize it. Same for the one below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this takes care of that:

julia/stdlib/REPL/src/REPL.jl

Line 50 in 5e75cfe

@nospecialize # use only declared type signatures

) = setup_interface(repl, hascolor, extra_repl_keymap)

# This non keyword method can be precompiled which is important
function setup_interface(
repl::LineEditREPL,
hascolor::Bool,
extra_repl_keymap::Union{Dict,Vector{<:Dict}},
extra_repl_keymap::Any, # Union{Dict,Vector{<:Dict}},
)
# The precompile statement emitter has problem outputting valid syntax for the
# type of `Union{Dict,Vector{<:Dict}}` (see #28808).
# This function is however important to precompile for REPL startup time, therefore,
# make the type Any and just assert that we have the correct type below.
@assert extra_repl_keymap isa Union{Dict,Vector{<:Dict}}

###
#
# This function returns the main interface that describes the REPL
Expand Down