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

fix ordering dependence of method cache insertion #16084

Merged
merged 1 commit into from
Apr 29, 2016
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: 1 addition & 4 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3829,7 +3829,7 @@ static Function *jl_cfunction_object(jl_function_t *ff, jl_value_t *declrt, jl_t
cfunc_sig = (jl_value_t*)jl_apply_tuple_type((jl_svec_t*)cfunc_sig);

// check the cache
if (jl_cfunction_list.unknown != NULL) {
if (jl_cfunction_list.unknown != jl_nothing) {
jl_typemap_entry_t *sf = jl_typemap_assoc_by_type(jl_cfunction_list, (jl_tupletype_t*)cfunc_sig, NULL, 1, 0, /*offs*/0);
if (sf) {
Function *f = (Function*)jl_unbox_voidpointer(sf->func.value);
Expand All @@ -3839,9 +3839,6 @@ static Function *jl_cfunction_object(jl_function_t *ff, jl_value_t *declrt, jl_t
}
}
}
else {
jl_cfunction_list.unknown = jl_nothing;
}
jl_typemap_entry_t *sf = jl_typemap_insert(&jl_cfunction_list, (jl_value_t*)jl_cfunction_list.unknown, (jl_tupletype_t*)cfunc_sig,
jl_emptysvec, NULL, jl_emptysvec, NULL, /*offs*/0, &cfunction_cache, NULL);

Expand Down
5 changes: 3 additions & 2 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1995,8 +1995,9 @@ static void pre_mark(void)
gc_push_root(jl_an_empty_cell, 0);
if (jl_module_init_order != NULL)
gc_push_root(jl_module_init_order, 0);
if (jl_cfunction_list.unknown != NULL)
gc_push_root(jl_cfunction_list.unknown, 0);
gc_push_root(jl_cfunction_list.unknown, 0);
gc_push_root(jl_anytuple_type_type, 0);
gc_push_root(jl_ANY_flag, 0);

// objects currently being finalized
for(i=0; i < to_finalize.len; i++) {
Expand Down
40 changes: 30 additions & 10 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,13 +368,13 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *ca
int8_t isstaged = definition->isstaged;
int need_guard_entries = 0;
int hasnewparams = 0;
int makesimplesig = 0;
jl_value_t *temp=NULL;
jl_value_t *temp2=NULL;
jl_value_t *temp3=NULL;
jl_lambda_info_t *newmeth=NULL;
jl_svec_t *newparams=NULL;
jl_svec_t *limited=NULL;
int cache_with_orig = 0;
JL_GC_PUSH5(&temp, &temp2, &temp3, &newmeth, &newparams);
size_t np = jl_nparams(type);
newparams = jl_svec_copy(type->parameters);
Expand All @@ -399,25 +399,30 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *ca
// avoid specializing on an argument of type Tuple
// unless matching a declared type of `::Type`
if (jl_is_type_type(elt) && jl_is_tuple_type(jl_tparam0(elt)) &&
(!jl_subtype(decl_i, (jl_value_t*)jl_type_type, 0) || is_kind(decl_i))) {
elt = jl_typeof(jl_tparam0(elt));
(!jl_subtype(decl_i, (jl_value_t*)jl_type_type, 0) || is_kind(decl_i))) { // Type{Tuple{...}}
elt = (jl_value_t*)jl_anytuple_type_type; // Type{T<:Tuple}
jl_svecset(newparams, i, elt);
hasnewparams = 1;
need_guard_entries = 1;
}

int notcalled_func = (i>0 && i<=8 && !(definition->called&(1<<(i-1))) &&
jl_subtype(elt,(jl_value_t*)jl_function_type,0));
if (decl_i == jl_ANY_flag ||
(notcalled_func && (decl_i == (jl_value_t*)jl_any_type ||
decl_i == (jl_value_t*)jl_function_type ||
(jl_is_uniontype(decl_i) && jl_svec_len(((jl_uniontype_t*)decl_i)->types)==2 &&
jl_subtype((jl_value_t*)jl_function_type, decl_i, 0) &&
jl_subtype((jl_value_t*)jl_datatype_type, decl_i, 0))))) {
if (decl_i == jl_ANY_flag) {
// don't specialize on slots marked ANY
jl_svecset(newparams, i, (jl_value_t*)jl_any_type);
hasnewparams = 1;
need_guard_entries = 1;
}
else if (notcalled_func && (decl_i == (jl_value_t*)jl_any_type ||
decl_i == (jl_value_t*)jl_function_type ||
(jl_is_uniontype(decl_i) && jl_svec_len(((jl_uniontype_t*)decl_i)->types)==2 &&
jl_subtype((jl_value_t*)jl_function_type, decl_i, 0) &&
jl_subtype((jl_value_t*)jl_datatype_type, decl_i, 0)))) {
// and attempt to despecialize types marked Function, Callable, or Any
// when called with a subtype of Function but is not called
jl_svecset(newparams, i, (jl_value_t*)jl_any_type);
jl_svecset(newparams, i, (jl_value_t*)jl_function_type);
makesimplesig = 1;
hasnewparams = 1;
need_guard_entries = 1;
}
Expand Down Expand Up @@ -545,6 +550,7 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *ca
need_guard_entries = 1;
}

int cache_with_orig = 0;
jl_svec_t* guardsigs = jl_emptysvec;
if (hasnewparams) {
if (origtype == NULL)
Expand Down Expand Up @@ -609,6 +615,20 @@ static jl_lambda_info_t *cache_method(jl_methtable_t *mt, union jl_typemap_t *ca
// in the method cache to prevent anything else from matching this entry
origtype = NULL;
}
if (makesimplesig && origtype == NULL) {
// reduce the complexity of rejecting this entry in the cache
Copy link
Member

Choose a reason for hiding this comment

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

I'm having trouble visualizing when this case happens. Could you give an example?

Copy link
Member Author

Choose a reason for hiding this comment

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

See the example in the PR. This lets us make an expensive cache entry like (<: Function), while still using the fast-path to reject it (although the (<: Any) it builds isn't a particularly useful example since it only has one argument)

Copy link
Member

Choose a reason for hiding this comment

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

I tried that but I always saw ::Function entries in the cache. I didn't the effect of this replacement with ::Any.

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 ends up in the simplified-signature field. the primary sig field still gets ::Function (which is also what we bother to print)

// by replacing non-simple types with jl_any_type to build origtype
// (the only case this applies to currently due to the above logic is jl_function_type)
size_t np = jl_nparams(type);
newparams = jl_svec_copy(type->parameters);
for (i = 0; i < np; i++) {
jl_value_t *elt = jl_svecref(newparams, i);
if (elt == (jl_value_t*)jl_function_type)
jl_svecset(newparams, i, jl_any_type);
}
origtype = jl_apply_tuple_type(newparams);
temp = origtype;
}

// here we infer types and specialize the method
jl_array_t *lilist=NULL;
Expand Down
7 changes: 7 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jl_datatype_t *jl_typedslot_type;
jl_datatype_t *jl_simplevector_type;
jl_typename_t *jl_tuple_typename;
jl_tupletype_t *jl_anytuple_type;
jl_datatype_t *jl_anytuple_type_type;
jl_datatype_t *jl_ntuple_type;
jl_typename_t *jl_ntuple_typename;
jl_datatype_t *jl_vararg_type;
Expand Down Expand Up @@ -3702,6 +3703,12 @@ void jl_init_types(void)
slot_sym = jl_symbol("slot");
static_parameter_sym = jl_symbol("static_parameter");
compiler_temp_sym = jl_symbol("#temp#");

tttvar = jl_new_typevar(jl_symbol("T"),
(jl_value_t*)jl_bottom_type,
Copy link
Contributor

Choose a reason for hiding this comment

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

funny indent?

(jl_value_t*)jl_anytuple_type);
jl_anytuple_type_type = jl_wrap_Type((jl_value_t*)tttvar);
jl_cfunction_list.unknown = jl_nothing;
}

#ifdef __cplusplus
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ extern JL_DLLEXPORT jl_datatype_t *jl_simplevector_type;
extern JL_DLLEXPORT jl_typename_t *jl_tuple_typename;
extern JL_DLLEXPORT jl_datatype_t *jl_anytuple_type;
#define jl_tuple_type jl_anytuple_type
extern JL_DLLEXPORT jl_datatype_t *jl_anytuple_type_type;
extern JL_DLLEXPORT jl_datatype_t *jl_ntuple_type;
extern JL_DLLEXPORT jl_typename_t *jl_ntuple_typename;
extern JL_DLLEXPORT jl_datatype_t *jl_vararg_type;
Expand Down