inline inexact method matches

[vtjnash]

@JeffBezanson Do you know where this going wrong? if !(aeitype <: methitype) appears to be failing, even though I haven't removed the precondition test of if !(atypes <: meth[1])

inspired by 024bbce#commitcomment-6137158

Is the following behavior correct? I think it must be true, but is also causing type-inference issues with this patch on the indicated line

julia> methods(convert,(Type{(Char,Char)},(Char,Char)))
1-element Array{Any,1}:
 convert(T::(Any,Any...),x::(Any,Any...)) at base.jl:21

julia> methods(convert,((Char,Char),(Char,Char)))
1-element Array{Any,1}:
 convert(T::(Any,Any...),x::(Any,Any...)) at base.jl:21

[JeffBezanson]

I think the problem is that we construct an expression of type (Char,) to serve as the vararg tail, but the type in the matched method is Char... which gets unpacked to Char.

Also this should throw method errors, not typeasserts.

[vtjnash]

that was much harder than expected to match all of the possible method matches to their arguments

i still need to convert the typeasserts into a no method error.

any idea why I can't inline methods with a type of Undef?

(ps. this does fix the performance regression it was supposed to fix)

[JeffBezanson]

What is a "method with a type of Undef"? You mean where an argument has type "Undef"?

Can you describe the performance regression in more detail? What call site was affected? Obviously inlining more things helps generally, but I'm curious about how this came up in practice.

[vtjnash]

Yes, the latter. I can inline a call1 method, unless Undef appears and confuses the compiler

It came up in the printf branch, where I was depending on the right specialized method being inlined for performance (see perf test change in this commit. When I have more time, I'll rerun the full perf test and see what else it affects)

[vtjnash]

this doesn't have any impact on the perf tests, other than to make the jn/printf2 branch as fast as before without resorting to editing the test :)

it is also ready to merge

[vtjnash]

this change would be even more interesting for applying outside of the inliner, to specialize call1 functions that we do not want to inline. however, that's work for another day (free free to open an issue and tag me on it if you want this soon)

[vtjnash]

Got another mystery for you Jeff. Why can't the ASCIIString constructor call the top(box)? It also apparently thinks that ASCIIString != ASCIIString

julia> ASCIIString("")
ERROR: box not defined
 in ASCIIString at no file

julia> code_typed(ASCIIString,(ASCIIString,))
1-element Array{Any,1}:
 :($(Expr(:lambda, {:data}, {{:_var0,:_var2,:_var3,:_var4},{{:data,ASCIIString,0},{:_var0,ASCIIString,18},{:_var2,Type{Array{Uint8,1}},18},{:_var3,Type{Array{Uint8,1}},18},{:_var4,Type{ASCIIString},18}},{}}, :(begin 
        _var4 = top(getfield)(Core,:ASCIIString)::Type{ASCIIString}
        _var3 = Array{Uint8,1}
        unless top(box)(Bool,top(or_int)(top(not_int)(top(isa)(_var3::Type{Array{Uint8,1}},Type{Array{Uint8,1}})::Bool)::Bool,top(box)(Bool,top(or_int)(top(not_int)(top(isa)(data::ASCIIString,ASCIIString)::Bool)::Bool,false)::Bool)::Bool)::Bool)::Bool goto 1
        throw(MethodError(convert,top(tuple)(_var3::Type{Array{Uint8,1}},data::ASCIIString)::(Any...,)))::None
        1: 
        _var0 = data::ASCIIString
        _var2 = _var3::Type{Array{Uint8,1}}
        return $(Expr(:new, :(_var4::Type{ASCIIString}), :(convert_default(_var2::Type{Array{Uint8,1}},_var0::ASCIIString,convert)::None)))::ASCIIString
    end::ASCIIString))))

[JeffBezanson]

box is inside Intrinsics and not imported most places, including Core where this constructor is.

Why is Array{Uint8,1}, which is just a constant type, getting pulled into a temp var? Same with data, which is just an argument that is never even assigned to.

[JeffBezanson]

Also, why is this call to convert considered an inexact match? The argument types are known exactly.

[vtjnash]

I thought top() got you these function, but perhaps not. Do I need getfield instead?

I was wondering the same, until I remembered this was a stress test where I was applying the argument test to all function calls

[vtjnash]

ok, it was harder than expected to convert the typeasserts into a MethodError, but I seem to have succceeded

[vtjnash]

make perf shows that this affects:
(a) restores printfd in the jn/printf2 branch to full speed, from about 90 to 30
(b) hurts spectralnorm speed from about 2.8 to 6.9

[JeffBezanson]

That's quite a slowdown in spectralnorm. Would be good to get a handle on the cause.

[vtjnash]

yes. however, i see now that the slowdown is unrelated to this pull request -- master is giving me the same timings too

[staticfloat]

Unfortuantely, the due to codespeed breakage, we don't have the exact commit that this slowdown was introduced by, but we do get a range.

EDIT: updated above link to look at the julia.mit.edu environment, which has a tighter range on things.

[JeffBezanson]

Ok, it's kind of a relief that the slowdown is not due to this PR, because that would be weird.

[vtjnash]

git bisect gave me e805fd6

that's a logical commit to have caused the slowdown -- i'll tweak the threshold again

[vtjnash]

@JeffBezanson ping