From f3157f306a7649d06c8c334bbe3cf714a1a32392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Mon, 19 Dec 2022 15:06:52 +0100 Subject: [PATCH] Set the default work start method to spawn on MacOS (#16089) --- src/lightning_app/CHANGELOG.md | 4 +++ src/lightning_app/core/work.py | 2 +- tests/tests_app/runners/backends/__init__.py | 0 .../runners/backends/test_mp_process.py | 27 +++++++++++++++++++ tests/tests_app/runners/test_multiprocess.py | 4 +-- 5 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/tests_app/runners/backends/__init__.py create mode 100644 tests/tests_app/runners/backends/test_mp_process.py diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index 4ee679fcd6..65d62b29da 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -28,9 +28,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Changed +- The default `start_method` for creating Work processes locally on MacOS is now 'spawn' (previously 'fork') ([#16089](https://github.com/Lightning-AI/lightning/pull/16089)) + + - The utility `lightning.app.utilities.cloud.is_running_in_cloud` now returns `True` during loading of the app locally when running with `--cloud` ([#16045](https://github.com/Lightning-AI/lightning/pull/16045)) + ### Deprecated - diff --git a/src/lightning_app/core/work.py b/src/lightning_app/core/work.py index b34f42f2fe..dfc0c9e570 100644 --- a/src/lightning_app/core/work.py +++ b/src/lightning_app/core/work.py @@ -51,7 +51,7 @@ class LightningWork: _run_executor_cls: Type[WorkRunExecutor] = WorkRunExecutor # TODO: Move to spawn for all Operating System. - _start_method = "spawn" if sys.platform == "win32" else "fork" + _start_method = "spawn" if sys.platform in ("darwin", "win32") else "fork" def __init__( self, diff --git a/tests/tests_app/runners/backends/__init__.py b/tests/tests_app/runners/backends/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/tests_app/runners/backends/test_mp_process.py b/tests/tests_app/runners/backends/test_mp_process.py new file mode 100644 index 0000000000..49a926b683 --- /dev/null +++ b/tests/tests_app/runners/backends/test_mp_process.py @@ -0,0 +1,27 @@ +from unittest import mock +from unittest.mock import MagicMock, Mock + +from lightning_app import LightningApp, LightningWork +from lightning_app.runners.backends import MultiProcessingBackend + + +@mock.patch("lightning_app.runners.backends.mp_process.multiprocessing") +def test_backend_create_work_with_set_start_method(multiprocessing_mock): + backend = MultiProcessingBackend(entrypoint_file="fake.py") + work = Mock(spec=LightningWork) + work._start_method = "test_start_method" + + app = LightningApp(work) + app.caller_queues = MagicMock() + app.delta_queue = MagicMock() + app.readiness_queue = MagicMock() + app.error_queue = MagicMock() + app.request_queues = MagicMock() + app.response_queues = MagicMock() + app.copy_request_queues = MagicMock() + app.copy_response_queues = MagicMock() + app.flow_to_work_delta_queues = MagicMock() + + backend.create_work(app=app, work=work) + multiprocessing_mock.get_context.assert_called_with("test_start_method") + multiprocessing_mock.get_context().Process().start.assert_called_once() diff --git a/tests/tests_app/runners/test_multiprocess.py b/tests/tests_app/runners/test_multiprocess.py index 2e1a34ab38..48bbedf555 100644 --- a/tests/tests_app/runners/test_multiprocess.py +++ b/tests/tests_app/runners/test_multiprocess.py @@ -68,7 +68,7 @@ class ContextWork(LightningWork): assert _get_context().value == "work" -class ContxtFlow(LightningFlow): +class ContextFlow(LightningFlow): def __init__(self): super().__init__() self.work = ContextWork() @@ -83,7 +83,7 @@ class ContxtFlow(LightningFlow): def test_multiprocess_runtime_sets_context(): """Test that the runtime sets the global variable COMPONENT_CONTEXT in Flow and Work.""" - MultiProcessRuntime(LightningApp(ContxtFlow())).dispatch() + MultiProcessRuntime(LightningApp(ContextFlow())).dispatch() @pytest.mark.parametrize(