Skip to content

Commit

Permalink
types: per comment, avoid copy when t is not bound (#32176)
Browse files Browse the repository at this point in the history 
As mentioned in #9378.
Fix recursion issue mentioned in #25796 by using inst_datatype_inner
instead of inst_datatype, so that we shouldn't be making copies of
any non-bound objects (anything maybe-cacheable) now.
  • Loading branch information
@vtjnash
vtjnash authored May 31, 2019
1 parent d8ff21c commit 217507f
Showing 1 changed file with 74 additions and 80 deletions.
154 changes: 74 additions & 80 deletions src/jltypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,8 +819,21 @@ static int within_typevar(jl_value_t *t, jl_value_t *vlb, jl_value_t *vub)
struct _jl_typestack_t;
typedef struct _jl_typestack_t jl_typestack_t;

static jl_value_t *inst_datatype(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack);
static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack, jl_typeenv_t *env);

// Build an environment mapping a TypeName's parameters to parameter values.
// This is the environment needed for instantiating a type's supertype and field types.
static jl_value_t *inst_datatype_env(jl_value_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack, jl_typeenv_t *env, int c)
{
if (jl_is_datatype(dt))
return inst_datatype_inner((jl_datatype_t*)dt, p, iparams, ntp, cacheable, stack, env);
assert(jl_is_unionall(dt));
jl_unionall_t *ua = (jl_unionall_t*)dt;
jl_typeenv_t e = { ua->var, iparams[c], env };
return inst_datatype_env(ua->body, p, iparams, ntp, cacheable, stack, &e, c + 1);
}

jl_value_t *jl_apply_type(jl_value_t *tc, jl_value_t **params, size_t n)
{
Expand All @@ -836,12 +849,13 @@ jl_value_t *jl_apply_type(jl_value_t *tc, jl_value_t **params, size_t n)
if (jl_is_datatype(u) && n == jl_nparams((jl_datatype_t*)u) &&
((jl_datatype_t*)u)->name->wrapper == tc) {
int cacheable = 1;
for(i=0; i < n; i++) {
for (i = 0; i < n; i++) {
if (jl_has_free_typevars(params[i])) {
cacheable = 0; break;
cacheable = 0;
break;
}
}
return inst_datatype((jl_datatype_t*)u, NULL, params, n, cacheable, NULL);
return inst_datatype_env(tc, NULL, params, n, cacheable, NULL, NULL, 0);
}
}
JL_GC_PUSH1(&tc);
Expand Down Expand Up @@ -1068,6 +1082,8 @@ static jl_value_t *normalize_vararg(jl_value_t *va)
return va;
}

static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev, jl_typestack_t *stack);

static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack, jl_typeenv_t *env)
{
Expand Down Expand Up @@ -1262,6 +1278,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
if (cacheable && !ndt->abstract)
ndt->uid = jl_assign_type_uid();

jl_datatype_t *primarydt = ((jl_datatype_t*)jl_unwrap_unionall(tn->wrapper));
if (istuple || isnamedtuple) {
ndt->super = jl_any_type;
}
Expand All @@ -1273,10 +1290,12 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
jl_cache_type_(ndt);
}
jl_svec_t *ftypes = dt->types;
if (ftypes == NULL)
ftypes = primarydt->types;
if (ftypes == NULL || dt->super == NULL) {
// in the process of creating this type definition:
// need to instantiate the super and types fields later
assert(inside_typedef && !istuple && !isnamedtuple);
assert((inside_typedef || primarydt->super) && !istuple && !isnamedtuple);
arraylist_push(&partial_inst, ndt);
}
else if (!isnamedtuple && !istuple) {
Expand All @@ -1287,7 +1306,10 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
}
else if (cacheable) {
// recursively instantiate the types of the fields
ndt->types = inst_ftypes(ftypes, env, stack);
if (dt->types == NULL)
ndt->types = jl_compute_fieldtypes(ndt);
else
ndt->types = inst_ftypes(ftypes, env, stack);
jl_gc_wb(ndt, ndt->types);
}
}
Expand Down Expand Up @@ -1315,25 +1337,6 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
return (jl_value_t*)ndt;
}

// Build an environment mapping a TypeName's parameters to parameter values.
// This is the environment needed for instantiating a type's supertype and field types.
static jl_value_t *inst_datatype_env(jl_value_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack, jl_typeenv_t *env, int c)
{
if (jl_is_datatype(dt))
return inst_datatype_inner((jl_datatype_t*)dt, p, iparams, ntp, cacheable, stack, env);
assert(jl_is_unionall(dt));
jl_unionall_t *ua = (jl_unionall_t*)dt;
jl_typeenv_t e = { ua->var, iparams[c], env };
return inst_datatype_env(ua->body, p, iparams, ntp, cacheable, stack, &e, c+1);
}

static jl_value_t *inst_datatype(jl_datatype_t *dt, jl_svec_t *p, jl_value_t **iparams, size_t ntp,
int cacheable, jl_typestack_t *stack)
{
return inst_datatype_env(dt->name->wrapper, p, iparams, ntp, cacheable, stack, NULL, 0);
}

