Skip to content

Commit

Permalink
fix deserializer jl_recache_type to walk type parameter list completely
Browse files Browse the repository at this point in the history
previously, it only walked through types with `uid = -1`,
but in fact we needed to walk through any uncached type,
including those with `uid = 0` and those gotten from jl_typeof

fix #17809

(cherry picked from commit 997da86)
ref #17820
  • Loading branch information
vtjnash authored and tkelman committed Aug 5, 2016
1 parent cede539 commit acfd04c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 21 deletions.
53 changes: 32 additions & 21 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -2222,35 +2222,46 @@ static jl_datatype_t *jl_recache_type(jl_datatype_t *dt, size_t start, jl_value_
{
if (v == NULL)
v = dt->instance; // the instance before unique'ing
jl_datatype_t *t; // the type after unique'ing
if (dt->uid == -1) {
jl_svec_t *tt = dt->parameters;
size_t l = jl_svec_len(tt);
if (l == 0) { // jl_cache_type doesn't work if length(parameters) == 0
dt->uid = jl_assign_type_uid();
t = dt;
}
else {
// recache all type parameters, then type type itself
size_t i;
for (i = 0; i < l; i++) {
jl_datatype_t *p = (jl_datatype_t*)jl_svecref(tt, i);
if (jl_is_datatype(p) && p->uid == -1) {
jl_svec_t *tt = dt->parameters;
if (dt->uid == 0 || dt->uid == -1) {
// recache all type parameters
size_t i, l = jl_svec_len(tt);
for (i = 0; i < l; i++) {
jl_datatype_t *p = (jl_datatype_t*)jl_svecref(tt, i);
if (jl_is_datatype(p)) {
if (p->uid == -1 || p->uid == 0) {
jl_datatype_t *cachep = jl_recache_type(p, start, NULL);
if (p != cachep)
if (p != cachep) {
assert(jl_types_equal((jl_value_t*)p, (jl_value_t*)cachep));
jl_svecset(tt, i, cachep);
}
}
}
else {
jl_datatype_t *tp = (jl_datatype_t*)jl_typeof(p);
if (jl_is_datatype_singleton(tp)) {
if (tp->uid == -1) {
tp = jl_recache_type(tp, start, NULL);
}
if ((jl_value_t*)p != tp->instance)
jl_svecset(tt, i, tp->instance);
assert(tp->uid != 0);
if (tp->uid == -1) {
tp = jl_recache_type(tp, start, NULL);
}
if (tp->instance && (jl_value_t*)p != tp->instance)
jl_svecset(tt, i, tp->instance);
}
}
}

jl_datatype_t *t; // the type after unique'ing
if (dt->uid == 0) {
return dt;
}
else if (dt->uid == -1) {
if (jl_svec_len(tt) == 0) { // jl_cache_type doesn't work if length(parameters) == 0
dt->uid = jl_assign_type_uid();
t = dt;
}
else {
dt->uid = 0;
t = (jl_datatype_t*)jl_cache_type_(dt);
assert(jl_types_equal((jl_value_t*)t, (jl_value_t*)dt));
}
}
else {
Expand Down
19 changes: 19 additions & 0 deletions test/compile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ try
include_dependency("bar.jl")
end
# test for creation of some reasonably complicated type
immutable MyType{T} end
const t17809s = Any[
Tuple{
Type{Ptr{MyType{i}}},
Array{Ptr{MyType{MyType{:sym}()}}(0), 0},
Val{Complex{Int}(1, 2)},
Val{3},
Val{nothing}}
for i = 0:25]
# test that types and methods get reconnected correctly
# issue 16529 (adding a method to a type with no instances)
(::Task)(::UInt8, ::UInt16, ::UInt32) = 2
Expand Down Expand Up @@ -110,6 +121,14 @@ try
@test Vector{Foo.NominalValue{Int32, Int64}}() == 3
@test Vector{Foo.NominalValue{UInt, UInt}}() == 4
@test Vector{Foo.NominalValue{Int, Int}}() == 5
@test all(i -> Foo.t17809s[i + 1] ===
Tuple{
Type{Ptr{Foo.MyType{i}}},
Array{Ptr{Foo.MyType{Foo.MyType{:sym}()}}(0), 0},
Val{Complex{Int}(1, 2)},
Val{3},
Val{nothing}},
0:25)
end

Baz_file = joinpath(dir, "Baz.jl")
Expand Down

2 comments on commit acfd04c

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, vs = "@0030eec2f332f353e6890ca289ac2aca55532dde")

(0.5.0-rc1+1 vs 0.5.0-rc0)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

Please sign in to comment.