Skip to content
This repository has been archived by the owner on Jul 17, 2024. It is now read-only.

Commit

Permalink
fix: Use daemon threads for SolverManager (#69)
Browse files Browse the repository at this point in the history
Before, if a SolverManager creates a Solver, it creates
a non-daemon thread, which can prevent the Python process from
EVER exiting unless forced.

Now, the SolverManager spawns only daemon threads, which allows the
Python process to exit. This also allows us to remove some test
configuration code that was used to force the JVM to exit.

Additionaly, SolverManager can take SolverConfig directly and may
take a SolutionManagerConfig too.

Removed some solver manager tests to reduced flakiness.
  • Loading branch information
Christopher-Chianelli authored Jun 13, 2024
1 parent b6b1fa1 commit 57ac260
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 69 deletions.
31 changes: 2 additions & 29 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@ def pytest_addoption(parser):
parser.addoption('--output-generated-classes', action='store', default='false')


def pytest_configure(config):
"""
Allows plugins and conftest files to perform initial configuration.
This hook is called for every plugin and initial conftest
file after command line options have been parsed.
"""
pass


def pytest_sessionstart(session):
"""
Called after the Session object has been created and
Expand All @@ -35,23 +26,5 @@ def pytest_sessionstart(session):
timefold.solver.init()

if session.config.getoption('--output-generated-classes') != 'false':
timefold.solver.set_class_output_directory(pathlib.Path('target', 'tox-generated-classes', 'python', f'{sys.version_info[0]}.{sys.version_info[1]}'))


exit_code = 0
def pytest_sessionfinish(session, exitstatus):
"""
Called after whole test run finished, right before
returning the exit status to the system.
"""
global exit_code
exit_code = exitstatus


def pytest_unconfigure(config):
"""
Called before test process is exited.
"""
global exit_code
from java.lang import System
System.exit(exit_code)
timefold.solver.set_class_output_directory(pathlib.Path('target', 'tox-generated-classes', 'python',
f'{sys.version_info[0]}.{sys.version_info[1]}'))
34 changes: 2 additions & 32 deletions tests/test_solver_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ def assert_problem_change_solver_run(solver_manager, solver_job):
assert solution.value_list[0].value == 6
assert solver_manager.get_solver_status(1) == SolverStatus.NOT_SOLVING

with SolverManager.create(SolverFactory.create(solver_config)) as solver_manager:

with SolverManager.create(solver_config, SolverManagerConfig(parallel_solver_count='AUTO')) as solver_manager:
lock.acquire()
solver_job = solver_manager.solve(1, problem)
assert_solver_run(solver_manager, solver_job)
Expand All @@ -126,12 +127,6 @@ def get_problem(problem_id):
.with_problem_finder(get_problem)).run()
assert_solver_run(solver_manager, solver_job)

lock.acquire()
solver_job = (solver_manager.solve_builder()
.with_problem_id(1)
.with_problem_finder(get_problem)).run()
assert_problem_change_solver_run(solver_manager, solver_job)

solution_list = []
semaphore = Semaphore(0)

Expand All @@ -149,17 +144,6 @@ def on_best_solution_changed(solution):
assert semaphore.acquire(timeout=1)
assert len(solution_list) == 1

solution_list = []
lock.acquire()
solver_job = (solver_manager.solve_builder()
.with_problem_id(1)
.with_problem_finder(get_problem)
.with_best_solution_consumer(on_best_solution_changed)
).run()
assert_problem_change_solver_run(solver_manager, solver_job)
assert semaphore.acquire(timeout=1)
assert len(solution_list) == 1

solution_list = []
lock.acquire()
solver_job = (solver_manager.solve_builder()
Expand All @@ -169,21 +153,7 @@ def on_best_solution_changed(solution):
.with_final_best_solution_consumer(on_best_solution_changed)
).run()
assert_solver_run(solver_manager, solver_job)
# Wait for 2 acquires, one for best solution consumer,
# another for final best solution consumer
assert semaphore.acquire(timeout=1)
assert semaphore.acquire(timeout=1)
assert len(solution_list) == 2

solution_list = []
lock.acquire()
solver_job = (solver_manager.solve_builder()
.with_problem_id(1)
.with_problem_finder(get_problem)
.with_best_solution_consumer(on_best_solution_changed)
.with_final_best_solution_consumer(on_best_solution_changed)
).run()
assert_problem_change_solver_run(solver_manager, solver_job)
# Wait for 2 acquires, one for best solution consumer,
# another for final best solution consumer
assert semaphore.acquire(timeout=1)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package ai.timefold.solver.python;

import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;

/**
* There a Catch-22 that occurs on shutdown:
* <p>
* - In order for Python to free its variables, it must be terminated.
* - In order for Python to be terminated, the JVM must be terminated.
* - In order for the JVM to be terminated, all its non-daemon threads must be terminated.
* - Executors keep all its threads alive until it is freed/have no more references.
* - In order for the Executor to be freed/have no more references, it cannot have a reference in Python.
* - To not have a reference in Python means Python must free its variables, creating the Catch-22
* <p>
* Thus, if non-daemon threads are used, and a {@link ai.timefold.solver.core.api.solver.SolverManager}
* solves at least one problem (creating a keep-alive thread in its {@link java.util.concurrent.ThreadPoolExecutor}),
* Python cannot shut down gracefully and will become unresponsive when interrupted.
* <p>
* This class uses {@link Executors#defaultThreadFactory()} to create a new thread, but sets the created
* thread to daemon mode so Python can shut down gracefully.
*/
public class DaemonThreadFactory implements ThreadFactory {
private static final ThreadFactory THREAD_FACTORY = Executors.defaultThreadFactory();

@Override
public Thread newThread(Runnable runnable) {
Thread out = THREAD_FACTORY.newThread(runnable);
out.setDaemon(true);
return out;
}
}
28 changes: 22 additions & 6 deletions timefold-solver-python-core/src/main/python/_solver_manager.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from ._problem_change import ProblemChange, ProblemChangeWrapper
from .config import SolverConfigOverride
from .config import SolverConfig, SolverConfigOverride, SolverManagerConfig
from ._solver_factory import SolverFactory
from ._future import wrap_future
from ._timefold_java_interop import update_log_level
Expand Down Expand Up @@ -342,22 +342,38 @@ def __init__(self, delegate: '_JavaSolverManager'):
self._delegate = delegate

@staticmethod
def create(solver_factory: 'SolverFactory[Solution_]') -> 'SolverManager[Solution_, ProblemId_]':
def create(solver_factory_or_config: 'SolverConfig | SolverFactory[Solution_]',
solver_manager_config: 'SolverManagerConfig' = None) -> 'SolverManager[Solution_, ProblemId_]':
"""
Use a `SolverFactory` to build a `SolverManager`.
Use a `SolverConfig` or `SolverFactory` to build a `SolverManager`.
Parameters
----------
solver_factory : SolverFactory[Solution_]
The `SolverFactory` to build the `SolverManager` from.
solver_factory_or_config : SolverConfig | SolverFactory[Solution_]
The `SolverConfig` or `SolverFactory` to build the `SolverManager` from.
solver_manager_config: SolverManagerConfig, optional
Additional settings that can be used to configure the `SolverManager`.
Returns
-------
SolverManager
A new `SolverManager` instance.
"""
from ai.timefold.solver.core.api.solver import SolverManager as JavaSolverManager
return SolverManager(JavaSolverManager.create(solver_factory._delegate)) # noqa
from ai.timefold.solver.python import DaemonThreadFactory

if solver_manager_config is None:
solver_manager_config = SolverManagerConfig()

java_solver_manager_config = solver_manager_config._to_java_solver_manager_config() # noqa
java_solver_manager_config.setThreadFactoryClass(DaemonThreadFactory.class_)

if isinstance(solver_factory_or_config, SolverConfig):
solver_factory_or_config = SolverFactory.create(solver_factory_or_config)

return SolverManager(JavaSolverManager.create(solver_factory_or_config._delegate, # noqa
java_solver_manager_config))

def solve(self, problem_id: ProblemId_, problem: Solution_,
final_best_solution_listener: Callable[[Solution_], None] = None) -> SolverJob[Solution_, ProblemId_]:
Expand Down
27 changes: 25 additions & 2 deletions timefold-solver-python-core/src/main/python/config/_config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from ..score import ConstraintFactory, Constraint, IncrementalScoreCalculator
from .._timefold_java_interop import is_enterprise_installed

from typing import Any, Optional, Callable, TypeVar, Generic, TYPE_CHECKING
from typing import Any, Optional, Callable, TypeVar, Generic, Literal, TYPE_CHECKING
from dataclasses import dataclass, field
from enum import Enum
from pathlib import Path
Expand Down Expand Up @@ -378,6 +378,29 @@ def _to_java_solver_config_override(self):
return out


@dataclass(kw_only=True)
class SolverManagerConfig:
"""
Includes settings to configure a SolverManager.
Attributes
----------
parallel_solver_count: int | 'AUTO', optional
If set to an integer, the number of parallel jobs that can be run
simultaneously.
If unset or set to 'AUTO', the number of parallel jobs is determined
based on the number of CPU cores available.
"""
parallel_solver_count: Optional[int | Literal['AUTO']] = field(default=None)

def _to_java_solver_manager_config(self):
from ai.timefold.solver.core.config.solver import SolverManagerConfig as JavaSolverManagerConfig
out = JavaSolverManagerConfig()
if self.parallel_solver_count is not None:
out = out.withParallelSolverCount(str(self.parallel_solver_count))
return out


__all__ = ['Duration', 'EnvironmentMode', 'TerminationCompositionStyle',
'RequiresEnterpriseError', 'MoveThreadCount',
'RequiresEnterpriseError', 'MoveThreadCount', 'SolverManagerConfig',
'SolverConfig', 'SolverConfigOverride', 'ScoreDirectorFactoryConfig', 'TerminationConfig']

0 comments on commit 57ac260

Please sign in to comment.