static jl_tupletype_t *jl_apply_tuple_type_v_(jl_value_t **p, size_t np, jl_svec_t *params)
{
int cacheable = 1;
Expand All @@ -1342,9 +1345,7 @@ static jl_tupletype_t *jl_apply_tuple_type_v_(jl_value_t **p, size_t np, jl_svec
if (!jl_is_concrete_type(p[i]))
cacheable = 0;
}
jl_datatype_t *ndt = (jl_datatype_t*)inst_datatype(jl_anytuple_type, params, p, np,
cacheable, NULL);
return ndt;
return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, params, p, np, cacheable, NULL, NULL);
}

JL_DLLEXPORT jl_tupletype_t *jl_apply_tuple_type(jl_svec_t *params)
Expand All @@ -1359,12 +1360,12 @@ JL_DLLEXPORT jl_tupletype_t *jl_apply_tuple_type_v(jl_value_t **p, size_t np)

jl_datatype_t *jl_inst_concrete_tupletype(jl_svec_t *p)
{
return (jl_datatype_t*)inst_datatype(jl_anytuple_type, p, jl_svec_data(p), jl_svec_len(p), 1, NULL);
return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, p, jl_svec_data(p), jl_svec_len(p), 1, NULL, NULL);
}

jl_datatype_t *jl_inst_concrete_tupletype_v(jl_value_t **p, size_t np)
{
return (jl_datatype_t*)inst_datatype(jl_anytuple_type, NULL, p, np, 1, NULL);
return (jl_datatype_t*)inst_datatype_inner(jl_anytuple_type, NULL, p, np, 1, NULL, NULL);
}

static jl_svec_t *inst_ftypes(jl_svec_t *p, jl_typeenv_t *env, jl_typestack_t *stack)
Expand Down Expand Up @@ -1393,7 +1394,7 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
jl_svec_t *tp = tt->parameters;
size_t ntp = jl_svec_len(tp);
// Instantiate NTuple{3,Int}
// Note this does not instantiate Tuple{Vararg{Int,3}}; that's done in inst_datatype
// Note this does not instantiate Tuple{Vararg{Int,3}}; that's done in inst_datatype_inner
if (jl_is_va_tuple(tt) && ntp == 1) {
// If this is a Tuple{Vararg{T,N}} with known N, expand it to
// a fixed-length tuple
Expand All @@ -1419,12 +1420,13 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
jl_value_t **iparams;
int onstack = ntp < jl_page_size/sizeof(jl_value_t*);
JL_GC_PUSHARGS(iparams, onstack ? ntp : 1);
jl_svec_t *ip_heap=NULL;
jl_svec_t *ip_heap = NULL;
if (!onstack) {
ip_heap = jl_alloc_svec(ntp);
iparams[0] = (jl_value_t*)ip_heap;
iparams = jl_svec_data(ip_heap);
}
int bound = 0;
int cacheable = 1;
if (jl_is_va_tuple(tt))
cacheable = 0;
Expand All @@ -1435,12 +1437,14 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_
iparams[i] = pi;
if (ip_heap)
jl_gc_wb(ip_heap, pi);
bound |= (pi != elt);
if (cacheable && !jl_is_concrete_type(pi))
cacheable = 0;
}
jl_value_t *result = inst_datatype((jl_datatype_t*)tt, ip_heap, iparams, ntp, cacheable, stack);
if (bound)
t = inst_datatype_inner(tt, ip_heap, iparams, ntp, cacheable, stack, env);
JL_GC_POP();
return result;
return t;
}

