From 84a01179cdefba0b3681ee2d859684ff64661710 Mon Sep 17 00:00:00 2001 From: "John K. Luebs" Date: Tue, 13 Feb 2024 20:55:37 -0600 Subject: [PATCH] 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 | 34 ++++++--- ckiwi/ckiwi.h | 6 +- kiwi.lua | 181 +++++++++++++++++++++++++++++------------------- 3 files changed, 135 insertions(+), 86 deletions(-) diff --git a/ckiwi/ckiwi.cpp b/ckiwi/ckiwi.cpp index 9a8c989..f888123 100644 --- a/ckiwi/ckiwi.cpp +++ b/ckiwi/ckiwi.cpp @@ -10,7 +10,7 @@ using namespace kiwi; namespace { template -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 -inline decltype(auto) to_var_cref(Args&&... args) { - return to_cref(std::forward(args)...); +inline decltype(auto) make_var_cref(Args&&... args) { + return make_cref(std::forward(args)...); } template -inline decltype(auto) to_constraint_cref(Args&&... args) { - return to_cref(std::forward(args)...); +inline decltype(auto) make_constraint_cref(Args&&... args) { + return make_cref(std::forward(args)...); } template @@ -45,7 +45,11 @@ class SharedRef { "SharedRef CS too small for T" ); - void destroy() { + R clone() const { + return make_cref(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(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; diff --git a/ckiwi/ckiwi.h b/ckiwi/ckiwi.h index 8897374..e63ff7d 100644 --- a/ckiwi/ckiwi.h +++ b/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); diff --git a/kiwi.lua b/kiwi.lua index 3cc9322..61c0a03 100644 --- a/kiwi.lua +++ b/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 = - 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.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 ret = ffi_new(Expression, expr.term_count + 1) --[[@as kiwi.Expression]] +local function add_expr_term(expr, var, coeff) + 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 ret = ffi_new(Expression, 1) --[[@as kiwi.Expression]] +local function new_expr_one(constant, var, coeff) + 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 term2 kiwi.Term +---@param var1 kiwi.Var +---@param var2 kiwi.Var +---@param coeff1 number? +---@param coeff2 number? ---@nodiscard -local function new_expr_pair(constant, term1, term2) - local ret = ffi_new(Expression, 2) --[[@as kiwi.Expression]] +local function new_expr_pair(constant, var1, var2, coeff1, coeff2) + 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( - Expression, - a.term_count + b.term_count, - a.constant + b.constant, - a.term_count + b.term_count - ) --[[@as kiwi.Expression]] - ffi_copy(ret.terms_, a.terms_, SIZEOF_TERM * a.term_count) ---@diagnostic disable-line: param-type-mismatch - ffi_copy(ret.terms_[a.term_count], b.terms_, SIZEOF_TERM * b.term_count) ---@diagnostic disable-line: param-type-mismatch + local a_count = a.term_count + local b_count = b.term_count + local ret = ffi_gc(ffi_new(Expression, a_count + b_count), expr_gc) --[[@as kiwi.Expression]] + + for i = 0, a_count - 1 do + local dt = ret.terms_[i] --[[@as kiwi.Term]] + local st = a.terms_[i] --[[@as kiwi.Term]] + 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( - new_expr_pair(-(constant or 0.0), Term(left), Term(right, -coeff)), - op, - strength - ) + return Constraint(new_expr_pair(-(constant or 0.0), right, left, -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( - new_expr_pair(-(constant or 0.0), Term(left), Term(right, -1.0)), - op, - strength - ) + return Constraint(new_expr_pair(-(constant or 0.0), right, left, -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))