Fix inference issue in MOIU.Model

[odow]

Closes #1255

Before

julia> function plain(N)
           model = MOIU.Model{Float64}()
           x = MOI.add_variables(model, N)
           return x
       end
plain (generic function with 1 method)

julia> function with_bounds(N)
           model = MOIU.Model{Float64}()
           x = MOI.add_variables(model, N)
           for xi in x
               MOI.add_constraint(model, MOI.SingleVariable(xi), MOI.GreaterThan(0.0))
           end
           return x
       end
with_bounds (generic function with 1 method)

julia> @btime plain(1_000_000);
  37.242 ms (175 allocations: 27.64 MiB)

julia> @btime with_bounds(1_000_000);
  95.231 ms (175 allocations: 27.64 MiB)

After

julia> @btime plain(1_000_000);
  37.303 ms (175 allocations: 27.64 MiB)

julia> @btime with_bounds(1_000_000);
  39.206 ms (175 allocations: 27.64 MiB)

[odow]

And proof from the original JuMP issue: #1255.

julia> @btime plain(1_000_000);
  99.679 ms (2000244 allocations: 142.09 MiB)

julia> @btime with_int_and_bounds(1_000_000);
  117.368 ms (2000244 allocations: 142.09 MiB)

[mlubin]

Nice!

[blegat]

This is awesome! However I don't quite understand what's happening. In both case there is the type unstability but in both cases we do not run the type unstable code. In the first case, the type unstable code is inside an if statement and in the second case, it's inside a separate function. However, looking at the output of @code_warntype, it seems to be inlined so why is that different ?

With these hints, type inference succeeds

The inference of which type ? The type of S1 is still not inferrable, the type of which variable is now inferrable ?

[odow]

oops. I didn't update the head of the PR. My original solution (annotating with ::Type{<:AbstractSet}) caused a weird error when it actually got thrown, so I used a function barrier and it seemed to do the same job.

[odow]

Here's what happens now:

julia> model = MOIU.Model{Float64}()
MOIU.Model{Float64}

julia> x = MOI.add_variable(model)
MathOptInterface.VariableIndex(1)

julia> @code_warntype MOIU.throw_if_upper_bound_set(x, MOI.LessThan{Float64}, 0x0, Float64)
Variables
  #self#::Core.Compiler.Const(MathOptInterface.Utilities.throw_if_upper_bound_set, false)
  variable::MathOptInterface.VariableIndex
  S2::Core.Compiler.Const(MathOptInterface.LessThan{Float64}, false)
  mask::UInt8
  T::Core.Compiler.Const(Float64, false)
  upper_mask::UInt8

Body::Nothing
1 ─      (upper_mask = mask & MathOptInterface.Utilities.UPPER_BOUND_MASK)
│   %2 = MathOptInterface.Utilities.iszero(upper_mask)::Bool
└──      goto #3 if not %2
2 ─      return
3 ─ %5 = MathOptInterface.Utilities.single_variable_flag(S2)::Core.Compiler.Const(0x04, false)
│   %6 = (%5 & MathOptInterface.Utilities.UPPER_BOUND_MASK)::Core.Compiler.Const(0x04, false)
│   %7 = MathOptInterface.Utilities.iszero(%6)::Core.Compiler.Const(false, false)
└──      goto #5 if not %7
4 ─      Core.Compiler.Const(:(return), false)
5 ┄      MathOptInterface.Utilities._throw_if_upper_bound_set(variable, S2, upper_mask, T)
└──      Core.Compiler.Const(:(return %10), false)

The _throw method still infers badly, but we only hit that if an error is thrown, so we don't really care.

[blegat]

That makes more sense. Is there a way to avoid the function being inlined? It's not being inlined now but it might be in a future Julia version

[odow]

My guess is that Julia would only inline if it infers nicely. I don't think we need to do anything explicitly here. It's something we can check in a future release.

[blegat]

If it stops inferring we won't notice as we have no tests. We could use @noinline