Manage GC data better

Make sure that all cdata references for variables is tracked. This is
done by attaching finalizers to Expression and Term objects. Some
effort is made to avoid creating tracked temporary objects (esp Terms)
now.

ckiwi/ckiwi.cpp

@@ -10,7 +10,7 @@ using namespace kiwi;
 namespace {
 
 template<typename T, typename R, typename... Args>
-inline R to_cref(Args&&... args) {
+inline R make_cref(Args&&... args) {
    static_assert(
        sizeof(R) >= sizeof(T),  //NOLINT(bugprone-sizeof-expression)
        "to_cref: R too small for T"
@@ -23,13 +23,13 @@ inline R to_cref(Args&&... args) {
 }
 
 template<typename... Args>
-inline decltype(auto) to_var_cref(Args&&... args) {
+inline decltype(auto) make_var_cref(Args&&... args) {
-   return to_cref<Variable, KiwiVarRef>(std::forward<Args>(args)...);
+   return make_cref<Variable, KiwiVarRef>(std::forward<Args>(args)...);
 }
 
 template<typename... Args>
-inline decltype(auto) to_constraint_cref(Args&&... args) {
+inline decltype(auto) make_constraint_cref(Args&&... args) {
-   return to_cref<Constraint, KiwiConstraintRef>(std::forward<Args>(args)...);
+   return make_cref<Constraint, KiwiConstraintRef>(std::forward<Args>(args)...);
 }
 
 template<class T, class R>
@@ -45,7 +45,11 @@ class SharedRef {
        "SharedRef<T,CS> CS too small for T"
    );
 
-   void destroy() {
+   R clone() const {
+      return make_cref<T, R>(cref());
+   }
+
+   void release() {
       if (cref_) {
          ptr()->~T();
          cref_ = nullptr;
@@ -201,11 +205,15 @@ inline KiwiErrPtr wrap_err(P ptr, R ref, F&& f) {
 extern "C" {
 
 KiwiVarRef kiwi_var_new(const char* name) {
-   return to_var_cref(name ? name : "");
+   return make_var_cref(name ? name : "");
 }
 
 void kiwi_var_del(KiwiVarRef var) {
-   VariableRef(var).destroy();
+   VariableRef(var).release();
+}
+
+KiwiVarRef kiwi_var_clone(KiwiVarRef var) {
+   return VariableRef(var).clone();
 }
 
 const char* kiwi_var_name(KiwiVarRef var) {
@@ -252,7 +260,7 @@ kiwi_constraint_new(KiwiExpressionConstPtr expression, enum KiwiRelOp op, double
             terms.emplace_back(var.cref(), t->coefficient);
       }
    }
-   return to_constraint_cref(
+   return make_constraint_cref(
        Expression(std::move(terms), expression->constant),
        static_cast<RelationalOperator>(op),
        strength
@@ -260,7 +268,11 @@ kiwi_constraint_new(KiwiExpressionConstPtr expression, enum KiwiRelOp op, double
 }
 
 void kiwi_constraint_del(KiwiConstraintRef constraint) {
-   ConstraintRef(constraint).destroy();
+   ConstraintRef(constraint).release();
+}
+
+KiwiConstraintRef kiwi_constraint_clone(KiwiConstraintRef constraint) {
+   return ConstraintRef(constraint).clone();
 }
 
 double kiwi_constraint_strength(KiwiConstraintRef constraint) {
@@ -290,7 +302,7 @@ int kiwi_constraint_expression(KiwiConstraintRef constraint, KiwiExpression* out
 
    auto* p = out->terms;
    for (const auto& t : terms) {
-      *p = KiwiTerm {to_var_cref(t.variable()), t.coefficient()};
+      *p = KiwiTerm {make_var_cref(t.variable()), t.coefficient()};
       ++p;
    }
    out->term_count = p - out->terms;

ckiwi/ckiwi.h

@@ -56,20 +56,18 @@ typedef struct KiwiSolver* KiwiSolverPtr;
 
 KiwiVarRef kiwi_var_new(const char* name);
 void kiwi_var_del(KiwiVarRef var);
+KiwiVarRef kiwi_var_clone(KiwiVarRef var);
 
 const char* kiwi_var_name(KiwiVarRef var);
-
 void kiwi_var_set_name(KiwiVarRef var, const char* name);
-
 double kiwi_var_value(KiwiVarRef var);
-
 void kiwi_var_set_value(KiwiVarRef var, double value);
-
 int kiwi_var_eq(KiwiVarRef var, KiwiVarRef other);
 
 KiwiConstraintRef
 kiwi_constraint_new(KiwiExpressionConstPtr expression, enum KiwiRelOp op, double strength);
 void kiwi_constraint_del(KiwiConstraintRef constraint);
+KiwiConstraintRef kiwi_constraint_clone(KiwiConstraintRef constraint);
 
 double kiwi_constraint_strength(KiwiConstraintRef constraint);
 

kiwi.lua

@@ -58,15 +58,12 @@ typedef struct KiwiSolver* KiwiSolverPtr;
 
 KiwiVarRef kiwi_var_new(const char* name);
 void kiwi_var_del(KiwiVarRef var);
+KiwiVarRef kiwi_var_clone(KiwiVarRef var);
 
 const char* kiwi_var_name(KiwiVarRef var);
-
 void kiwi_var_set_name(KiwiVarRef var, const char* name);
-
 double kiwi_var_value(KiwiVarRef var);
-
 void kiwi_var_set_value(KiwiVarRef var, double value);
-
 int kiwi_var_eq(KiwiVarRef var, KiwiVarRef other);
 
 KiwiConstraintRef
@@ -99,8 +96,8 @@ void free(void *);
 ]])
 
 local strformat = string.format
-local ffi_cast, ffi_copy, ffi_gc, ffi_istype, ffi_new, ffi_string =
+local ffi_cast, ffi_gc, ffi_istype, ffi_new, ffi_string =
-   ffi.cast, ffi.copy, ffi.gc, ffi.istype, ffi.new, ffi.string
+   ffi.cast, ffi.gc, ffi.istype, ffi.new, ffi.string
 
 local concat = table.concat
 local has_table_new, new_tab = pcall(require, "table.new")
@@ -164,9 +161,6 @@ kiwi.Expression = Expression
 local Constraint = ffi.typeof("struct KiwiConstraintRefType") --[[@as kiwi.Constraint]]
 kiwi.Constraint = Constraint
 
--- JIT compiler NYI: bad argument type if ffi.sizeof is used with a structure member
-local SIZEOF_TERM = ffi.sizeof(Term)
-
 --- Define a constraint with expressions as `a <= b`.
 ---@param a kiwi.Expression|kiwi.Term|kiwi.Var|number
 ---@param b kiwi.Expression|kiwi.Term|kiwi.Var|number
@@ -194,39 +188,63 @@ function kiwi.eq(a, b, strength)
    return Constraint(a - b, "EQ", strength)
 end
 
+local function expr_gc(expr)
+   local terms_ = expr.terms_
+   for i = 0, expr.term_count - 1 do
+      ckiwi.kiwi_var_del(terms_[i].var)
+   end
+end
+
 ---@param expr kiwi.Expression
----@param term kiwi.Term
+---@param var kiwi.Var
+---@param coeff number?
 ---@nodiscard
-local function add_expr_term(expr, term)
+local function add_expr_term(expr, var, coeff)
-   local ret = ffi_new(Expression, expr.term_count + 1) --[[@as kiwi.Expression]]
+   local ret = ffi_gc(ffi_new(Expression, expr.term_count + 1), expr_gc) --[[@as kiwi.Expression]]
+   for i = 0, expr.term_count - 1 do
+      local st = expr.terms_[i] --[[@as kiwi.Term]]
+      local dt = ret.terms_[i] --[[@as kiwi.Term]]
+      dt.var = ckiwi.kiwi_var_clone(st.var)
+      dt.coefficient = st.coefficient
+   end
+   local dt = ret.terms_[expr.term_count]
+   dt.var = ckiwi.kiwi_var_clone(var)
+   dt.coefficient = coeff or 1.0
    ret.constant = expr.constant
    ret.term_count = expr.term_count + 1
-   ffi_copy(ret.terms_, expr.terms_, SIZEOF_TERM * expr.term_count) ---@diagnostic disable-line: param-type-mismatch
-   ret.terms_[expr.term_count] = term
    return ret
 end
 
 ---@param constant number
----@param term kiwi.Term
+---@param var kiwi.Var
+---@param coeff number?
 ---@nodiscard
-local function new_expr_one(constant, term)
+local function new_expr_one(constant, var, coeff)
-   local ret = ffi_new(Expression, 1) --[[@as kiwi.Expression]]
+   local ret = ffi_gc(ffi_new(Expression, 1), expr_gc) --[[@as kiwi.Expression]]
+   local dt = ret.terms_[0]
+   dt.var = ckiwi.kiwi_var_clone(var)
+   dt.coefficient = coeff or 1.0
    ret.constant = constant
    ret.term_count = 1
-   ret.terms_[0] = term
    return ret
 end
 
 ---@param constant number
----@param term1 kiwi.Term
+---@param var1 kiwi.Var
----@param term2 kiwi.Term
+---@param var2 kiwi.Var
+---@param coeff1 number?
+---@param coeff2 number?
 ---@nodiscard
-local function new_expr_pair(constant, term1, term2)
+local function new_expr_pair(constant, var1, var2, coeff1, coeff2)
-   local ret = ffi_new(Expression, 2) --[[@as kiwi.Expression]]
+   local ret = ffi_gc(ffi_new(Expression, 2), expr_gc) --[[@as kiwi.Expression]]
+   local dt = ret.terms_[0]
+   dt.var = ckiwi.kiwi_var_clone(var1)
+   dt.coefficient = coeff1 or 1.0
+   dt = ret.terms_[1]
+   dt.var = ckiwi.kiwi_var_clone(var2)
+   dt.coefficient = coeff2 or 1.0
    ret.constant = constant
    ret.term_count = 2
-   ret.terms_[0] = term1
-   ret.terms_[1] = term2
    return ret
 end
 
@@ -275,6 +293,7 @@ ffi.metatype(Var, {
    __new = function(_, name)
       return ffi_gc(ckiwi.kiwi_var_new(name), ckiwi.kiwi_var_del)
    end,
+   __gc = ckiwi.kiwi_var_del,
 
    __mul = function(a, b)
       if type(a) == "number" then
@@ -296,18 +315,17 @@ ffi.metatype(Var, {
 
    __add = function(a, b)
       if ffi_istype(Var, b) then
-         local bt = Term(b)
          if type(a) == "number" then
-            return new_expr_one(a, bt)
+            return new_expr_one(a, b)
          else
-            return new_expr_pair(0.0, Term(a), bt)
+            return new_expr_pair(0.0, a, b)
          end
       elseif ffi_istype(Term, b) then
-         return new_expr_pair(0.0, b, Term(a))
+         return new_expr_pair(0.0, b.var, a, b.coefficient)
       elseif ffi_istype(Expression, b) then
-         return add_expr_term(b, Term(a))
+         return add_expr_term(b, a)
       elseif type(b) == "number" then
-         return new_expr_one(b, Term(a))
+         return new_expr_one(b, a)
       end
       error("Invalid var +")
    end,
@@ -325,8 +343,8 @@ ffi.metatype(Var, {
 --- Each term is a variable multiplied by a constant coefficient (default 1.0).
 ---@class kiwi.Term: ffi.ctype*
 ---@overload fun(var: kiwi.Var, coefficient: number?): kiwi.Term
----@field coefficient number
 ---@field var kiwi.Var
+---@field coefficient number
 ---@operator mul(number): kiwi.Term
 ---@operator div(number): kiwi.Term
 ---@operator unm: kiwi.Term
@@ -347,14 +365,16 @@ end
 ---@param constant number?
 ---@return kiwi.Expression
 function Term_cls:toexpr(constant)
-   return new_expr_one(constant or 0.0, self)
+   return new_expr_one(constant or 0.0, self.var, self.coefficient)
 end
 
 ffi.metatype(Term, {
    __index = Term_cls,
 
    __new = function(_, var, coefficient)
-      return ffi_new(Term, var, coefficient or 1.0)
+      return ffi_gc(ffi_new(Term, ckiwi.kiwi_var_clone(var), coefficient or 1.0), function(term)
+         ckiwi.kiwi_var_del(term.var)
+      end)
    end,
 
    __mul = function(a, b)
@@ -377,17 +397,17 @@ ffi.metatype(Term, {
 
    __add = function(a, b)
       if ffi_istype(Var, b) then
-         return new_expr_pair(0.0, a, Term(b))
+         return new_expr_pair(0.0, a.var, b, a.coefficient)
       elseif ffi_istype(Term, b) then
          if type(a) == "number" then
-            return new_expr_one(a, b)
+            return new_expr_one(a, b.var, b.coefficient)
          else
-            return new_expr_pair(0.0, a, b)
+            return new_expr_pair(0.0, a.var, b.var, a.coefficient, b.coefficient)
          end
       elseif ffi_istype(Expression, b) then
-         return add_expr_term(b, a)
+         return add_expr_term(b, a.var, a.coefficient)
       elseif type(b) == "number" then
-         return new_expr_one(b, a)
+         return new_expr_one(b, a.var, a.coefficient)
       end
       error("Invalid term + op")
    end,
@@ -397,8 +417,7 @@ ffi.metatype(Term, {
    end,
 
    __tostring = function(term)
-      return tostring(term.var:name())
+      return tostring(term.coefficient) .. " " .. term.var:name()
-      --return tostring(term.coefficient) .. " " .. term.var:name()
    end,
 })
 
@@ -407,10 +426,15 @@ do
    ---@param constant number
    ---@nodiscard
    local function mul_expr_constant(expr, constant)
-      local ret = ffi_new(Expression, expr.term_count, expr.constant * constant, expr.term_count) --[[@as kiwi.Expression]]
+      local ret = ffi_gc(ffi_new(Expression, expr.term_count), expr_gc) --[[@as kiwi.Expression]]
       for i = 0, expr.term_count - 1 do
-         ret.terms_[i] = ffi_new(Term, expr.terms_[i].var, expr.terms_[i].coefficient * constant) --[[@as kiwi.Term]]
+         local st = expr.terms_[i] --[[@as kiwi.Term]]
+         local dt = ret.terms_[i] --[[@as kiwi.Term]]
+         dt.var = ckiwi.kiwi_var_clone(st.var)
+         dt.coefficient = st.coefficient * constant
       end
+      ret.constant = expr.constant * constant
+      ret.term_count = expr.term_count
       return ret
    end
 
@@ -418,14 +442,24 @@ do
    ---@param b kiwi.Expression
    ---@nodiscard
    local function add_expr_expr(a, b)
-      local ret = ffi_new(
+      local a_count = a.term_count
-         Expression,
+      local b_count = b.term_count
-         a.term_count + b.term_count,
+      local ret = ffi_gc(ffi_new(Expression, a_count + b_count), expr_gc) --[[@as kiwi.Expression]]
-         a.constant + b.constant,
+
-         a.term_count + b.term_count
+      for i = 0, a_count - 1 do
-      ) --[[@as kiwi.Expression]]
+         local dt = ret.terms_[i] --[[@as kiwi.Term]]
-      ffi_copy(ret.terms_, a.terms_, SIZEOF_TERM * a.term_count) ---@diagnostic disable-line: param-type-mismatch
+         local st = a.terms_[i] --[[@as kiwi.Term]]
-      ffi_copy(ret.terms_[a.term_count], b.terms_, SIZEOF_TERM * b.term_count) ---@diagnostic disable-line: param-type-mismatch
+         dt.var = ckiwi.kiwi_var_clone(st.var)
+         dt.coefficient = st.coefficient
+      end
+      for i = 0, b_count - 1 do
+         local dt = ret.terms_[a_count + i] --[[@as kiwi.Term]]
+         local st = b.terms_[i] --[[@as kiwi.Term]]
+         dt.var = ckiwi.kiwi_var_clone(st.var)
+         dt.coefficient = st.coefficient
+      end
+      ret.constant = a.constant + b.constant
+      ret.term_count = a_count + b_count
       return ret
    end
 
@@ -433,11 +467,16 @@ do
    ---@param constant number
    ---@nodiscard
    local function new_expr_constant(expr, constant)
-      local ret = ffi_new(Expression, expr.term_count) --[[@as kiwi.Expression]]
+      local ret = ffi_gc(ffi_new(Expression, expr.term_count), expr_gc) --[[@as kiwi.Expression]]
+
+      for i = 0, expr.term_count - 1 do
+         local dt = ret.terms_[i] --[[@as kiwi.Term]]
+         local st = expr.terms_[i] --[[@as kiwi.Term]]
+         dt.var = ckiwi.kiwi_var_clone(st.var)
+         dt.coefficient = st.coefficient
+      end
       ret.constant = constant
       ret.term_count = expr.term_count
-
-      ffi_copy(ret.terms_, expr.terms_, SIZEOF_TERM * expr.term_count) ---@diagnostic disable-line: param-type-mismatch
       return ret
    end
 
@@ -473,7 +512,8 @@ do
    function Expression_cls:terms()
       local terms = new_tab(self.term_count, 0)
       for i = 0, self.term_count - 1 do
-         terms[i + 1] = self.terms_[i]
+         local t = self.terms_[i] --[[@as kiwi.Term]]
+         terms[i + 1] = Term(t.var, t.coefficient)
       end
       return terms
    end
@@ -488,7 +528,15 @@ do
       __index = Expression_cls,
 
       __new = function(_, terms, constant)
-         return ffi_new(Expression, #terms, constant or 0.0, #terms, terms)
+         local e = ffi_gc(ffi_new(Expression, #terms), expr_gc) --[[@as kiwi.Expression]]
+         for i, t in ipairs(terms) do
+            local dt = e.terms_[i - 1] --[[@as kiwi.Term]]
+            dt.var = ckiwi.kiwi_var_clone(t.var)
+            dt.coefficient = t.coefficient
+         end
+         e.constant = constant or 0.0
+         e.term_count = #terms
+         return e
       end,
 
       __mul = function(a, b)
@@ -511,7 +559,7 @@ do
 
       __add = function(a, b)
          if ffi_istype(Var, b) then
-            return add_expr_term(a, Term(b))
+            return add_expr_term(a, b)
          elseif ffi_istype(Expression, b) then
             if type(a) == "number" then
                return new_expr_constant(b, a + b.constant)
@@ -519,7 +567,7 @@ do
                return add_expr_expr(a, b)
             end
          elseif ffi_istype(Term, b) then
-            return add_expr_term(a, b)
+            return add_expr_term(a, b.var, b.coefficient)
          elseif type(b) == "number" then
             return new_expr_constant(a, a.constant + b)
          end
@@ -569,9 +617,9 @@ function Constraint_cls:expression()
    local n = ckiwi.kiwi_constraint_expression(self, expr, SZ)
    if n > SZ then
       expr = ffi_new(Expression, n) --[[@as kiwi.Expression]]
-      ckiwi.kiwi_constraint_expression(self, expr, n)
+      n = ckiwi.kiwi_constraint_expression(self, expr, n)
    end
-   return expr
+   return ffi_gc(expr, expr_gc) --[[@as kiwi.Expression]] ---@diagnostic disable-line: param-type-mismatch
 end
 
 --- Create a constraint between a pair of variables with ratio.
@@ -586,11 +634,7 @@ end
 ---@nodiscard
 function kiwi.new_pair_ratio_constraint(left, coeff, right, constant, op, strength)
    assert(ffi_istype(Var, left) and ffi_istype(Var, right))
-   return Constraint(
+   return Constraint(new_expr_pair(-(constant or 0.0), right, left, -coeff), op, strength)
-      new_expr_pair(-(constant or 0.0), Term(left), Term(right, -coeff)),
-      op,
-      strength
-   )
 end
 
 --- Create a constraint between a pair of variables with ratio.
@@ -604,11 +648,7 @@ end
 ---@nodiscard
 function kiwi.new_pair_constraint(left, right, constant, op, strength)
    assert(ffi_istype(Var, left) and ffi_istype(Var, right))
-   return Constraint(
+   return Constraint(new_expr_pair(-(constant or 0.0), right, left, -1.0), op, strength)
-      new_expr_pair(-(constant or 0.0), Term(left), Term(right, -1.0)),
-      op,
-      strength
-   )
 end
 
 --- Create a single term constraint
@@ -621,7 +661,7 @@ end
 ---@nodiscard
 function kiwi.new_single_constraint(var, constant, op, strength)
    assert(ffi_istype(Var, var))
-   return Constraint(new_expr_one(-(constant or 0.0), Term(var)), op, strength)
+   return Constraint(new_expr_one(-(constant or 0.0), var), op, strength)
 end
 
 local Strength = kiwi.Strength
@@ -701,7 +741,6 @@ local function try_solver(f, solver, item, ...)
       local kind = err.kind
       local message = err.message ~= nil and ffi_string(err.message) or ""
       if err.must_free then
-         print("FEEE")
          C.free(err)
       end
       error(new_error(kind, message, solver, item))