From bb1e493bd87989bb513d9d425d72ad6e3983e2c2 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Tue, 11 Apr 2017 15:00:06 -0400 Subject: [PATCH 1/2] speed up ambiguous method checking and intersection; helps #21173 --- src/gf.c | 28 +++++++++----- src/julia_internal.h | 4 +- src/subtype.c | 91 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 111 insertions(+), 12 deletions(-) diff --git a/src/gf.c b/src/gf.c index de023ba2163ea..c6df93465cf1a 100644 --- a/src/gf.c +++ b/src/gf.c @@ -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; @@ -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 @@ -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; @@ -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); diff --git a/src/julia_internal.h b/src/julia_internal.h index 8eb3842a8a9be..e40cd82eea32c 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -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); diff --git a/src/subtype.c b/src/subtype.c index eeb5d4376f64d..8a1984c7a285f 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -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; + } + } + } + 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; @@ -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); @@ -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; @@ -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 From 75a6a9057d75c9938afa2f407bee0a80cf5f6a4c Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Wed, 12 Apr 2017 12:58:58 -0400 Subject: [PATCH 2/2] add a fast path function for checking for empty intersections add `obviously_disjoint` fast path to type_morespecific further helps #21173 --- src/gf.c | 6 ++---- src/julia.h | 1 + src/subtype.c | 45 +++++++++++++++++++++++++++++++++++---------- src/typemap.c | 2 +- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/gf.c b/src/gf.c index c6df93465cf1a..c61853f0e3218 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1397,7 +1397,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method size_t ins = 0; for (i = 1; i < na; i += 2) { jl_value_t *backedgetyp = backedges[i - 1]; - if (jl_type_intersection(backedgetyp, (jl_value_t*)type) != (jl_value_t*)jl_bottom_type) { + if (!jl_has_empty_intersection(backedgetyp, (jl_value_t*)type)) { jl_method_instance_t *backedge = (jl_method_instance_t*)backedges[i]; invalidate_method_instance(backedge, env.max_world, 0); env.invalidated = 1; @@ -1731,10 +1731,8 @@ JL_DLLEXPORT int jl_has_call_ambiguities(jl_tupletype_t *types, jl_method_t *m) if (m->ambig == jl_nothing) return 0; for (size_t i = 0; i < jl_array_len(m->ambig); i++) { jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(m->ambig, i); - if (jl_type_intersection((jl_value_t*)mambig->sig, - (jl_value_t*)types) != (jl_value_t*)jl_bottom_type) { + if (!jl_has_empty_intersection((jl_value_t*)mambig->sig, (jl_value_t*)types)) return 1; - } } return 0; } diff --git a/src/julia.h b/src/julia.h index f5efda7e378c7..b3188fa2b5905 100644 --- a/src/julia.h +++ b/src/julia.h @@ -982,6 +982,7 @@ JL_DLLEXPORT int jl_isa(jl_value_t *a, jl_value_t *t); JL_DLLEXPORT int jl_types_equal(jl_value_t *a, jl_value_t *b); JL_DLLEXPORT jl_value_t *jl_type_union(jl_value_t **ts, size_t n); JL_DLLEXPORT jl_value_t *jl_type_intersection(jl_value_t *a, jl_value_t *b); +JL_DLLEXPORT int jl_has_empty_intersection(jl_value_t *x, jl_value_t *y); JL_DLLEXPORT jl_value_t *jl_type_unionall(jl_tvar_t *v, jl_value_t *body); JL_DLLEXPORT const char *jl_typename_str(jl_value_t *v); JL_DLLEXPORT const char *jl_typeof_str(jl_value_t *v); diff --git a/src/subtype.c b/src/subtype.c index 8a1984c7a285f..113cb1675085e 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -91,6 +91,7 @@ typedef struct { int invdepth; // current number of invariant constructors we're nested in int ignore_free; // treat free vars as black boxes; used during intersection int intersection; // true iff subtype is being called from intersection + int emptiness_only; // true iff intersection only needs to test for emptiness } jl_stenv_t; // state manipulation utilities @@ -238,7 +239,7 @@ 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) +static int obviously_disjoint(jl_value_t *a, jl_value_t *b, int specificity) { if (a == b || a == (jl_value_t*)jl_any_type || b == (jl_value_t*)jl_any_type) return 0; @@ -280,7 +281,8 @@ static int obviously_disjoint(jl_value_t *a, jl_value_t *b) else if (jl_is_va_tuple(bd)) { nb -= 1; } - else if (na != nb) { + else if (!specificity && na != nb) { + // note: some disjoint types (e.g. tuples of different lengths) can be more specific return 1; } np = na < nb ? na : nb; @@ -298,13 +300,18 @@ static int obviously_disjoint(jl_value_t *a, jl_value_t *b) 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)) + else if (obviously_disjoint(ai, bi, specificity)) return 1; } - else { + else if (!specificity) { + // Tuple{1} is more specific than Tuple{Any} return 1; } } + else if (jl_is_type(bi)) { + if (!specificity) + return 1; + } else if (!jl_egal(ai, bi)) { return 1; } @@ -534,8 +541,9 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8 // if the var for this unionall (based on identity) already appears somewhere // in the environment, rename to get a fresh var. while (btemp != NULL) { - if (btemp->var == u->var || jl_has_typevar(btemp->lb, u->var) || - jl_has_typevar(btemp->ub, u->var)) { + if (btemp->var == u->var || + (btemp->lb != jl_bottom_type && jl_has_typevar(btemp->lb, u->var)) || + (btemp->ub != (jl_value_t*)jl_any_type && jl_has_typevar(btemp->ub, u->var))) { u = rename_unionall(u); break; } @@ -619,7 +627,7 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8 btemp = e->vars; while (btemp != NULL) { jl_value_t *vi = btemp->ub; - if (vi != (jl_value_t*)vb.var && jl_has_typevar(vi, vb.var)) { + if (vi != (jl_value_t*)vb.var && vi != (jl_value_t*)jl_any_type && jl_has_typevar(vi, vb.var)) { btemp->ub = jl_new_struct(jl_unionall_type, vb.var, vi); btemp->lb = jl_bottom_type; } @@ -1003,6 +1011,7 @@ static void init_stenv(jl_stenv_t *e, jl_value_t **env, int envsz) e->invdepth = 0; e->ignore_free = 0; e->intersection = 0; + e->emptiness_only = 0; e->Lunions.depth = 0; e->Runions.depth = 0; e->Lunions.more = 0; e->Runions.more = 0; } @@ -2012,6 +2021,8 @@ static jl_value_t *intersect_all(jl_value_t *x, jl_value_t *y, jl_stenv_t *e) int lastset = 0, niter = 0; jl_value_t *ii = intersect(x, y, e, 0); while (e->Runions.more) { + if (e->emptiness_only && ii != jl_bottom_type) + return ii; e->Runions.depth = 0; int set = e->Runions.more - 1; e->Runions.more = 0; @@ -2042,16 +2053,28 @@ static jl_value_t *intersect_all(jl_value_t *x, jl_value_t *y, jl_stenv_t *e) // type intersection entry points -JL_DLLEXPORT jl_value_t *jl_intersect_types(jl_value_t *x, jl_value_t *y) +static jl_value_t *intersect_types(jl_value_t *x, jl_value_t *y, int emptiness_only) { jl_stenv_t e; - if (obviously_disjoint(x, y)) + if (obviously_disjoint(x, y, 0)) return jl_bottom_type; init_stenv(&e, NULL, 0); e.intersection = 1; + e.emptiness_only = emptiness_only; return intersect_all(x, y, &e); } +JL_DLLEXPORT jl_value_t *jl_intersect_types(jl_value_t *x, jl_value_t *y) +{ + return intersect_types(x, y, 0); +} + +// TODO: this can probably be done more efficiently +JL_DLLEXPORT int jl_has_empty_intersection(jl_value_t *x, jl_value_t *y) +{ + return intersect_types(x, y, 1) == jl_bottom_type; +} + // return a SimpleVector of all vars from UnionAlls wrapping a given type jl_svec_t *jl_outer_unionall_vars(jl_value_t *u) { @@ -2073,7 +2096,7 @@ jl_svec_t *jl_outer_unionall_vars(jl_value_t *u) 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 (obviously_disjoint(a, b, 0)) { if (issubty && a == jl_bottom_type) *issubty = 1; return jl_bottom_type; } @@ -2570,6 +2593,8 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty JL_DLLEXPORT int jl_type_morespecific(jl_value_t *a, jl_value_t *b) { + if (obviously_disjoint(a, b, 1)) + return 0; if (jl_subtype(b, a)) return 0; if (jl_subtype(a, b)) return 1; return type_morespecific_(a, b, 0, NULL); diff --git a/src/typemap.c b/src/typemap.c index 0119678b92584..b70accd357b87 100644 --- a/src/typemap.c +++ b/src/typemap.c @@ -515,7 +515,7 @@ int jl_typemap_intersection_visitor(union jl_typemap_t map, int offs, else { // else an array scan is required to check subtypes // first, fast-path: optimized pre-intersection test to see if `ty` could intersect with any Type - if (typetype || jl_type_intersection((jl_value_t*)jl_type_type, ty) != jl_bottom_type) + if (typetype || !jl_has_empty_intersection((jl_value_t*)jl_type_type, ty)) if (!jl_typemap_intersection_array_visitor(&cache->targ, ty, 1, offs, closure)) return 0; } }