Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

type stability of pm_Integer et al #36

Open
kalmarek opened this issue Oct 25, 2018 · 2 comments
Open

type stability of pm_Integer et al #36

kalmarek opened this issue Oct 25, 2018 · 2 comments
Labels
question Further information is requested

Comments

@kalmarek
Copy link
Contributor

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

@saschatimme
Copy link
Collaborator

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.

@kalmarek
Copy link
Contributor Author

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 kalmarek added the question Further information is requested label Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants