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

speed up ambiguous method checking and intersection; helps #21173 #21351

Merged
merged 2 commits into from
Apr 12, 2017
Merged

speed up ambiguous method checking and intersection; helps #21173 #21351

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb1e493
speed up ambiguous method checking and intersection; helps #21173
JeffBezanson Apr 11, 2017
75a6a90
add a fast path function for checking for empty intersections
JeffBezanson Apr 12, 2017
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
28 changes: 19 additions & 9 deletions src/gf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1090,12 +1090,6 @@ void print_func_loc(JL_STREAM *s, jl_method_t *m)
however, (AbstractArray, AbstractMatrix, Foo) and (AbstractMatrix, AbstractArray, Bar) are fine
since Foo and Bar are disjoint, so there would be no confusion over
which one to call.

There is also this kind of ambiguity: foo{T,S}(T, S) vs. foo(Any,Any)
In this case jl_types_equal() is true, but one is jl_type_morespecific
or jl_type_match_morespecific than the other.
To check this, jl_types_equal_generic needs to be more sophisticated
so (T,T) is not equivalent to (Any,Any). (TODO)
*/
struct ambiguous_matches_env {
struct typemap_intersection_env match;
Expand Down Expand Up @@ -1124,8 +1118,23 @@ static int check_ambiguous_visitor(jl_typemap_entry_t *oldentry, struct typemap_
// or !jl_type_morespecific(sig, type) [after]
// based on their sort order in the typemap
// now we are checking that the reverse is true
if (!jl_type_morespecific((jl_value_t*)(closure->after ? type : sig),
(jl_value_t*)(closure->after ? sig : type))) {
int msp;
if (closure->match.issubty) {
assert(closure->after);
msp = 1;
}
else if (closure->after) {
assert(!jl_subtype((jl_value_t*)sig, (jl_value_t*)type));
msp = jl_type_morespecific_no_subtype((jl_value_t*)type, (jl_value_t*)sig);
}
else {
if (jl_subtype((jl_value_t*)sig, (jl_value_t*)type))
msp = 1;
else
msp = jl_type_morespecific_no_subtype((jl_value_t*)sig, (jl_value_t*)type);
}

if (!msp) {
// see if the intersection is covered by another existing method
// that will resolve the ambiguity (by being more specific than either)
// (if type-morespecific made a mistake, this also might end up finding
Expand Down Expand Up @@ -1196,7 +1205,7 @@ static jl_value_t *check_ambiguous_matches(union jl_typemap_t defs, jl_typemap_e
env.match.type = (jl_value_t*)type;
env.match.va = va;
env.match.ti = NULL;
env.match.env = NULL;
env.match.env = jl_emptysvec;
env.defs = defs;
env.newentry = newentry;
env.shadowed = NULL;
Expand Down Expand Up @@ -1418,6 +1427,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
env.match.va = va;
env.match.type = (jl_value_t*)type;
env.match.fptr = invalidate_backedges;
env.match.env = NULL;

if (jl_is_method(oldvalue)) {
jl_typemap_intersection_visitor(((jl_method_t*)oldvalue)->specializations, 0, &env.match);
Expand Down
4 changes: 2 additions & 2 deletions src/julia_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,14 @@ int jl_tuple_isa(jl_value_t **child, size_t cl, jl_datatype_t *pdt);

int jl_has_intersect_type_not_kind(jl_value_t *t);
int jl_subtype_invariant(jl_value_t *a, jl_value_t *b, int ta);
jl_value_t *jl_type_match(jl_value_t *a, jl_value_t *b);
jl_value_t *jl_type_match_morespecific(jl_value_t *a, jl_value_t *b);
jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np);
jl_datatype_t *jl_inst_concrete_tupletype(jl_svec_t *p);
JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method, jl_tupletype_t *simpletype);
void jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_t fptr);
jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t **penv, int *issubty);
jl_value_t *jl_type_intersection_env(jl_value_t *a, jl_value_t *b, jl_svec_t **penv);
// specificity comparison assuming !(a <: b) and !(b <: a)
JL_DLLEXPORT int jl_type_morespecific_no_subtype(jl_value_t *a, jl_value_t *b);
jl_value_t *jl_instantiate_type_with(jl_value_t *t, jl_value_t **env, size_t n);
JL_DLLEXPORT jl_value_t *jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals);
jl_value_t *jl_substitute_var(jl_value_t *t, jl_tvar_t *var, jl_value_t *val);
Expand Down
91 changes: 90 additions & 1 deletion src/subtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,84 @@ static int obviously_unequal(jl_value_t *a, jl_value_t *b)
return 0;
}

