Polymake.jl icon indicating copy to clipboard operation
Polymake.jl copied to clipboard

type stability of pm_Integer et al

Open kalmarek opened this issue 7 years ago • 2 comments

Do something about return type instability, i.e.

julia> @code_warntype pm_Integer(3)
Body::Any
 1 ─ %1 = invoke Base.getindex(Polymake.__cxxwrap_pointers::Array{Ptr{Nothing},1}, 26::Int64)::Ptr{Nothing}
 │   %2 = invoke Base.getindex(Polymake.__cxxwrap_pointers::Array{Ptr{Nothing},1}, 25::Int64)::Ptr{Nothing}
 │   %3 = $(Expr(:foreigncall, :(%2), Any, svec(Ptr{Nothing}, Int64), :(:ccall), 2, :(%1), :(arg1), :(arg1), :(%1)))::Any
 └──      return %3                                                                          │

(note: this doesn't change if pm_Integer subtypes Base.Integer or not

kalmarek avatar Oct 25 '18 10:10 kalmarek

I looked into the Julia ccall documentation and the problem ist on the CxxWrap side. ccall can only handle simpel c types, everything else will be Any. So the question is whether CxxWrap has enough type information during the code gen step to add a type annotation.

saschatimme avatar Oct 26 '18 15:10 saschatimme

on CxxWrap-v0.10.1:

julia> @code_warntype Polymake.Integer(3)
Variables
  #self#::Core.Compiler.Const(Polymake.Integer, false)
  arg1::Int64

Body::Polymake.IntegerAllocated
1 ─ %1  = Polymake.IntegerAllocated::Core.Compiler.Const(Polymake.IntegerAllocated, false)
│   %2  = Core.apply_type(Polymake.Ptr, Polymake.Cvoid)::Core.Compiler.Const(Ptr{Nothing}, false)
│   %3  = Base.getindex(Polymake.__cxxwrap_pointers, 41)::Tuple{Ptr{Nothing},Ptr{Nothing},Bool}
│   %4  = Base.getindex(%3, 2)::Ptr{Nothing}
│   %5  = Base.cconvert(%2, %4)::Ptr{Nothing}
│   %6  = Base.cconvert(Int64, arg1)::Int64
│   %7  = Base.getindex(Polymake.__cxxwrap_pointers, 41)::Tuple{Ptr{Nothing},Ptr{Nothing},Bool}
│   %8  = Base.getindex(%7, 1)::Ptr{Nothing}
│   %9  = Core.apply_type(Polymake.Ptr, Polymake.Cvoid)::Core.Compiler.Const(Ptr{Nothing}, false)
│   %10 = Base.unsafe_convert(%9, %5)::Ptr{Nothing}
│   %11 = Base.unsafe_convert(Int64, %6)::Int64
│   %12 = $(Expr(:foreigncall, :(%8), Any, svec(Ptr{Nothing}, Int64), 0, :(:ccall), :(%10), :(%11), :(%6), :(%5)))::Any
│   %13 = Base.convert(%1, %12)::Any
│   %14 = Core.typeassert(%13, %1)::Polymake.IntegerAllocated
└──       return %14

and benchmarks:

julia> @benchmark Polymake.Integer(3)
BenchmarkTools.Trial: 
  memory estimate:  40 bytes
  allocs estimate:  3
  --------------
  minimum time:     184.470 ns (0.00% GC)
  median time:      213.161 ns (0.00% GC)
  mean time:        378.662 ns (13.06% GC)
  maximum time:     324.806 μs (40.88% GC)
  --------------
  samples:          10000
  evals/sample:     555

julia> @benchmark Polymake.Integer(3) + 3
BenchmarkTools.Trial: 
  memory estimate:  88 bytes
  allocs estimate:  7
  --------------
  minimum time:     452.398 ns (0.00% GC)
  median time:      486.000 ns (0.00% GC)
  mean time:        745.274 ns (8.85% GC)
  maximum time:     938.657 μs (28.77% GC)
  --------------
  samples:          10000
  evals/sample:     196

julia> @benchmark BigInt(3)
BenchmarkTools.Trial: 
  memory estimate:  40 bytes
  allocs estimate:  2
  --------------
  minimum time:     53.896 ns (0.00% GC)
  median time:      67.135 ns (0.00% GC)
  mean time:        170.517 ns (38.25% GC)
  maximum time:     190.757 μs (78.46% GC)
  --------------
  samples:          10000
  evals/sample:     984

julia> @benchmark BigInt(3) + 3
BenchmarkTools.Trial: 
  memory estimate:  88 bytes
  allocs estimate:  5
  --------------
  minimum time:     147.720 ns (0.00% GC)
  median time:      159.803 ns (0.00% GC)
  mean time:        366.945 ns (32.75% GC)
  maximum time:     222.718 μs (79.13% GC)
  --------------
  samples:          10000
  evals/sample:     811

about 3× slower than BigInt

kalmarek avatar Apr 22 '20 11:04 kalmarek