Break type stability of Rational with unrelated function (0.5-specific)

[timholy]

Unitful breaks the type-stability of Rational:

julia> using Unitful
INFO: Recompiling stale cache file /home/tim/.julia/lib/v0.5/Unitful.ji for module Unitful.

julia> @code_warntype Rational(5463, 20)
Variables:
  #self#::Type{Rational}
  n::Int64
  d::Int64

Body:
  begin 
      return $(Expr(:invoke, LambdaInfo for Rational{Int64}(::Int64, ::Int64), Rational{Int64}, :(n), :(d)))
  end::Any

If you comment out this line and this function, then it's back to normal. Indeed, just the signature of the latter function (i.e., defining a blank body) is sufficient to break the type-stability of Rational.

What I find curious is that the latter function does not obviously have anything to do with Rational. Seems to suggest some kind of corruption.

[timholy]

CC @ajkeller34

[ajkeller34]

I am equal parts amused and disturbed. I'll take a look, but at first glance I have no idea why that would happen...

[timholy]

I suspect this is not your fault at all, and that it's uncovered some latent bug. I could be wrong, though.

[andreasnoack]

Which version of Julia?

[ajkeller34]

I can reproduce the error with:

julia> versioninfo()
Julia Version 0.5.0-rc4+0
Commit 9c76c3e (2016-09-09 01:43 UTC)
Platform Info:
  System: Darwin (x86_64-apple-darwin13.4.0)
  CPU: Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.7.1 (ORCJIT, haswell)

[vtjnash]

possibly related to #18449

[timholy]

It is indeed precompilation-dependent.

[timholy]

This appears to be fixed on current master (but not 0.5). The same situation seems to apply to JuliaNLSolvers/Optim.jl#283. However, the probable fix (#18413) is probably not trivially backportable? If so, this probably needs to be a priority for point-releases on 0.5.

[timholy]

git bisect identifies 9bb8af8 as the origin, and I verified manually that the bug is present with that commit but not with the previous one on the release-0.5 branch. CC @vtjnash.

[vtjnash]

Yes, I believe I see what is wrong. I believe it is also broken on master, but just shifted (due to some code I neglected to put back after the rearrangement in #18413)

[PallHaraldsson]

I was surprised by "Break type"; -> "Breaks type" I guess better for title?

[Isn't it ALWAYS better to have type stability? Can you think of a counterexample, and thus you would want to modify code from type stable to unstable?]

[gragusa]

It actually breaks many functions. For instance, loading a package in a fresh julia session randn becomes type unstable. This is related to JuliaNLSolvers/Optim.jl#283 . Interestingly, if in a new session I first run randn() then randn() is type stable even after the package is loaded.

# julia
using KernelDensity
@code_warntype randn(Float64)

Variables:
  #self#::Base.Random.#randn
  #unused#::Type{Float64}

Body:
  begin
      return $(Expr(:invoke, LambdaInfo for randn(::MersenneTwister, ::Type{Float64}), :(Base.Random.randn), :(Base.Random.GLOBAL_RNG), :($(Expr(:static_parameter, 1)))))
  end::Any

# julia
randn()
using KernelDensity
@code_warntype randn(Float64)

Variables:
  #self#::Base.Random.#randn
  #unused#::Type{Float64}

Body:
  begin
      return $(Expr(:invoke, LambdaInfo for randn(::MersenneTwister, ::Type{Float64}), :(Base.Random.randn), :(Base.Random.GLOBAL_RNG), :($(Expr(:static_parameter, 1)))))
  end::Float64

There are many packages for which randn() becomes unstable (Primes.jl, Roots.jl, Optim.jl, etc.). The are all precompiled.

[KristofferC]

Is there a plan to deal with this for 5.1? It seems to slowly pop up in more and more places. Can we revert the offending commit? While the commit message says it fixed a performance bug, this is also a pretty significant and unintuitive performance bug.

[timholy]

I may have even seen a dispatch error EDIT: nvm

[tkelman]

closed by #18869, as long as it doesn't break anything in upcoming pkgeval runs

[yuyichao]

Why reopen?

[yuyichao]

(is it not fixed by #18869 ?)

[simonster]

The issue in #20821 is happening on the current release-0.5 branch, even though #18869 was merged months ago.

[simonster]

(Also I am not sure it's quite the same issue, since @code_warntype looks okay there.) It seems likely to have a similar root cause, since the same code is affected.

[yuyichao]

On 0.5.0 it's certainly showing a type instability

[simonster]

On 8d4ef37:

julia> import Primes

julia> f(x) = log(x);

julia> @code_warntype f(1.0)
Variables:
  #self#::#f
  x::Float64

Body:
  begin 
      return $(Expr(:invoke, LambdaInfo for log(::Float64), :(Main.log), :(x)))
  end::Float64

which is the same thing I see if I don't import Primes. But the generated code is still bad, so it seems #18869 did not fully resolve this issue.

[yuyichao]

Just compiled a release-0.5. Seems that the original case has the same issue....

[vtjnash]

dup #265?

[simonster]

Like #265, this is fixed in 0.6, but otherwise I don't really see the connection. AFAICT, f is being compiled wrong the first time.

[KristofferC]

Looks like fixed in 0.6