JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.43k stars 5.45k forks source link

type inference problem with captured type in closures #23618

Open sbromberger opened 7 years ago

sbromberger commented 7 years ago
function foo(a::AbstractVector)
    T = eltype(a)
    b = Vector{T}()
    c = [Set{T}() for x in a]
    return length(c)
end

a = rand(1:10, 200);

@code_warntype shows an inference failure in b and c.

Changing to c = Set{T}[Set{T}() for x in a] fixes c's inference issue.

yuyichao commented 7 years ago

b is just https://github.com/JuliaLang/julia/pull/23280 c is real and seems to be caused by closure captured type only being specialized on T::DataType.

ChrisRackauckas commented 7 years ago

Is this the same as https://github.com/JuliaLang/julia/issues/15276 ?

yuyichao commented 7 years ago

Depending on what do you call the scope of #15276. Both of these are related to closures but the sources are quite different.

All cases in #15276 are about the frontend not able to tell if a variable can be mutated in the closure/while the closure is executing so it creates a Core.Box for it (the original title was more specific about this). In this case, the lowering correctly determined that the variable doesn't need a Box but it fails to generate code the Type{T} and instead generated/inferred code for DataType.

sbromberger commented 7 years ago

x-ref https://discourse.julialang.org/t/performance-issue-with-use-of-eltype/5764. That is, there is a secondary issue that performance is slow even with explicit typing due to type instability in the inner comprehension loop.

tkf commented 4 years ago

A MWE is:

julia> let f = Int
           typeof(x -> f(x))
       end
var"#51#52"{DataType}

This comes up in with types:

julia> let f = Int ∘ identity
           @inferred f(1)
       end
ERROR: return type Int64 does not match inferred return type Any
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] top-level scope at REPL[71]:2

A simple workaround is:

julia> begin
       prefer_singleton_callable(::Type{T}) where T = SingletonCallable{T}()
       prefer_singleton_callable(f) = f
       struct SingletonCallable{T} end
       (::SingletonCallable{T})(x) where T = T(x)
       end;

julia> let f = prefer_singleton_callable(Int) ∘ identity
           @inferred f(1)
       end
1

Can this be done during lowering?

schlichtanders commented 2 years ago

I just stumbled upon this issue and am surprised that it is that old. Is there a reason that this hasn't been prioritized yet?

JeffBezanson commented 2 years ago

If something seems important but has been sitting around for a long time, it's usually a safe bet that it's difficult.

timholy commented 2 years ago

One of the few certain fixes is to allow lowering to call type inference. Currently lowering is written in lisp and is firmly "upstream" of type inference---we can't run type inference until code is presented in lowered form.

If there isn't a simpler solution that would be truly reliable, I am not sure I can think of a bug fix that would require more re-engineering than this one. Care to tackle it, @schlichtanders? :smile:

KristofferC commented 2 years ago

Seems this issue is almost fixed in https://github.com/JuliaLang/julia/pull/40985, just need a bit more.

timholy commented 2 years ago

Yeah, I guess this one is a bit easier than #15276, which is the one that really annoys me.

schlichtanders commented 2 years ago

One of the few certain fixes is to allow lowering to call type inference. Currently lowering is written in lisp and is firmly "upstream" of type inference---we can't run type inference until code is presented in lowered form.

If there isn't a simpler solution that would be truly reliable, I am not sure I can think of a bug fix that would require more re-engineering than this one

Very impressive indeed. Thanks for sharing this summary. I tried to optimize my package ExtensibleEffects.jl when I stumbled upon this, so I am more on the user side of things and won't have enough time and resources to improve Core Julia here ;-)

mschauer commented 1 year ago

The minimum working example doesn't trigger this anymore

julia> versioninfo()
Julia Version 1.10.0-beta1

julia> let f = Int ∘ identity
                  @inferred f(1)
              end
1

but the initial issue seems to persist:

julia> @code_warntype foo(a)
MethodInstance for foo(::Vector{Int64})
  from foo(a::AbstractVector) @ Main REPL[152]:1
Arguments
  #self#::Core.Const(foo)
  a::Vector{Int64}
Locals
  #563::var"#563#564"{DataType}
  c::Vector # <------
  b::Vector{Int64}
  T::Type{Int64}
Body::Int64
KristofferC commented 11 months ago

Still seems to be an issue