Release GVL during routing solves when no Ruby transit callbacks are registered

ext/or-tools/routing.cpp

@@ -5,6 +5,7 @@
 #include <ortools/constraint_solver/routing_parameters.h>
 #include <rice/rice.hpp>
 #include <rice/stl.hpp>
+#include <ruby/thread.h>
 
 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<RoutingSolveArgs*>(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<RoutingModel, RoutingIndexManager, RoutingModelParameters>(), 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",

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

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