static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t *stack, int check)
Expand All @@ -1451,88 +1455,78 @@ static jl_value_t *inst_type_w_(jl_value_t *t, jl_typeenv_t *env, jl_typestack_t
while (e != NULL) {
if (e->var == (jl_tvar_t*)t) {
jl_value_t *val = e->val;
// TODO jb/subtype this seems unnecessary
//if (check && !jl_is_typevar(val) && !within_typevar(val, (jl_tvar_t*)t)) {
// jl_type_error_rt("type parameter",
// jl_symbol_name(((jl_tvar_t*)t)->name), t, val);
//}
return val;
}
e = e->prev;
}
return (jl_value_t*)t;
return t;
}
if (jl_is_unionall(t)) {
if (!jl_has_free_typevars(t))
return t;
jl_unionall_t *ua = (jl_unionall_t*)t;
jl_value_t *res=NULL, *lb=ua->var->lb, *ub=ua->var->ub;
JL_GC_PUSH3(&lb, &ub, &res);
res = jl_new_struct(jl_unionall_type, ua->var, NULL);
if (jl_has_bound_typevars(ua->var->lb, env))
lb = inst_type_w_(ua->var->lb, env, stack, check);
if (jl_has_bound_typevars(ua->var->ub, env))
ub = inst_type_w_(ua->var->ub, env, stack, check);
if (lb != ua->var->lb || ub != ua->var->ub) {
((jl_unionall_t*)res)->var = jl_new_typevar(ua->var->name, lb, ub);
jl_gc_wb(res, ((jl_unionall_t*)res)->var);
jl_value_t *lb = NULL;
jl_value_t *var = NULL;
jl_value_t *newbody = NULL;
JL_GC_PUSH3(&lb, &var, &newbody);
lb = inst_type_w_(ua->var->lb, env, stack, check);
var = inst_type_w_(ua->var->ub, env, stack, check);
if (lb != ua->var->lb || var != ua->var->ub) {
var = (jl_value_t*)jl_new_typevar(ua->var->name, lb, var);
}
else {
var = (jl_value_t*)ua->var;
}
jl_typeenv_t newenv = { ua->var, (jl_value_t*)((jl_unionall_t*)res)->var, env };
jl_value_t *newbody = inst_type_w_(ua->body, &newenv, stack, check);
jl_typeenv_t newenv = { ua->var, var, env };
newbody = inst_type_w_(ua->body, &newenv, stack, check);
if (newbody == (jl_value_t*)jl_emptytuple_type) {
// NTuple{0} => Tuple{} can make a typevar disappear
res = (jl_value_t*)jl_emptytuple_type;
t = (jl_value_t*)jl_emptytuple_type;
}
else {
((jl_unionall_t*)res)->body = newbody;
jl_gc_wb(res, newbody);
else if (newbody != ua->body || var != (jl_value_t*)ua->var) {
// if t's parameters are not bound in the environment, return it uncopied (#9378)
t = jl_new_struct(jl_unionall_type, var, newbody);
}
JL_GC_POP();
return res;
return t;
}
if (jl_is_uniontype(t)) {
jl_uniontype_t *u = (jl_uniontype_t*)t;
jl_value_t *a = inst_type_w_(u->a, env, stack, check);
jl_value_t *b = NULL;
JL_GC_PUSH2(&a, &b);
b = inst_type_w_(u->b, env, stack, check);
jl_value_t *res;
if (a == u->a && b == u->b) {
res = t;
}
else {
if (a != u->a || b != u->b) {
jl_value_t *uargs[2] = {a, b};
res = jl_type_union(uargs, 2);
t = jl_type_union(uargs, 2);
}
JL_GC_POP();
return res;
return t;
}
if (!jl_is_datatype(t))
return t;
jl_datatype_t *tt = (jl_datatype_t*)t;
jl_svec_t *tp = tt->parameters;
if (tp == jl_emptysvec)
return (jl_value_t*)t;
return t;
jl_typename_t *tn = tt->name;
if (tn == jl_tuple_typename)
return inst_tuple_w_(t, env, stack, check);
size_t ntp = jl_svec_len(tp);
jl_value_t **iparams;
JL_GC_PUSHARGS(iparams, ntp);
int cacheable = 1, bound = 0;
for(i=0; i < ntp; i++) {
for (i = 0; i < ntp; i++) {
jl_value_t *elt = jl_svecref(tp, i);
iparams[i] = inst_type_w_(elt, env, stack, check);
bound |= (iparams[i] != elt);
if (cacheable && jl_has_free_typevars(iparams[i]))
jl_value_t *pi = inst_type_w_(elt, env, stack, check);
iparams[i] = pi;
bound |= (pi != elt);
if (cacheable && jl_has_free_typevars(pi))
cacheable = 0;
}
// if t's parameters are not bound in the environment, return it uncopied (#9378)
if (!bound) { JL_GC_POP(); return (jl_value_t*)t; }

jl_value_t *result = inst_datatype((jl_datatype_t*)tt, NULL, iparams, ntp, cacheable, stack);
if (bound)
t = inst_datatype_inner(tt, NULL, iparams, ntp, cacheable, stack, env);
JL_GC_POP();
return result;
return t;
}

static jl_value_t *instantiate_with(jl_value_t *t, jl_value_t **env, size_t n, jl_typeenv_t *te)
Expand All @@ -1549,21 +1543,21 @@ jl_value_t *jl_instantiate_type_with(jl_value_t *t, jl_value_t **env, size_t n)
return instantiate_with(t, env, n, NULL);
}

static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev)
static jl_value_t *_jl_instantiate_type_in_env(jl_value_t *ty, jl_unionall_t *env, jl_value_t **vals, jl_typeenv_t *prev, jl_typestack_t *stack)
{
jl_typeenv_t en = { env->var, vals[0], prev };
if (jl_is_unionall(env->body))
return _jl_instantiate_type_in_env(ty, (jl_unionall_t*)env->body, vals + 1, &en);
return _jl_instantiate_type_in_env(ty, (jl_unionall_t*)env->body, vals + 1, &en, stack);
else
return inst_type_w_(ty, &en, NULL, 1);
return inst_type_w_(ty, &en, stack, 1);
}

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 *typ = ty;
if (jl_is_unionall(env)) {
JL_TRY {
typ = _jl_instantiate_type_in_env(ty, env, vals, NULL);
typ = _jl_instantiate_type_in_env(ty, env, vals, NULL, NULL);
}
JL_CATCH {
typ = jl_bottom_type;
Expand Down

0 comments on commit 217507f

Please sign in to comment.