static int obviously_disjoint(jl_value_t *a, jl_value_t *b)
{
if (a == b || a == (jl_value_t*)jl_any_type || b == (jl_value_t*)jl_any_type)
return 0;
if (jl_is_leaf_type(a) && !((jl_datatype_t*)a)->abstract &&
jl_is_leaf_type(b) && !((jl_datatype_t*)b)->abstract &&
// TODO: remove these 2 lines if and when Tuple{Union{}} === Union{}
(((jl_datatype_t*)a)->name != jl_tuple_typename ||
((jl_datatype_t*)b)->name != jl_tuple_typename))
return 1;
if (jl_is_unionall(a)) a = jl_unwrap_unionall(a);
if (jl_is_unionall(b)) b = jl_unwrap_unionall(b);
if (jl_is_datatype(a) && jl_is_datatype(b)) {
jl_datatype_t *ad = (jl_datatype_t*)a, *bd = (jl_datatype_t*)b;
if (ad->name != bd->name) {
jl_datatype_t *temp = ad;
while (temp != jl_any_type && temp->name != bd->name)
temp = temp->super;
if (temp == jl_any_type) {
temp = bd;
while (temp != jl_any_type && temp->name != ad->name)
temp = temp->super;
if (temp == jl_any_type)
return 1;
bd = temp;
}
else {
ad = temp;
}
}
int istuple = (ad->name == jl_tuple_typename);
size_t np;
if (istuple) {
size_t na = jl_nparams(ad), nb = jl_nparams(bd);
if (jl_is_va_tuple(ad)) {
na -= 1;
if (jl_is_va_tuple(bd))
nb -= 1;
}
else if (jl_is_va_tuple(bd)) {
nb -= 1;
}
else if (na != nb) {
return 1;
}
np = na < nb ? na : nb;
}
else {
np = jl_nparams(ad);
}
size_t i;
for(i=0; i < np; i++) {
jl_value_t *ai = jl_tparam(ad,i);
jl_value_t *bi = jl_tparam(bd,i);
if (jl_is_typevar(ai) || jl_is_typevar(bi))
continue;
if (jl_is_type(ai)) {
if (jl_is_type(bi)) {
if (istuple && (ai == jl_bottom_type || bi == jl_bottom_type))
; // TODO: this can return 1 if and when Tuple{Union{}} === Union{}
else if (obviously_disjoint(ai, bi))
return 1;
}
else {
return 1;
}
}
else if (!jl_egal(ai, bi)) {
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

what about unions?

Copy link
Member

Choose a reason for hiding this comment

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

oh, I see, you never will recurse into a union?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, so far I just ignore unions here. Turns out to be pretty effective anyway.

}
}
}
else if (a == jl_bottom_type || b == jl_bottom_type) {
return 1;
}
return 0;
}

static int in_union(jl_value_t *u, jl_value_t *x)
{
if (u == x) return 1;
Expand Down Expand Up @@ -1967,6 +2045,8 @@ static jl_value_t *intersect_all(jl_value_t *x, jl_value_t *y, jl_stenv_t *e)
JL_DLLEXPORT jl_value_t *jl_intersect_types(jl_value_t *x, jl_value_t *y)
{
jl_stenv_t e;
if (obviously_disjoint(x, y))
return jl_bottom_type;
init_stenv(&e, NULL, 0);
e.intersection = 1;
return intersect_all(x, y, &e);
Expand All @@ -1992,12 +2072,16 @@ jl_svec_t *jl_outer_unionall_vars(jl_value_t *u)
// sets *issubty to 1 iff `a` is a subtype of `b`
jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t **penv, int *issubty)
{
if (issubty) *issubty = 0;
if (obviously_disjoint(a, b)) {
if (issubty && a == jl_bottom_type) *issubty = 1;
return jl_bottom_type;
}
int szb = jl_subtype_env_size(b);
int sz = 0, i = 0;
jl_value_t **env, **ans;
JL_GC_PUSHARGS(env, szb+1);
ans = &env[szb]; *ans = jl_bottom_type;
if (issubty) *issubty = 0;
if (jl_subtype_env(a, b, env, szb)) {
*ans = a; sz = szb;
if (issubty) *issubty = 1;
Expand Down Expand Up @@ -2491,6 +2575,11 @@ JL_DLLEXPORT int jl_type_morespecific(jl_value_t *a, jl_value_t *b)
return type_morespecific_(a, b, 0, NULL);
}

JL_DLLEXPORT int jl_type_morespecific_no_subtype(jl_value_t *a, jl_value_t *b)
{
return type_morespecific_(a, b, 0, NULL);
}

#ifdef __cplusplus
}
#endif