From 7b92f71de549ed1ad617105090e97422f877778f Mon Sep 17 00:00:00 2001 From: Eric Kreutzer Date: Fri, 3 Jul 2026 17:23:35 -0600 Subject: [PATCH] Release GVL during routing solves when no Ruby callbacks are registered Ruby-proc transit callbacks re-enter Ruby mid-search, so the solve bindings hold the GVL. Models built purely from transit matrices and vectors never call back into Ruby; for those, wrap SolveWithParameters and SolveFromAssignmentWithParameters in rb_thread_call_without_gvl so long solves stop blocking every other thread in the process. Tracking lives in RoutingModel Ruby wrappers: registering a Ruby transit callback keeps solves on the GVL-holding path. Public API is unchanged. --- ext/or-tools/routing.cpp | 47 ++++++++++++++--- lib/or_tools/routing_model.rb | 26 ++++++++++ test/routing_gvl_test.rb | 97 +++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 test/routing_gvl_test.rb diff --git a/ext/or-tools/routing.cpp b/ext/or-tools/routing.cpp index ecd8c2d..b2406c3 100644 --- a/ext/or-tools/routing.cpp +++ b/ext/or-tools/routing.cpp @@ -5,6 +5,7 @@ #include #include #include +#include using operations_research::Assignment; using operations_research::ConstraintSolverParameters; @@ -20,6 +21,24 @@ using operations_research::RoutingNodeIndex; using operations_research::RoutingSearchParameters; using operations_research::RoutingSearchStatus; + +namespace { + struct RoutingSolveArgs { + RoutingModel* model; + const Assignment* assignment; + const RoutingSearchParameters* parameters; + const Assignment* result; + }; + + void* routing_solve_without_gvl(void* data) { + auto* args = static_cast(data); + args->result = args->assignment + ? args->model->SolveFromAssignmentWithParameters(args->assignment, *args->parameters) + : args->model->SolveWithParameters(*args->parameters); + return nullptr; + } +} + using Rice::Array; using Rice::Class; using Rice::Module; @@ -332,7 +351,7 @@ void init_routing(Rice::Module& m) { .define_constructor(Rice::Constructor(), Rice::Arg("_index_manager"), Rice::Arg("_parameters") = operations_research::DefaultRoutingModelParameters()) .define_method("register_unary_transit_vector", &RoutingModel::RegisterUnaryTransitVector) .define_method( - "register_unary_transit_callback", + "_register_unary_transit_callback", [](RoutingModel& self, Object callback) { // TODO guard callback? return self.RegisterUnaryTransitCallback( @@ -347,7 +366,7 @@ void init_routing(Rice::Module& m) { }, Rice::Arg("_callback").keepAlive()) .define_method("register_transit_matrix", &RoutingModel::RegisterTransitMatrix) .define_method( - "register_transit_callback", + "_register_transit_callback", [](RoutingModel& self, Object callback) { // TODO guard callback? return self.RegisterTransitCallback( @@ -430,14 +449,26 @@ void init_routing(Rice::Module& m) { .define_method("close_model", &RoutingModel::CloseModel) // solve defined in Ruby .define_method( - "solve_with_parameters", - [](RoutingModel& self, const RoutingSearchParameters& search_parameters) { - return self.SolveWithParameters(search_parameters); + "_solve_with_parameters", + [](RoutingModel& self, const RoutingSearchParameters& search_parameters, bool release_gvl) { + if (!release_gvl) { + return self.SolveWithParameters(search_parameters); + } + + RoutingSolveArgs args{&self, nullptr, &search_parameters, nullptr}; + rb_thread_call_without_gvl(routing_solve_without_gvl, &args, RUBY_UBF_IO, nullptr); + return args.result; }) .define_method( - "solve_from_assignment_with_parameters", - [](RoutingModel& self, const Assignment& assignment, const RoutingSearchParameters& search_parameters) { - return self.SolveFromAssignmentWithParameters(&assignment, search_parameters); + "_solve_from_assignment_with_parameters", + [](RoutingModel& self, const Assignment& assignment, const RoutingSearchParameters& search_parameters, bool release_gvl) { + if (!release_gvl) { + return self.SolveFromAssignmentWithParameters(&assignment, search_parameters); + } + + RoutingSolveArgs args{&self, &assignment, &search_parameters, nullptr}; + rb_thread_call_without_gvl(routing_solve_without_gvl, &args, RUBY_UBF_IO, nullptr); + return args.result; }) .define_method("compute_lower_bound", &RoutingModel::ComputeLowerBound) .define_method("status", diff --git a/lib/or_tools/routing_model.rb b/lib/or_tools/routing_model.rb index 8ad06b0..c754b19 100644 --- a/lib/or_tools/routing_model.rb +++ b/lib/or_tools/routing_model.rb @@ -21,5 +21,31 @@ def solve( def add_disjunction(indices, penalty, max_cardinality = 1, penalty_cost_behavior = :penalize_once) _add_disjunction(indices, penalty, max_cardinality, penalty_cost_behavior) end + + # Ruby-proc transit callbacks re-enter Ruby during the solve, so the GVL + # can only be released when a model registers none of them. + def register_transit_callback(callback) + @ruby_transit_callbacks = true + _register_transit_callback(callback) + end + + def register_unary_transit_callback(callback) + @ruby_transit_callbacks = true + _register_unary_transit_callback(callback) + end + + def solve_with_parameters(search_parameters) + _solve_with_parameters(search_parameters, !ruby_transit_callbacks?) + end + + def solve_from_assignment_with_parameters(assignment, search_parameters) + _solve_from_assignment_with_parameters(assignment, search_parameters, !ruby_transit_callbacks?) + end + + private + + def ruby_transit_callbacks? + defined?(@ruby_transit_callbacks) && @ruby_transit_callbacks + end end end diff --git a/test/routing_gvl_test.rb b/test/routing_gvl_test.rb new file mode 100644 index 0000000..fa3875a --- /dev/null +++ b/test/routing_gvl_test.rb @@ -0,0 +1,97 @@ +require_relative "test_helper" + +class RoutingGvlTest < Minitest::Test + def test_matrix_model_releases_gvl_during_solve + routing, manager = matrix_model + counter = 0 + ticker = Thread.new { loop { counter += 1 } } + + assignment = routing.solve(time_limit: 1, first_solution_strategy: :path_cheapest_arc, local_search_metaheuristic: :guided_local_search) + + ticker.kill + assert assignment + assert_operator assignment.objective_value, :>, 0 + assert_operator counter, :>, 1_000, "expected ticker thread to run while solving" + ensure + ticker&.kill + end + + def test_callback_model_keeps_gvl_and_solves + routing, manager = callback_model + counter = 0 + ticker = Thread.new { loop { counter += 1 } } + + assignment = routing.solve(time_limit: 1, first_solution_strategy: :path_cheapest_arc, local_search_metaheuristic: :guided_local_search) + + ticker.kill + assert assignment + assert_operator assignment.objective_value, :>, 0 + ensure + ticker&.kill + end + + def test_matrix_and_callback_models_agree + matrix_routing, = matrix_model + callback_routing, = callback_model + + matrix_assignment = matrix_routing.solve(first_solution_strategy: :path_cheapest_arc) + callback_assignment = callback_routing.solve(first_solution_strategy: :path_cheapest_arc) + + assert_equal callback_assignment.objective_value, matrix_assignment.objective_value + end + + def test_solve_from_assignment_releases_gvl + routing, manager = matrix_model + routing.close_model + initial = routing.read_assignment_from_routes([(1...node_count).to_a], true) + assert initial + + counter = 0 + ticker = Thread.new { loop { counter += 1 } } + + search_parameters = ORTools.default_routing_search_parameters + search_parameters.time_limit = 1 + search_parameters.local_search_metaheuristic = :guided_local_search + assignment = routing.solve_from_assignment_with_parameters(initial, search_parameters) + + ticker.kill + assert assignment + assert_operator counter, :>, 1_000, "expected ticker thread to run while solving" + ensure + ticker&.kill + end + + private + + def node_count + 30 + end + + def distances + @distances ||= begin + rng = Random.new(42) + Array.new(node_count) do |i| + Array.new(node_count) { |j| (i == j) ? 0 : rng.rand(100..5000) } + end + end + end + + def matrix_model + manager = ORTools::RoutingIndexManager.new(node_count, 1, 0) + routing = ORTools::RoutingModel.new(manager) + transit = routing.register_transit_matrix(distances) + routing.set_arc_cost_evaluator_of_all_vehicles(transit) + [routing, manager] + end + + def callback_model + manager = ORTools::RoutingIndexManager.new(node_count, 1, 0) + routing = ORTools::RoutingModel.new(manager) + callback = lambda do |from_index, to_index| + distances[manager.index_to_node(from_index)][manager.index_to_node(to_index)] + end + transit = routing.register_transit_callback(callback) + routing.set_arc_cost_evaluator_of_all_vehicles(transit) + [routing, manager] + end +end