JuliaLang / julia

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

[release-1.6] First time printing is slow #39349

Closed mforets closed 2 years ago

mforets commented 3 years ago

This may be a Julia issue, reported in:

[mforets@localhost bin]$ ./julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.6.0-beta1 (2021-01-08)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> @time using Polyhedra
  2.419120 seconds (6.69 M allocations: 486.807 MiB, 3.52% gc time, 0.41% compilation time)

julia> function foo()
           A = [1. 1; 1 -1; -1 0]
           b = [1, 0, 0]
           rep = hrep(A, b)
           plib = default_library(2, Float64)
           p = polyhedron(rep, plib)
       end
foo (generic function with 1 method)

julia> @time p = foo();
  0.022039 seconds (46.42 k allocations: 2.957 MiB, 99.94% compilation time)

julia> p # long time
Polyhedron DefaultPolyhedron{Float64, MixedMatHRep{Float64, Matrix{Float64}}, MixedMatVRep{Float64, Matrix{Float64}}}:
3-element iterator of HalfSpace{Float64, Vector{Float64}}:
 HalfSpace([1.0, 1.0], 1.0)
 HalfSpace([1.0, -1.0], 0.0)
 HalfSpace([-1.0, 0.0], 0.0)

There was a similar issue in Julia v1.01 (https://github.com/JuliaPolyhedra/Polyhedra.jl/issues/128) and it was fixed in https://github.com/JuliaLang/julia/pull/30010, but the problem seems to have come back.

CC: @blegat, @schillic

JeffBezanson commented 3 years ago

You're not kidding, that is a REALLY long time. I waited several minutes and it didn't finish. Finishes instantly with --compile=min though.

JeffBezanson commented 3 years ago

Ok, 6 minutes in 1.5, 29 minutes on master...

JeffBezanson commented 3 years ago

Got some interesting information on this. The regression happened in roughly 2 steps. One big one is #27843, which is not surprising since it widens types more slowly.

The other one was caused, believe it or not, by changing VERSION to 1.7.0-DEV. I imagine that would be due to version checks in packages enabling or disabling some code. All the time there is in METHOD_MATCH, so perhaps some extra method signatures are putting us on a bad path. The same regression can be triggered prior to 1.7.0-DEV by upgrading from

  [d8a4904e] MutableArithmetics v0.1.0
  [67491407] Polyhedra v0.5.8

to

  [d8a4904e] MutableArithmetics v0.2.14
  [67491407] Polyhedra v0.6.12

so perhaps that involves similar code changes.

vtjnash commented 3 years ago

Should we revert #27843, or is it more probable we can fix an underlying issue and get this to be much faster?

JeffBezanson commented 3 years ago

Yeah, I'm inclined to revert it, but I might try a couple variations of that logic as well.

mforets commented 3 years ago

thanks a lot for promptly addressing this issue!

blegat commented 3 years ago

Thanks a lot ! I have just tried this on master and

julia> p
Polyhedron DefaultPolyhedron{Float64, MixedMatHRep{Float64, Matrix{Float64}}, MixedMatVRep{Float64, Matrix{Float64}}}:
3-element iterator of HalfSpace{Float64, Vector{Float64}}:
 HalfSpace([1.0, 1.0], 1.0)
 HalfSpace([1.0, -1.0], 0.0)
 HalfSpace([-1.0, 0.0], 0.0)

only took a few seconds. I thought it would only remove one of the regressions from the 6 minutes to the 29 minutes of master so I was expecting the time to be between 6 and 29 minutes. But instead, it seems to not only have solved the regression but also the issue (that 6 minutes was already too slow) with Julia v1.5 since it only took a few seconds. Is that expected or am I not understanding this correctly ?

KristofferC commented 3 years ago

Reopening since https://github.com/JuliaLang/julia/pull/39406 couldn't have fixed this since the commit it reverts is not on the 1.6 beta.

ViralBShah commented 2 years ago

Works in a couple of seconds now.