From 06258e5434f662422426e8fd3b848a420d02a64b Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 20 Oct 2023 18:38:23 +0000 Subject: [PATCH 01/50] move pycoro to mrc --- ci/conda/environments/dev_env.yml | 1 + python/mrc/_pymrc/CMakeLists.txt | 1 + python/mrc/_pymrc/include/pymrc/coro.hpp | 433 ++++++++++++++++++++ python/mrc/_pymrc/src/coro.cpp | 26 ++ python/mrc/_pymrc/tests/CMakeLists.txt | 2 + python/mrc/_pymrc/tests/coro/CMakeLists.txt | 29 ++ python/mrc/_pymrc/tests/coro/module.cpp | 72 ++++ python/mrc/core/CMakeLists.txt | 1 + python/mrc/core/coro.cpp | 67 +++ python/tests/test_pycoro.py | 68 +++ 10 files changed, 700 insertions(+) create mode 100644 python/mrc/_pymrc/include/pymrc/coro.hpp create mode 100644 python/mrc/_pymrc/src/coro.cpp create mode 100644 python/mrc/_pymrc/tests/coro/CMakeLists.txt create mode 100644 python/mrc/_pymrc/tests/coro/module.cpp create mode 100644 python/mrc/core/coro.cpp create mode 100644 python/tests/test_pycoro.py diff --git a/ci/conda/environments/dev_env.yml b/ci/conda/environments/dev_env.yml index 5af8a91c9..08d31e0c0 100644 --- a/ci/conda/environments/dev_env.yml +++ b/ci/conda/environments/dev_env.yml @@ -59,6 +59,7 @@ dependencies: - pybind11-stubgen=0.10 - pytest - pytest-timeout + - pytest-asyncio - python=3.10 - scikit-build>=0.17 - sysroot_linux-64=2.17 diff --git a/python/mrc/_pymrc/CMakeLists.txt b/python/mrc/_pymrc/CMakeLists.txt index 2e81eac88..ed385504f 100644 --- a/python/mrc/_pymrc/CMakeLists.txt +++ b/python/mrc/_pymrc/CMakeLists.txt @@ -18,6 +18,7 @@ find_package(prometheus-cpp REQUIRED) # Keep all source files sorted!!! add_library(pymrc + src/coro.cpp src/executor.cpp src/logging.cpp src/module_registry.cpp diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp new file mode 100644 index 000000000..7f046ca67 --- /dev/null +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -0,0 +1,433 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +// Dont directly include python headers +// IWYU pragma: no_include + +namespace mrc::pymrc::coro { + +class PYBIND11_EXPORT StopIteration : public pybind11::stop_iteration +{ + public: + StopIteration(pybind11::object&& result) : stop_iteration("--"), m_result(std::move(result)){}; + ~StopIteration() override; + + void set_error() const override + { + PyErr_SetObject(PyExc_StopIteration, this->m_result.ptr()); + } + + private: + pybind11::object m_result; +}; + +class PYBIND11_EXPORT CppToPyAwaitable : public std::enable_shared_from_this +{ + public: + CppToPyAwaitable() = default; + + template + CppToPyAwaitable(mrc::coroutines::Task&& task) + { + auto converter = [](mrc::coroutines::Task incoming_task) -> mrc::coroutines::Task { + DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL when resuming a C++ coroutine"; + + mrc::pymrc::PyHolder holder; + + if constexpr (std::is_same_v) + { + co_await incoming_task; + + // Need the GIL to make the return object + pybind11::gil_scoped_acquire gil; + + holder = pybind11::none(); + } + else + { + auto result = co_await incoming_task; + + // Need the GIL to cast the return object + pybind11::gil_scoped_acquire gil; + + holder = pybind11::cast(std::move(result)); + } + + co_return holder; + }; + + m_task = converter(std::move(task)); + } + + CppToPyAwaitable(mrc::coroutines::Task&& task) : m_task(std::move(task)) {} + + std::shared_ptr iter() + { + return this->shared_from_this(); + } + + std::shared_ptr await() + { + return this->shared_from_this(); + } + + void next() + { + // Need to release the GIL before waiting + pybind11::gil_scoped_release nogil; + + // Run the tick function which will resume the coroutine + this->tick(); + + if (m_task.is_ready()) + { + pybind11::gil_scoped_acquire gil; + + // job done -> throw + auto exception = StopIteration(std::move(m_task.promise().result())); + + // Destroy the task now that we have the value + m_task.destroy(); + + throw exception; + } + } + + protected: + virtual void tick() + { + if (!m_has_resumed) + { + m_has_resumed = true; + + m_task.resume(); + } + } + + bool m_has_resumed{false}; + mrc::coroutines::Task m_task; +}; + +/** + * @brief Similar to CppToPyAwaitable but will yield to other fibers when waiting for the coroutine to finish. Use this + * once per loop at the main entry point for the asyncio loop + * + */ +class PYBIND11_EXPORT BoostFibersMainPyAwaitable : public CppToPyAwaitable +{ + public: + using CppToPyAwaitable::CppToPyAwaitable; + + protected: + void tick() override + { + // Call the base class and then see if any fibers need processing by calling yield + CppToPyAwaitable::tick(); + + bool has_fibers = boost::fibers::has_ready_fibers(); + + if (has_fibers) + { + // Yield to other fibers + boost::this_fiber::yield(); + } + } +}; + +class PYBIND11_EXPORT PyTaskToCppAwaitable +{ + public: + PyTaskToCppAwaitable() = default; + PyTaskToCppAwaitable(mrc::pymrc::PyObjectHolder&& task) : m_task(std::move(task)) + { + pybind11::gil_scoped_acquire acquire; + if (pybind11::module_::import("inspect").attr("iscoroutine")(m_task).cast()) + { + m_task = pybind11::module_::import("asyncio").attr("create_task")(m_task); + } + } + + bool await_ready() const noexcept + { + // Always suspend + return false; + } + + void await_suspend(std::coroutine_handle<> caller) noexcept + { + pybind11::gil_scoped_acquire gil; + + auto done_callback = pybind11::cpp_function([this, caller](pybind11::object future) { + try + { + // Save the result value + m_result = future.attr("result")(); + } catch (pybind11::error_already_set) + { + m_exception_ptr = std::current_exception(); + } + + pybind11::gil_scoped_release nogil; + + // Resume the coroutine + caller.resume(); + }); + + m_task.attr("add_done_callback")(done_callback); + } + + mrc::pymrc::PyHolder await_resume() + { + if (m_exception_ptr) + { + std::rethrow_exception(m_exception_ptr); + } + + return std::move(m_result); + } + + private: + mrc::pymrc::PyObjectHolder m_task; + mrc::pymrc::PyHolder m_result; + std::exception_ptr m_exception_ptr; +}; + +// ====== HELPER MACROS ====== + +#define MRC_PYBIND11_FAIL_ABSTRACT(cname, fnname) \ + pybind11::pybind11_fail(MRC_CONCAT_STR("Tried to call pure virtual function \"" << PYBIND11_STRINGIFY(cname) \ + << "::" << fnname << "\"")); + +// ====== OVERRIDE PURE TEMPLATE ====== +#define MRC_PYBIND11_OVERRIDE_PURE_TEMPLATE_NAME(ret_type, abstract_cname, cname, name, fn, ...) \ + do \ + { \ + PYBIND11_OVERRIDE_IMPL(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__); \ + if constexpr (std::is_same_v) \ + { \ + MRC_PYBIND11_FAIL_ABSTRACT(PYBIND11_TYPE(abstract_cname), name); \ + } \ + else \ + { \ + return cname::fn(__VA_ARGS__); \ + } \ + } while (false) + +#define MRC_PYBIND11_OVERRIDE_PURE_TEMPLATE(ret_type, abstract_cname, cname, fn, ...) \ + MRC_PYBIND11_OVERRIDE_PURE_TEMPLATE_NAME(PYBIND11_TYPE(ret_type), \ + PYBIND11_TYPE(abstract_cname), \ + PYBIND11_TYPE(cname), \ + #fn, \ + fn, \ + __VA_ARGS__) +// ====== OVERRIDE PURE TEMPLATE ====== + +// ====== OVERRIDE COROUTINE IMPL ====== +#define MRC_PYBIND11_OVERRIDE_CORO_IMPL(ret_type, cname, name, ...) \ + do \ + { \ + DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL when resuming a C++ coroutine"; \ + pybind11::gil_scoped_acquire gil; \ + pybind11::function override = pybind11::get_override(static_cast(this), name); \ + if (override) \ + { \ + auto o_coro = override(__VA_ARGS__); \ + auto asyncio_module = pybind11::module::import("asyncio"); \ + /* Return type must be a coroutine to allow calling asyncio.create_task() */ \ + if (!asyncio_module.attr("iscoroutine")(o_coro).cast()) \ + { \ + pybind11::pybind11_fail(MRC_CONCAT_STR("Return value from overriden async function " \ + << PYBIND11_STRINGIFY(cname) << "::" << name \ + << " did not return a coroutine. Returned: " \ + << pybind11::str(o_coro).cast())); \ + } \ + auto o_task = asyncio_module.attr("create_task")(o_coro); \ + mrc::pymrc::PyHolder o_result; \ + { \ + pybind11::gil_scoped_release nogil; \ + o_result = co_await mrc::pymrc::coro::PyTaskToCppAwaitable(std::move(o_task)); \ + DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL after returning from co_await"; \ + } \ + if (pybind11::detail::cast_is_temporary_value_reference::value) \ + { \ + static pybind11::detail::override_caster_t caster; \ + co_return pybind11::detail::cast_ref(std::move(o_result), caster); \ + } \ + co_return pybind11::detail::cast_safe(std::move(o_result)); \ + } \ + } while (false) +// ====== OVERRIDE COROUTINE IMPL====== + +// ====== OVERRIDE COROUTINE ====== +#define MRC_PYBIND11_OVERRIDE_CORO_NAME(ret_type, cname, name, fn, ...) \ + do \ + { \ + MRC_PYBIND11_OVERRIDE_CORO_IMPL(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__); \ + return cname::fn(__VA_ARGS__); \ + } while (false) + +#define MRC_PYBIND11_OVERRIDE_CORO(ret_type, cname, fn, ...) \ + MRC_PYBIND11_OVERRIDE_CORO_NAME(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) +// ====== OVERRIDE COROUTINE ====== + +// ====== OVERRIDE COROUTINE PURE====== +#define MRC_PYBIND11_OVERRIDE_CORO_PURE_NAME(ret_type, cname, name, fn, ...) \ + do \ + { \ + MRC_PYBIND11_OVERRIDE_CORO_IMPL(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__); \ + MRC_PYBIND11_FAIL_ABSTRACT(PYBIND11_TYPE(cname), name); \ + } while (false) + +#define MRC_PYBIND11_OVERRIDE_CORO_PURE(ret_type, cname, fn, ...) \ + MRC_PYBIND11_OVERRIDE_CORO_PURE_NAME(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) +// ====== OVERRIDE COROUTINE PURE====== + +// ====== OVERRIDE COROUTINE PURE TEMPLATE====== +#define MRC_PYBIND11_OVERRIDE_CORO_PURE_TEMPLATE_NAME(ret_type, abstract_cname, cname, name, fn, ...) \ + do \ + { \ + MRC_PYBIND11_OVERRIDE_CORO_IMPL(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__); \ + if constexpr (std::is_same_v) \ + { \ + MRC_PYBIND11_FAIL_ABSTRACT(PYBIND11_TYPE(abstract_cname), name); \ + } \ + else \ + { \ + co_return co_await cname::fn(__VA_ARGS__); \ + } \ + } while (false) + +#define MRC_PYBIND11_OVERRIDE_CORO_PURE_TEMPLATE(ret_type, abstract_cname, cname, fn, ...) \ + MRC_PYBIND11_OVERRIDE_CORO_PURE_TEMPLATE_NAME(PYBIND11_TYPE(ret_type), \ + PYBIND11_TYPE(abstract_cname), \ + PYBIND11_TYPE(cname), \ + #fn, \ + fn, \ + __VA_ARGS__) +// ====== OVERRIDE COROUTINE PURE TEMPLATE====== + +} // namespace mrc::pymrc::coro + +// NOLINTNEXTLINE(modernize-concat-nested-namespaces) +namespace PYBIND11_NAMESPACE { +namespace detail { + +/** + * @brief Provides a type caster for converting a C++ coroutine to a python awaitable. Include this file in any pybind11 + * module to automatically convert the types. Allows for converting arguments and return values. + * + * @tparam ReturnT The return type of the coroutine + */ +template +struct type_caster> +{ + public: + /** + * This macro establishes the name 'inty' in + * function signatures and declares a local variable + * 'value' of type inty + */ + PYBIND11_TYPE_CASTER(mrc::coroutines::Task, _("typing.Awaitable[") + make_caster::name + _("]")); + + /** + * Conversion part 1 (Python->C++): convert a PyObject into a inty + * instance or return false upon failure. The second argument + * indicates whether implicit conversions should be applied. + */ + bool load(handle src, bool convert) + { + if (!src || src.is_none()) + { + return false; + } + + if (!PyCoro_CheckExact(src.ptr())) + { + return false; + } + + auto cpp_coro = [](mrc::pymrc::PyHolder py_task) -> mrc::coroutines::Task { + DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL when resuming a C++ coroutine"; + + // Always assume we are resuming without the GIL + pybind11::gil_scoped_acquire gil; + + auto asyncio_task = pybind11::module_::import("asyncio").attr("create_task")(py_task); + + mrc::pymrc::PyHolder py_result; + { + // Release the GIL before awaiting + pybind11::gil_scoped_release nogil; + + py_result = co_await mrc::pymrc::coro::PyTaskToCppAwaitable(std::move(asyncio_task)); + } + + // Now cast back to the C++ type + if (pybind11::detail::cast_is_temporary_value_reference::value) + { + static pybind11::detail::override_caster_t caster; + co_return pybind11::detail::cast_ref(std::move(py_result), caster); + } + co_return pybind11::detail::cast_safe(std::move(py_result)); + }; + + value = cpp_coro(pybind11::reinterpret_borrow(std::move(src))); + + return true; + } + + /** + * Conversion part 2 (C++ -> Python): convert an inty instance into + * a Python object. The second and third arguments are used to + * indicate the return value policy and parent object (for + * ``return_value_policy::reference_internal``) and are generally + * ignored by implicit casters. + */ + static handle cast(mrc::coroutines::Task src, return_value_policy policy, handle parent) + { + // Wrap the object in a CppToPyAwaitable + std::shared_ptr awaitable = std::make_shared( + std::move(src)); + + // Convert the object to a python object + auto py_awaitable = pybind11::cast(std::move(awaitable)); + + return py_awaitable.release(); + } +}; + +} // namespace detail +} // namespace PYBIND11_NAMESPACE diff --git a/python/mrc/_pymrc/src/coro.cpp b/python/mrc/_pymrc/src/coro.cpp new file mode 100644 index 000000000..21a373419 --- /dev/null +++ b/python/mrc/_pymrc/src/coro.cpp @@ -0,0 +1,26 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "pymrc/coro.hpp" + +namespace mrc::pymrc::coro { + +namespace py = pybind11; + +StopIteration::~StopIteration() = default; + +} // namespace mrc::pycoro diff --git a/python/mrc/_pymrc/tests/CMakeLists.txt b/python/mrc/_pymrc/tests/CMakeLists.txt index 4ac354a78..f40e20d72 100644 --- a/python/mrc/_pymrc/tests/CMakeLists.txt +++ b/python/mrc/_pymrc/tests/CMakeLists.txt @@ -17,6 +17,8 @@ list(APPEND CMAKE_MESSAGE_CONTEXT "tests") find_package(pybind11 REQUIRED) +add_subdirectory(coro) + # Keep all source files sorted!!! add_executable(test_pymrc test_codable_pyobject.cpp diff --git a/python/mrc/_pymrc/tests/coro/CMakeLists.txt b/python/mrc/_pymrc/tests/coro/CMakeLists.txt new file mode 100644 index 000000000..a26d7fa9e --- /dev/null +++ b/python/mrc/_pymrc/tests/coro/CMakeLists.txt @@ -0,0 +1,29 @@ +# ============================================================================= +# Copyright (c) 2023, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +list(APPEND CMAKE_MESSAGE_CONTEXT "coro") + +set(MODULE_SOURCE_FILES) + +# Add the module file +list(APPEND MODULE_SOURCE_FILES module.cpp) + +# Create the python module +mrc_add_pybind11_module(coro + INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/include + SOURCE_FILES ${MODULE_SOURCE_FILES} + LINK_TARGETS mrc::pymrc +) + +list(POP_BACK CMAKE_MESSAGE_CONTEXT) diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp new file mode 100644 index 000000000..8cb21e4c0 --- /dev/null +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -0,0 +1,72 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include + +#include + +namespace morpheus::tests::pycoro { + +mrc::coroutines::Task subtract(int a, int b) +{ + co_return a - b; +} + +mrc::coroutines::Task call_fib_async(mrc::pymrc::PyHolder fib, int value, int minus) +{ + auto result = co_await subtract(value, minus); + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fib, result]() { + pybind11::gil_scoped_acquire acquire; + return fib(result); + }()); +} + +mrc::coroutines::Task raise_at_depth_async(mrc::pymrc::PyHolder fn, int depth) +{ + if (depth <= 0) + { + throw std::runtime_error("depth reached zero in c++"); + } + + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn, depth]() { + pybind11::gil_scoped_acquire acquire; + return fn(depth - 1); + }()); +} + +mrc::coroutines::Task call_async(mrc::pymrc::PyHolder fn) +{ + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn]() { + pybind11::gil_scoped_acquire acquire; + return fn(); + }()); +} + +PYBIND11_MODULE(coro, _module) +{ + _module.def("call_fib_async", &call_fib_async); + _module.def("raise_at_depth_async", &raise_at_depth_async); + _module.def("call_async", &call_async); +} + +} // namespace morpheus::tests::pycoro diff --git a/python/mrc/core/CMakeLists.txt b/python/mrc/core/CMakeLists.txt index d635e071f..f04b17f1f 100644 --- a/python/mrc/core/CMakeLists.txt +++ b/python/mrc/core/CMakeLists.txt @@ -16,6 +16,7 @@ list(APPEND CMAKE_MESSAGE_CONTEXT "core") mrc_add_pybind11_module(common SOURCE_FILES common.cpp) +mrc_add_pybind11_module(coro SOURCE_FILES coro.cpp) mrc_add_pybind11_module(executor SOURCE_FILES executor.cpp) mrc_add_pybind11_module(logging SOURCE_FILES logging.cpp) mrc_add_pybind11_module(node SOURCE_FILES node.cpp) diff --git a/python/mrc/core/coro.cpp b/python/mrc/core/coro.cpp new file mode 100644 index 000000000..c22d3be62 --- /dev/null +++ b/python/mrc/core/coro.cpp @@ -0,0 +1,67 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "pymrc/coro.hpp" + +#include +#include +#include +#include +#include +#include // IWYU pragma: keep + +#include +#include +#include +#include +#include + +namespace mrc::pymrc::coro { + +namespace py = pybind11; + +PYBIND11_MODULE(coro, _module) +{ + _module.doc() = R"pbdoc( + ----------------------- + .. currentmodule:: morpheus.llm + .. autosummary:: + :toctree: _generate + + )pbdoc"; + + py::class_>(_module, "CppToPyAwaitable") + .def(py::init<>()) + .def("__iter__", &CppToPyAwaitable::iter) + .def("__await__", &CppToPyAwaitable::await) + .def("__next__", &CppToPyAwaitable::next); + + py::class_>( + _module, "BoostFibersMainPyAwaitable") + .def(py::init<>()); + + _module.def("wrap_coroutine", [](coroutines::Task> fn) -> coroutines::Task { + DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL when resuming a C++ coroutine"; + + auto strings = co_await fn; + + co_return strings[0]; + }); + + // _module.attr("__version__") = + // MRC_CONCAT_STR(morpheus_VERSION_MAJOR << "." << morpheus_VERSION_MINOR << "." << morpheus_VERSION_PATCH); +} +} // namespace mrc::pycoro diff --git a/python/tests/test_pycoro.py b/python/tests/test_pycoro.py new file mode 100644 index 000000000..3a513a085 --- /dev/null +++ b/python/tests/test_pycoro.py @@ -0,0 +1,68 @@ +# SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import asyncio +import pytest + +from mrc.core import coro as pycoro # pylint: disable=morpheus-incorrect-lib-from-import + +@pytest.mark.asyncio +async def test_pycoro(): + + hit_inside = False + + async def inner(): + + nonlocal hit_inside + + result = await pycoro.wrap_coroutine(asyncio.sleep(1, result=['a', 'b', 'c'])) + + hit_inside = True + + return [result] + + returned_val = await pycoro.wrap_coroutine(inner()) + + assert returned_val == 'a' + assert hit_inside + + +@pytest.mark.asyncio +async def test_pycoro_many(): + + expected_count = 1000 + hit_count = 0 + + start_time = asyncio.get_running_loop().time() + + async def inner(): + + nonlocal hit_count + + await asyncio.sleep(1) + + hit_count += 1 + + return ['a', 'b', 'c'] + + coros = [pycoro.wrap_coroutine(inner()) for _ in range(expected_count)] + + returned_vals = await asyncio.gather(*coros) + + end_time = asyncio.get_running_loop().time() + + assert returned_vals == ['a'] * expected_count + assert hit_count == expected_count + assert (end_time - start_time) < 1.5 From 44c35756aa7af8f3b08f8a4ac6c2be6223d7eb9a Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 20 Oct 2023 22:21:45 +0000 Subject: [PATCH 02/50] python coroutine tests --- python/mrc/_pymrc/tests/coro/module.cpp | 19 +++-- python/tests/test_pycoro.py | 107 +++++++++++++++++++++--- 2 files changed, 108 insertions(+), 18 deletions(-) diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp index 8cb21e4c0..9427e92cf 100644 --- a/python/mrc/_pymrc/tests/coro/module.cpp +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -35,10 +35,10 @@ mrc::coroutines::Task subtract(int a, int b) mrc::coroutines::Task call_fib_async(mrc::pymrc::PyHolder fib, int value, int minus) { auto result = co_await subtract(value, minus); - co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fib, result]() { + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([](auto fib, auto result) { pybind11::gil_scoped_acquire acquire; return fib(result); - }()); + }(fib, result)); } mrc::coroutines::Task raise_at_depth_async(mrc::pymrc::PyHolder fn, int depth) @@ -48,22 +48,29 @@ mrc::coroutines::Task raise_at_depth_async(mrc::pymrc::PyH throw std::runtime_error("depth reached zero in c++"); } - co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn, depth]() { + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([](auto fn, auto depth) { pybind11::gil_scoped_acquire acquire; return fn(depth - 1); - }()); + }(fn, depth)); + + // co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn, depth]() { + // pybind11::gil_scoped_acquire acquire; + // return fn(depth - 1); + // }()); } mrc::coroutines::Task call_async(mrc::pymrc::PyHolder fn) { - co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn]() { + co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([](auto fn) { pybind11::gil_scoped_acquire acquire; return fn(); - }()); + }(fn)); } PYBIND11_MODULE(coro, _module) { + pybind11::module_::import("mrc.core.coro"); // satisfies automatic type conversions for tasks + _module.def("call_fib_async", &call_fib_async); _module.def("raise_at_depth_async", &raise_at_depth_async); _module.def("call_async", &call_async); diff --git a/python/tests/test_pycoro.py b/python/tests/test_pycoro.py index 3a513a085..7b207a161 100644 --- a/python/tests/test_pycoro.py +++ b/python/tests/test_pycoro.py @@ -17,43 +17,47 @@ import pytest from mrc.core import coro as pycoro # pylint: disable=morpheus-incorrect-lib-from-import +from mrc._pymrc.tests.coro.coro import call_async +from mrc._pymrc.tests.coro.coro import call_fib_async +from mrc._pymrc.tests.coro.coro import raise_at_depth_async + @pytest.mark.asyncio -async def test_pycoro(): +async def test_coro(): - hit_inside = False + # hit_inside = False async def inner(): - nonlocal hit_inside + # nonlocal hit_inside result = await pycoro.wrap_coroutine(asyncio.sleep(1, result=['a', 'b', 'c'])) - hit_inside = True + # hit_inside = True return [result] returned_val = await pycoro.wrap_coroutine(inner()) assert returned_val == 'a' - assert hit_inside + # assert hit_inside @pytest.mark.asyncio -async def test_pycoro_many(): +async def test_coro_many(): expected_count = 1000 - hit_count = 0 + # hit_count = 0 start_time = asyncio.get_running_loop().time() async def inner(): - nonlocal hit_count + # nonlocal hit_count - await asyncio.sleep(1) + await asyncio.sleep(0.1) - hit_count += 1 + # hit_count += 1 return ['a', 'b', 'c'] @@ -64,5 +68,84 @@ async def inner(): end_time = asyncio.get_running_loop().time() assert returned_vals == ['a'] * expected_count - assert hit_count == expected_count - assert (end_time - start_time) < 1.5 + # assert hit_count == expected_count + # assert (end_time - start_time) < 1.5 + + +@pytest.mark.asyncio +async def test_python_cpp_async_interleave(): + + def fib(n): + if n < 0: + raise ValueError() + + if n < 2: + return 1 + + return fib(n - 1) + fib(n - 2) + + async def fib_async(n): + if n < 0: + raise ValueError() + + if n < 2: + return 1 + + task_a = call_fib_async(fib_async, n, 1) + task_b = call_fib_async(fib_async, n, 2) + + [a, b] = await asyncio.gather(task_a, task_b) + + return a + b + + assert fib(15) == await fib_async(15) + + +@pytest.mark.asyncio +async def test_python_cpp_async_exception(): + + async def py_raise_at_depth_async(n: int): + if n <= 0: + raise RuntimeError("depth reached zero in python") + + await raise_at_depth_async(py_raise_at_depth_async, n - 1) + + depth = 100 + + with pytest.raises(RuntimeError) as ex: + await raise_at_depth_async(py_raise_at_depth_async, depth + 1) + assert "python" in str(ex.value) + + with pytest.raises(RuntimeError) as ex: + await raise_at_depth_async(py_raise_at_depth_async, depth) + assert "c++" in str(ex.value) + + +@pytest.mark.asyncio +async def test_can_cancel_coroutine_from_python(): + + counter = 0 + + async def increment_recursively(): + nonlocal counter + await asyncio.sleep(0) + counter += 1 + await call_async(increment_recursively) + + task = asyncio.ensure_future(call_async(increment_recursively)) + + await asyncio.sleep(0) + assert counter == 0 + await asyncio.sleep(0) + await asyncio.sleep(0) + assert counter == 1 + await asyncio.sleep(0) + await asyncio.sleep(0) + assert counter == 2 + + task.cancel() + + with pytest.raises(asyncio.exceptions.CancelledError): + await task + + assert counter == 3 From 49fc8355a9c10467be5fb36c7a622ce76ff345d5 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 24 Oct 2023 20:16:44 +0000 Subject: [PATCH 03/50] remove commented code --- python/mrc/_pymrc/tests/coro/module.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp index 9427e92cf..b51edc0e9 100644 --- a/python/mrc/_pymrc/tests/coro/module.cpp +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -52,11 +52,6 @@ mrc::coroutines::Task raise_at_depth_async(mrc::pymrc::PyH pybind11::gil_scoped_acquire acquire; return fn(depth - 1); }(fn, depth)); - - // co_return co_await mrc::pymrc::coro::PyTaskToCppAwaitable([fn, depth]() { - // pybind11::gil_scoped_acquire acquire; - // return fn(depth - 1); - // }()); } mrc::coroutines::Task call_async(mrc::pymrc::PyHolder fn) From 2b2363eb75935cb6f13aad164eda7f7824160fe5 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 24 Oct 2023 20:17:57 +0000 Subject: [PATCH 04/50] adjust indentation of `\` --- python/mrc/_pymrc/include/pymrc/coro.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index 7f046ca67..43739be38 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -278,7 +278,7 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable mrc::pymrc::PyHolder o_result; \ { \ pybind11::gil_scoped_release nogil; \ - o_result = co_await mrc::pymrc::coro::PyTaskToCppAwaitable(std::move(o_task)); \ + o_result = co_await mrc::pymrc::coro::PyTaskToCppAwaitable(std::move(o_task)); \ DCHECK_EQ(PyGILState_Check(), 0) << "Should not have the GIL after returning from co_await"; \ } \ if (pybind11::detail::cast_is_temporary_value_reference::value) \ From 5a807f6fb6ad8149155752cae1dca97667250be3 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 24 Oct 2023 21:10:36 +0000 Subject: [PATCH 05/50] fix styles --- python/mrc/_pymrc/include/pymrc/coro.hpp | 8 +++++--- python/mrc/_pymrc/src/coro.cpp | 4 ++-- python/mrc/_pymrc/tests/coro/CMakeLists.txt | 2 +- python/mrc/core/coro.cpp | 7 ++++--- python/tests/test_pycoro.py | 3 ++- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index 43739be38..2f6fea905 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -28,10 +28,12 @@ #include #include +#include #include #include #include #include +#include #include // Dont directly include python headers @@ -180,7 +182,7 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable } } - bool await_ready() const noexcept + bool await_ready() noexcept // NOLINT(readability-convert-member-functions-to-static) { // Always suspend return false; @@ -419,8 +421,8 @@ struct type_caster> static handle cast(mrc::coroutines::Task src, return_value_policy policy, handle parent) { // Wrap the object in a CppToPyAwaitable - std::shared_ptr awaitable = std::make_shared( - std::move(src)); + std::shared_ptr awaitable = + std::make_shared(std::move(src)); // Convert the object to a python object auto py_awaitable = pybind11::cast(std::move(awaitable)); diff --git a/python/mrc/_pymrc/src/coro.cpp b/python/mrc/_pymrc/src/coro.cpp index 21a373419..8bb57cb84 100644 --- a/python/mrc/_pymrc/src/coro.cpp +++ b/python/mrc/_pymrc/src/coro.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,4 +23,4 @@ namespace py = pybind11; StopIteration::~StopIteration() = default; -} // namespace mrc::pycoro +} // namespace mrc::pymrc::coro diff --git a/python/mrc/_pymrc/tests/coro/CMakeLists.txt b/python/mrc/_pymrc/tests/coro/CMakeLists.txt index a26d7fa9e..788d04832 100644 --- a/python/mrc/_pymrc/tests/coro/CMakeLists.txt +++ b/python/mrc/_pymrc/tests/coro/CMakeLists.txt @@ -1,5 +1,5 @@ # ============================================================================= -# Copyright (c) 2023, NVIDIA CORPORATION. +# Copyright (c) 2022-2023, NVIDIA CORPORATION. # # Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except # in compliance with the License. You may obtain a copy of the License at diff --git a/python/mrc/core/coro.cpp b/python/mrc/core/coro.cpp index c22d3be62..8139ce9ec 100644 --- a/python/mrc/core/coro.cpp +++ b/python/mrc/core/coro.cpp @@ -49,8 +49,9 @@ PYBIND11_MODULE(coro, _module) .def("__await__", &CppToPyAwaitable::await) .def("__next__", &CppToPyAwaitable::next); - py::class_>( - _module, "BoostFibersMainPyAwaitable") + py::class_>( // + _module, + "BoostFibersMainPyAwaitable") .def(py::init<>()); _module.def("wrap_coroutine", [](coroutines::Task> fn) -> coroutines::Task { @@ -64,4 +65,4 @@ PYBIND11_MODULE(coro, _module) // _module.attr("__version__") = // MRC_CONCAT_STR(morpheus_VERSION_MAJOR << "." << morpheus_VERSION_MINOR << "." << morpheus_VERSION_PATCH); } -} // namespace mrc::pycoro +} // namespace mrc::pymrc::coro diff --git a/python/tests/test_pycoro.py b/python/tests/test_pycoro.py index 7b207a161..6222d4b92 100644 --- a/python/tests/test_pycoro.py +++ b/python/tests/test_pycoro.py @@ -14,12 +14,13 @@ # limitations under the License. import asyncio + import pytest -from mrc.core import coro as pycoro # pylint: disable=morpheus-incorrect-lib-from-import from mrc._pymrc.tests.coro.coro import call_async from mrc._pymrc.tests.coro.coro import call_fib_async from mrc._pymrc.tests.coro.coro import raise_at_depth_async +from mrc.core import coro as pycoro @pytest.mark.asyncio From 12e54fb1d2bedca2ac9f20ca2da28130e6c23624 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 25 Oct 2023 17:50:29 +0000 Subject: [PATCH 06/50] adjust styles --- python/mrc/_pymrc/include/pymrc/coro.hpp | 4 +--- python/mrc/_pymrc/tests/coro/module.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index 2f6fea905..5c80398cc 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -28,12 +28,10 @@ #include #include -#include #include #include #include #include -#include #include // Dont directly include python headers @@ -182,7 +180,7 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable } } - bool await_ready() noexcept // NOLINT(readability-convert-member-functions-to-static) + static bool await_ready() noexcept // NOLINT(readability-convert-member-functions-to-static) { // Always suspend return false; diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp index b51edc0e9..4d69d5a0d 100644 --- a/python/mrc/_pymrc/tests/coro/module.cpp +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -15,14 +15,15 @@ * limitations under the License. */ -#include - #include +#include #include #include #include +#include #include +#include #include namespace morpheus::tests::pycoro { @@ -64,7 +65,7 @@ mrc::coroutines::Task call_async(mrc::pymrc::PyHolder fn) PYBIND11_MODULE(coro, _module) { - pybind11::module_::import("mrc.core.coro"); // satisfies automatic type conversions for tasks + pybind11::module_::import("mrc.core.coro"); // satisfies automatic type conversions for tasks _module.def("call_fib_async", &call_fib_async); _module.def("raise_at_depth_async", &raise_at_depth_async); From 4822967bc141968ae0f3dbcbfcf9f0dc79e2e12a Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 25 Oct 2023 18:07:54 +0000 Subject: [PATCH 07/50] add back some timing and hit-count logic to coro tests --- python/tests/test_pycoro.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/python/tests/test_pycoro.py b/python/tests/test_pycoro.py index 6222d4b92..b92b12290 100644 --- a/python/tests/test_pycoro.py +++ b/python/tests/test_pycoro.py @@ -14,7 +14,6 @@ # limitations under the License. import asyncio - import pytest from mrc._pymrc.tests.coro.coro import call_async @@ -48,17 +47,17 @@ async def inner(): async def test_coro_many(): expected_count = 1000 - # hit_count = 0 + hit_count = 0 start_time = asyncio.get_running_loop().time() async def inner(): - # nonlocal hit_count + nonlocal hit_count await asyncio.sleep(0.1) - # hit_count += 1 + hit_count += 1 return ['a', 'b', 'c'] @@ -69,8 +68,8 @@ async def inner(): end_time = asyncio.get_running_loop().time() assert returned_vals == ['a'] * expected_count - # assert hit_count == expected_count - # assert (end_time - start_time) < 1.5 + assert hit_count == expected_count + assert (end_time - start_time) < 1.5 @pytest.mark.asyncio From ff44453b19cbc188e7322718d15df19c12379d8a Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 25 Oct 2023 18:47:06 +0000 Subject: [PATCH 08/50] remove unused namespace and rename pycoro --- python/mrc/_pymrc/tests/coro/module.cpp | 4 ---- python/tests/{test_pycoro.py => test_coro.py} | 8 ++++---- 2 files changed, 4 insertions(+), 8 deletions(-) rename python/tests/{test_pycoro.py => test_coro.py} (93%) diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp index 4d69d5a0d..df3b3ad0f 100644 --- a/python/mrc/_pymrc/tests/coro/module.cpp +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -26,8 +26,6 @@ #include #include -namespace morpheus::tests::pycoro { - mrc::coroutines::Task subtract(int a, int b) { co_return a - b; @@ -71,5 +69,3 @@ PYBIND11_MODULE(coro, _module) _module.def("raise_at_depth_async", &raise_at_depth_async); _module.def("call_async", &call_async); } - -} // namespace morpheus::tests::pycoro diff --git a/python/tests/test_pycoro.py b/python/tests/test_coro.py similarity index 93% rename from python/tests/test_pycoro.py rename to python/tests/test_coro.py index b92b12290..485144768 100644 --- a/python/tests/test_pycoro.py +++ b/python/tests/test_coro.py @@ -19,7 +19,7 @@ from mrc._pymrc.tests.coro.coro import call_async from mrc._pymrc.tests.coro.coro import call_fib_async from mrc._pymrc.tests.coro.coro import raise_at_depth_async -from mrc.core import coro as pycoro +from mrc.core import coro @pytest.mark.asyncio @@ -31,13 +31,13 @@ async def inner(): # nonlocal hit_inside - result = await pycoro.wrap_coroutine(asyncio.sleep(1, result=['a', 'b', 'c'])) + result = await coro.wrap_coroutine(asyncio.sleep(1, result=['a', 'b', 'c'])) # hit_inside = True return [result] - returned_val = await pycoro.wrap_coroutine(inner()) + returned_val = await coro.wrap_coroutine(inner()) assert returned_val == 'a' # assert hit_inside @@ -61,7 +61,7 @@ async def inner(): return ['a', 'b', 'c'] - coros = [pycoro.wrap_coroutine(inner()) for _ in range(expected_count)] + coros = [coro.wrap_coroutine(inner()) for _ in range(expected_count)] returned_vals = await asyncio.gather(*coros) From d54ca8f7ebe53b562afa73fdab9f8758f0e9c937 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 25 Oct 2023 18:48:13 +0000 Subject: [PATCH 09/50] fix styles --- python/mrc/_pymrc/tests/coro/module.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/python/mrc/_pymrc/tests/coro/module.cpp b/python/mrc/_pymrc/tests/coro/module.cpp index df3b3ad0f..c5332c78c 100644 --- a/python/mrc/_pymrc/tests/coro/module.cpp +++ b/python/mrc/_pymrc/tests/coro/module.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include From 5412e76a13727d536ffe056c18bfe018d06d74f1 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 25 Oct 2023 19:02:53 +0000 Subject: [PATCH 10/50] fix styles --- python/tests/test_coro.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/tests/test_coro.py b/python/tests/test_coro.py index 485144768..940160f18 100644 --- a/python/tests/test_coro.py +++ b/python/tests/test_coro.py @@ -14,6 +14,7 @@ # limitations under the License. import asyncio + import pytest from mrc._pymrc.tests.coro.coro import call_async From 26fae58982da9c5b57bde290b4efb05a24f0f942 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 27 Oct 2023 01:05:25 +0000 Subject: [PATCH 11/50] add asynciorunnable --- cpp/mrc/include/mrc/coroutines/scheduler.hpp | 18 - cpp/mrc/src/public/coroutines/scheduler.cpp | 12 +- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 386 ++++++++++++++++++ .../include/pymrc/asyncio_scheduler.hpp | 126 ++++++ 4 files changed, 513 insertions(+), 29 deletions(-) create mode 100644 python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp create mode 100644 python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index 1b0aac502..f799db83a 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -25,13 +25,8 @@ #include #include -// IWYU thinks this is needed, but it's not -// IWYU pragma: no_include "mrc/coroutines/task_container.hpp" - namespace mrc::coroutines { -class TaskContainer; // IWYU pragma: keep - /** * @brief Scheduler base class * @@ -75,9 +70,6 @@ class Scheduler : public std::enable_shared_from_this */ [[nodiscard]] virtual auto schedule() -> Operation; - // Enqueues a message without waiting for it. Must return void since the caller will not get the return value - virtual void schedule(Task&& task); - /** * Schedules any coroutine handle that is ready to be resumed. * @param handle The coroutine handle to schedule. @@ -103,13 +95,6 @@ class Scheduler : public std::enable_shared_from_this protected: virtual auto on_thread_start(std::size_t) -> void; - /** - * @brief Get the task container object - * - * @return TaskContainer& - */ - TaskContainer& get_task_container() const; - private: /** * @brief When co_await schedule() is called, this function will be executed by the awaiter. Each scheduler @@ -123,9 +108,6 @@ class Scheduler : public std::enable_shared_from_this mutable std::mutex m_mutex; - // Maintains the lifetime of fire-and-forget tasks scheduled with schedule(Task&& task) - std::unique_ptr m_task_container; - thread_local static Scheduler* m_thread_local_scheduler; thread_local static std::size_t m_thread_id; }; diff --git a/cpp/mrc/src/public/coroutines/scheduler.cpp b/cpp/mrc/src/public/coroutines/scheduler.cpp index af2e70294..f4f7776ac 100644 --- a/cpp/mrc/src/public/coroutines/scheduler.cpp +++ b/cpp/mrc/src/public/coroutines/scheduler.cpp @@ -39,18 +39,13 @@ std::coroutine_handle<> Scheduler::Operation::await_suspend(std::coroutine_handl return m_scheduler.schedule_operation(this); } -Scheduler::Scheduler() : m_task_container(new TaskContainer(*this)) {} +Scheduler::Scheduler() = default; auto Scheduler::schedule() -> Operation { return Operation{*this}; } -void Scheduler::schedule(Task&& task) -{ - return m_task_container->start(std::move(task)); -} - auto Scheduler::yield() -> Operation { return schedule(); @@ -77,9 +72,4 @@ auto Scheduler::on_thread_start(std::size_t thread_id) -> void m_thread_local_scheduler = this; } -TaskContainer& Scheduler::get_task_container() const -{ - return *m_task_container; -} - } // namespace mrc::coroutines diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp new file mode 100644 index 000000000..88cd91773 --- /dev/null +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -0,0 +1,386 @@ +#pragma once + +#include "pymrc/asyncio_scheduler.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace mrc::pymrc { + +template +using Task = mrc::coroutines::Task; + +class ExceptionCatcher +{ + public: + void set_exception(std::exception_ptr ex) + { + auto lock = std::lock_guard(m_mutex); + m_exceptions.push(ex); + } + + bool has_exception() + { + auto lock = std::lock_guard(m_mutex); + return not m_exceptions.empty(); + } + + void rethrow_next_exception() + { + auto lock = std::lock_guard(m_mutex); + + if (m_exceptions.empty()) + { + return; + } + + auto ex = m_exceptions.front(); + m_exceptions.pop(); + + std::rethrow_exception(ex); + } + + private: + std::mutex m_mutex{}; + std::queue m_exceptions{}; +}; + +template +class BoostFutureAwaiter +{ + class Awaiter; + + public: + BoostFutureAwaiter(std::function fn) : m_fn(std::move(fn)) {} + + template + auto operator()(ArgsT&&... args) -> Awaiter + { + // Make a copy of m_fn here so we can call this operator again + return Awaiter(m_fn, std::forward(args)...); + } + + private: + class Awaiter + { + public: + using return_t = typename std::function::result_type; + + template + Awaiter(std::function fn, ArgsT&&... args) + { + m_future = boost::fibers::async(boost::fibers::launch::post, fn, std::forward(args)...); + } + + bool await_ready() noexcept + { + return false; + } + + bool await_suspend(std::coroutine_handle<> continuation) noexcept + { + // Launch a new fiber that waits on the future and then resumes the coroutine + boost::fibers::async( + boost::fibers::launch::post, + [this](std::coroutine_handle<> continuation) { + // Wait on the future + m_future.wait(); + + // Resume the coroutine + continuation.resume(); + }, + std::move(continuation)); + + return true; + } + + auto await_resume() noexcept + { + return m_future.get(); + } + + private: + boost::fibers::future m_future; + std::function)> m_inner_fn; + }; + + std::function m_fn; +}; + +template +class IReadable +{ + public: + virtual ~IReadable() = default; + virtual Task async_read(T& value) = 0; +}; + +template +class BoostFutureReader : public IReadable +{ + public: + template + BoostFutureReader(FuncT&& fn) : m_awaiter(std::forward(fn)) + {} + + Task async_read(T& value) override + { + co_return co_await m_awaiter(std::ref(value)); + } + + private: + BoostFutureAwaiter m_awaiter; +}; + +template +class IWritable +{ + public: + virtual ~IWritable() = default; + virtual Task async_write(T&& value) = 0; +}; + +template +class BoostFutureWriter : public IWritable +{ + public: + template + BoostFutureWriter(FuncT&& fn) : m_awaiter(std::forward(fn)) + {} + + Task async_write(T&& value) override + { + co_return co_await m_awaiter(std::move(value)); + } + + private: + BoostFutureAwaiter m_awaiter; +}; + +template +class CoroutineRunnableSink : public mrc::node::WritableProvider, + public mrc::node::ReadableAcceptor, + public mrc::node::SinkChannelOwner +{ + protected: + CoroutineRunnableSink() + { + // Set the default channel + this->set_channel(std::make_unique>()); + } + + auto build_readable_generator(std::stop_token stop_token) -> mrc::coroutines::AsyncGenerator + { + auto read_awaiter = BoostFutureReader([this](T& value) { + return this->get_readable_edge()->await_read(value); + }); + + while (!stop_token.stop_requested()) + { + T value; + + // Pull a message off of the upstream channel + auto status = co_await read_awaiter.async_read(std::ref(value)); + + if (status != mrc::channel::Status::success) + { + break; + } + + co_yield std::move(value); + } + + co_return; + } +}; + +template +class CoroutineRunnableSource : public mrc::node::WritableAcceptor, + public mrc::node::ReadableProvider, + public mrc::node::SourceChannelOwner +{ + protected: + CoroutineRunnableSource() + { + // Set the default channel + this->set_channel(std::make_unique>()); + } + + // auto build_readable_generator(std::stop_token stop_token) + // -> mrc::coroutines::AsyncGenerator + // { + // while (!stop_token.stop_requested()) + // { + // co_yield mrc::coroutines::detail::VoidValue{}; + // } + + // co_return; + // } + + auto build_writable_receiver() -> std::shared_ptr> + { + return std::make_shared>([this](T&& value) { + return this->get_writable_edge()->await_write(std::move(value)); + }); + } +}; + +template +class AsyncioRunnable : public CoroutineRunnableSink, + public CoroutineRunnableSource, + public mrc::runnable::RunnableWithContext<> +{ + using state_t = mrc::runnable::Runnable::State; + using task_buffer_t = mrc::coroutines::ClosableRingBuffer; + + public: + AsyncioRunnable(size_t concurrency = 8) : m_concurrency(concurrency){}; + ~AsyncioRunnable() override = default; + + private: + void run(mrc::runnable::Context& ctx) override; + void on_state_update(const state_t& state) final; + + Task main_task(std::shared_ptr scheduler); + + Task process_one(InputT&& value, + std::shared_ptr> writer, + task_buffer_t& task_buffer, + std::shared_ptr on, + ExceptionCatcher& catcher); + + virtual mrc::coroutines::AsyncGenerator on_data(InputT&& value) = 0; + + std::stop_source m_stop_source; + + size_t m_concurrency{8}; +}; + +template +void AsyncioRunnable::run(mrc::runnable::Context& ctx) +{ + // auto& scheduler = ctx.scheduler(); + + // TODO(MDD): Eventually we should get this from the context object. For now, just create it directly + auto scheduler = std::make_shared(m_concurrency); + + // Now use the scheduler to run the main task until it is complete + scheduler->run_until_complete(this->main_task(scheduler)); + + // Need to drop the output edges + mrc::node::SourceProperties::release_edge_connection(); + mrc::node::SinkProperties::release_edge_connection(); +} + +template +Task AsyncioRunnable::main_task(std::shared_ptr scheduler) +{ + // Get the generator and receiver + auto input_generator = CoroutineRunnableSink::build_readable_generator(m_stop_source.get_token()); + auto output_receiver = CoroutineRunnableSource::build_writable_receiver(); + + // Create the task buffer to limit the number of running tasks + task_buffer_t task_buffer{{.capacity = m_concurrency}}; + + size_t i = 0; + + auto iter = co_await input_generator.begin(); + + coroutines::TaskContainer outstanding_tasks(scheduler); + + ExceptionCatcher catcher{}; + + while (not catcher.has_exception() and iter != input_generator.end()) + { + // Weird bug, cant directly move the value into the process_one call + auto data = std::move(*iter); + + // Wait for an available slot in the task buffer + co_await task_buffer.write(i); + + outstanding_tasks.start(this->process_one(std::move(data), output_receiver, task_buffer, scheduler, catcher)); + + // Advance the iterator + co_await ++iter; + ++i; + } + + // Close the buffer + task_buffer.close(); + + // Now block until all tasks are complete + co_await task_buffer.completed(); + + co_await outstanding_tasks.garbage_collect_and_yield_until_empty(); + + catcher.rethrow_next_exception(); +} + +template +Task AsyncioRunnable::process_one(InputT&& value, + std::shared_ptr> writer, + task_buffer_t& task_buffer, + std::shared_ptr on, + ExceptionCatcher& catcher) +{ + co_await on->yield(); + + try + { + // Call the on_data function + auto on_data_gen = this->on_data(std::move(value)); + + auto iter = co_await on_data_gen.begin(); + + while (iter != on_data_gen.end()) + { + // Weird bug, cant directly move the value into the async_write call + auto data = std::move(*iter); + + co_await writer->async_write(std::move(data)); + + // Advance the iterator + co_await ++iter; + } + } catch (...) + { + // TODO(cwharris): communicate error back to the runnable's main main task + catcher.set_exception(std::current_exception()); + } + + // Return the slot to the task buffer + co_await task_buffer.read(); +} + +template +void AsyncioRunnable::on_state_update(const state_t& state) +{ + switch (state) + { + case state_t::Stop: + // Do nothing, we wait for the upstream channel to return closed + // m_stop_source.request_stop(); + break; + + case state_t::Kill: + + m_stop_source.request_stop(); + break; + + default: + break; + } +} + +} // namespace mrc::pymrc diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp new file mode 100644 index 000000000..26e30fd58 --- /dev/null +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -0,0 +1,126 @@ +#pragma once + +#include "pymrc/coro.hpp" +#include "pymrc/utilities/acquire_gil.hpp" + +#include +#include +#include +#include +#include + +namespace py = pybind11; + +namespace mrc::pymrc { +class AsyncioScheduler : public mrc::coroutines::Scheduler +{ + public: + AsyncioScheduler(size_t concurrency) {} + + std::string description() const override + { + return "AsyncioScheduler"; + } + + void resume(std::coroutine_handle<> coroutine) override + { + if (coroutine.done()) + { + LOG(WARNING) << "AsyncioScheduler::resume() > Attempted to resume a completed coroutine"; + return; + } + + py::gil_scoped_acquire gil; + + auto& loop = this->get_loop(); + + // TODO(MDD): Check whether or not we need thread safe version + loop.attr("call_soon_threadsafe")(py::cpp_function([this, handle = std::move(coroutine)]() { + if (handle.done()) + { + LOG(WARNING) << "AsyncioScheduler::resume() > Attempted to resume a completed coroutine"; + return; + } + + py::gil_scoped_release nogil; + + handle.resume(); + })); + } + + mrc::pymrc::PyHolder& init_loop() + { + CHECK_EQ(PyGILState_Check(), 1) << "Must have the GIL when calling AsyncioScheduler::init_loop()"; + + std::unique_lock lock(m_mutex); + + if (m_loop) + { + return m_loop; + } + + auto asyncio_mod = py::module_::import("asyncio"); + + py::object loop; + + try + { + // Otherwise check if one is already allocated + loop = asyncio_mod.attr("get_running_loop")(); + } catch (std::runtime_error&) + { + // Need to create a loop + LOG(INFO) << "AsyncioScheduler::run() > Creating new event loop"; + + // Gets (or more likely, creates) an event loop and runs it forever until stop is called + loop = asyncio_mod.attr("new_event_loop")(); + + // Set the event loop as the current event loop + asyncio_mod.attr("set_event_loop")(loop); + } + + m_loop = std::move(loop); + + return m_loop; + } + + // Runs the task until its complete + void run_until_complete(coroutines::Task<>&& task) + { + mrc::pymrc::AcquireGIL gil; + + auto& loop = this->init_loop(); + + LOG(INFO) << "AsyncioScheduler::run() > Calling run_until_complete() on main_task()"; + + // Use the BoostFibersMainPyAwaitable to allow fibers to be progressed + loop.attr("run_until_complete")(mrc::pymrc::coro::BoostFibersMainPyAwaitable(std::move(task))); + } + + private: + std::coroutine_handle<> schedule_operation(Operation* operation) override + { + this->resume(std::move(operation->m_awaiting_coroutine)); + + return std::noop_coroutine(); + } + + mrc::pymrc::PyHolder& get_loop() + { + if (!m_loop) + { + throw std::runtime_error("Must call init_loop() before get_loop()"); + } + + // TODO(MDD): Check that we are on the same thread as the loop + return m_loop; + } + + std::mutex m_mutex; + + std::atomic_size_t m_outstanding{0}; + + mrc::pymrc::PyHolder m_loop; +}; + +} // namespace mrc::pymrc From 7eb5da7026a6f108763dd32f67f2c95e55dbb6f6 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 27 Oct 2023 16:34:30 +0000 Subject: [PATCH 12/50] remove unncessary using --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 88cd91773..baeb82a8b 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -18,9 +18,6 @@ namespace mrc::pymrc { -template -using Task = mrc::coroutines::Task; - class ExceptionCatcher { public: @@ -122,8 +119,8 @@ template class IReadable { public: - virtual ~IReadable() = default; - virtual Task async_read(T& value) = 0; + virtual ~IReadable() = default; + virtual coroutines::Task async_read(T& value) = 0; }; template @@ -134,7 +131,7 @@ class BoostFutureReader : public IReadable BoostFutureReader(FuncT&& fn) : m_awaiter(std::forward(fn)) {} - Task async_read(T& value) override + coroutines::Task async_read(T& value) override { co_return co_await m_awaiter(std::ref(value)); } @@ -147,8 +144,8 @@ template class IWritable { public: - virtual ~IWritable() = default; - virtual Task async_write(T&& value) = 0; + virtual ~IWritable() = default; + virtual coroutines::Task async_write(T&& value) = 0; }; template @@ -159,7 +156,7 @@ class BoostFutureWriter : public IWritable BoostFutureWriter(FuncT&& fn) : m_awaiter(std::forward(fn)) {} - Task async_write(T&& value) override + coroutines::Task async_write(T&& value) override { co_return co_await m_awaiter(std::move(value)); } @@ -252,13 +249,13 @@ class AsyncioRunnable : public CoroutineRunnableSink, void run(mrc::runnable::Context& ctx) override; void on_state_update(const state_t& state) final; - Task main_task(std::shared_ptr scheduler); + coroutines::Task<> main_task(std::shared_ptr scheduler); - Task process_one(InputT&& value, - std::shared_ptr> writer, - task_buffer_t& task_buffer, - std::shared_ptr on, - ExceptionCatcher& catcher); + coroutines::Task<> process_one(InputT&& value, + std::shared_ptr> writer, + task_buffer_t& task_buffer, + std::shared_ptr on, + ExceptionCatcher& catcher); virtual mrc::coroutines::AsyncGenerator on_data(InputT&& value) = 0; @@ -284,7 +281,7 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) } template -Task AsyncioRunnable::main_task(std::shared_ptr scheduler) +coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr scheduler) { // Get the generator and receiver auto input_generator = CoroutineRunnableSink::build_readable_generator(m_stop_source.get_token()); @@ -328,11 +325,11 @@ Task AsyncioRunnable::main_task(std::shared_ptr -Task AsyncioRunnable::process_one(InputT&& value, - std::shared_ptr> writer, - task_buffer_t& task_buffer, - std::shared_ptr on, - ExceptionCatcher& catcher) +coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, + std::shared_ptr> writer, + task_buffer_t& task_buffer, + std::shared_ptr on, + ExceptionCatcher& catcher) { co_await on->yield(); From 622fbac6052d51367fd2c00f0e8df93dae9878a3 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 27 Oct 2023 16:35:33 +0000 Subject: [PATCH 13/50] adjust includes for scheduler.cpp/hpp --- cpp/mrc/include/mrc/coroutines/scheduler.hpp | 2 -- cpp/mrc/src/public/coroutines/scheduler.cpp | 4 ---- 2 files changed, 6 deletions(-) diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index f799db83a..0dc7660ea 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -17,8 +17,6 @@ #pragma once -#include "mrc/coroutines/task.hpp" - #include #include #include diff --git a/cpp/mrc/src/public/coroutines/scheduler.cpp b/cpp/mrc/src/public/coroutines/scheduler.cpp index f4f7776ac..da3b7b35d 100644 --- a/cpp/mrc/src/public/coroutines/scheduler.cpp +++ b/cpp/mrc/src/public/coroutines/scheduler.cpp @@ -17,14 +17,10 @@ #include "mrc/coroutines/scheduler.hpp" -#include "mrc/coroutines/task_container.hpp" - #include -#include #include #include -#include namespace mrc::coroutines { From a2a093844dc834c21127886a1a55c257424589aa Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 27 Oct 2023 17:14:31 +0000 Subject: [PATCH 14/50] remove unncessary bool return for await_suspend --- python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index baeb82a8b..fb11a332b 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -85,7 +85,7 @@ class BoostFutureAwaiter return false; } - bool await_suspend(std::coroutine_handle<> continuation) noexcept + void await_suspend(std::coroutine_handle<> continuation) noexcept { // Launch a new fiber that waits on the future and then resumes the coroutine boost::fibers::async( @@ -98,8 +98,6 @@ class BoostFutureAwaiter continuation.resume(); }, std::move(continuation)); - - return true; } auto await_resume() noexcept From 64a346b2e3d90b270994fa18105955415dd479a9 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 27 Oct 2023 17:27:09 +0000 Subject: [PATCH 15/50] add copyright header --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 17 +++++++++++++++++ .../_pymrc/include/pymrc/asyncio_scheduler.hpp | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index fb11a332b..633b1dabe 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -1,3 +1,20 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include "pymrc/asyncio_scheduler.hpp" diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index 26e30fd58..e09ec0e5e 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -1,3 +1,20 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include "pymrc/coro.hpp" From e157311131afa619b98cc3a31d0690a96cb7deb0 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Mon, 30 Oct 2023 18:32:34 +0000 Subject: [PATCH 16/50] asyncio_runnable test --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 4 +- .../include/pymrc/asyncio_scheduler.hpp | 35 +++--- python/mrc/_pymrc/tests/CMakeLists.txt | 1 + .../_pymrc/tests/test_asyncio_runnable.cpp | 119 ++++++++++++++++++ 4 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 python/mrc/_pymrc/tests/test_asyncio_runnable.cpp diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 633b1dabe..1b8dcfcc0 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -291,8 +291,8 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) scheduler->run_until_complete(this->main_task(scheduler)); // Need to drop the output edges - mrc::node::SourceProperties::release_edge_connection(); - mrc::node::SinkProperties::release_edge_connection(); + mrc::node::SourceProperties::release_edge_connection(); + mrc::node::SinkProperties::release_edge_connection(); } template diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index e09ec0e5e..95ab6d909 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -25,6 +25,9 @@ #include #include #include +#include + +#include namespace py = pybind11; @@ -78,25 +81,29 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler auto asyncio_mod = py::module_::import("asyncio"); - py::object loop; + auto loop = [asyncio_mod]() -> py::object { + try + { + return asyncio_mod.attr("get_running_loop")(); + } catch (...) + { + return py::none(); + } + }(); - try + if (not loop.is_none()) { - // Otherwise check if one is already allocated - loop = asyncio_mod.attr("get_running_loop")(); - } catch (std::runtime_error&) - { - // Need to create a loop - LOG(INFO) << "AsyncioScheduler::run() > Creating new event loop"; + throw std::runtime_error("asyncio loop already running, but runnable is expected to create it."); + } - // Gets (or more likely, creates) an event loop and runs it forever until stop is called - loop = asyncio_mod.attr("new_event_loop")(); + // Need to create a loop + LOG(INFO) << "AsyncioScheduler::run() > Creating new event loop"; - // Set the event loop as the current event loop - asyncio_mod.attr("set_event_loop")(loop); - } + // Gets (or more likely, creates) an event loop and runs it forever until stop is called + m_loop = asyncio_mod.attr("new_event_loop")(); - m_loop = std::move(loop); + // Set the event loop as the current event loop + asyncio_mod.attr("set_event_loop")(m_loop); return m_loop; } diff --git a/python/mrc/_pymrc/tests/CMakeLists.txt b/python/mrc/_pymrc/tests/CMakeLists.txt index f40e20d72..02186de90 100644 --- a/python/mrc/_pymrc/tests/CMakeLists.txt +++ b/python/mrc/_pymrc/tests/CMakeLists.txt @@ -21,6 +21,7 @@ add_subdirectory(coro) # Keep all source files sorted!!! add_executable(test_pymrc + test_asyncio_runnable.cpp test_codable_pyobject.cpp test_executor.cpp test_main.cpp diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp new file mode 100644 index 000000000..1902eef58 --- /dev/null +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -0,0 +1,119 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "test_pymrc.hpp" + +#include "pymrc/asyncio_runnable.hpp" +#include "pymrc/executor.hpp" +#include "pymrc/pipeline.hpp" +#include "pymrc/port_builders.hpp" +#include "pymrc/types.hpp" + +#include "mrc/node/rx_node.hpp" +#include "mrc/node/rx_sink.hpp" +#include "mrc/node/rx_sink_base.hpp" +#include "mrc/node/rx_source.hpp" +#include "mrc/node/rx_source_base.hpp" +#include "mrc/options/options.hpp" +#include "mrc/options/topology.hpp" +#include "mrc/segment/builder.hpp" +#include "mrc/segment/object.hpp" +#include "mrc/types.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include // IWYU pragma: keep +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace py = pybind11; +namespace pymrc = mrc::pymrc; +using namespace std::string_literals; +using namespace py::literals; + +PYMRC_TEST_CLASS(AsyncioRunnable); + +class MyAsyncioRunnable : public pymrc::AsyncioRunnable +{ + mrc::coroutines::AsyncGenerator on_data(int&& value) override + { + co_yield value; + co_yield value * 2; + }; +}; + +TEST_F(TestAsyncioRunnable, Execute) +{ + std::atomic counter = 0; + pymrc::Pipeline p; + + pybind11::module_::import("mrc.core.coro"); + + auto init = [&counter](mrc::segment::IBuilder& seg) { + auto src = seg.make_source("src", [](rxcpp::subscriber& s) { + if (s.is_subscribed()) + { + s.on_next(5); + s.on_next(10); + s.on_next(7); + } + + s.on_completed(); + }); + + auto internal = seg.construct_object("internal"); + + auto sink = seg.make_sink("sink", [&counter](unsigned int x) { + counter.fetch_add(x, std::memory_order_relaxed); + }); + + seg.make_edge(src, internal); + seg.make_edge(internal, sink); + }; + + p.make_segment("seg1"s, init); + p.make_segment("seg2"s, init); + + auto options = std::make_shared(); + options->topology().user_cpuset("0"); + // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. + options->engine_factories().set_default_engine_type(runnable::EngineType::Thread); + + pymrc::Executor exec{options}; + exec.register_pipeline(p); + + exec.start(); + exec.join(); + + EXPECT_EQ(counter, 132); +} \ No newline at end of file From d964e006175b93673ebf051082e6028ffde61861 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 31 Oct 2023 02:55:17 +0000 Subject: [PATCH 17/50] fix imports --- .../_pymrc/tests/test_asyncio_runnable.cpp | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index 1902eef58..18ef222bc 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -20,41 +20,29 @@ #include "pymrc/asyncio_runnable.hpp" #include "pymrc/executor.hpp" #include "pymrc/pipeline.hpp" -#include "pymrc/port_builders.hpp" #include "pymrc/types.hpp" -#include "mrc/node/rx_node.hpp" +#include "mrc/coroutines/async_generator.hpp" #include "mrc/node/rx_sink.hpp" -#include "mrc/node/rx_sink_base.hpp" #include "mrc/node/rx_source.hpp" -#include "mrc/node/rx_source_base.hpp" +#include "mrc/options/engine_groups.hpp" #include "mrc/options/options.hpp" #include "mrc/options/topology.hpp" +#include "mrc/runnable/types.hpp" #include "mrc/segment/builder.hpp" #include "mrc/segment/object.hpp" -#include "mrc/types.hpp" -#include -#include +#include #include -#include -#include #include #include -#include // IWYU pragma: keep +#include #include #include -#include -#include -#include -#include +#include #include -#include #include -#include -#include -#include namespace py = pybind11; namespace pymrc = mrc::pymrc; From d45658873f9f58d84615b5d64d37bee2206bb581 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 31 Oct 2023 16:45:05 +0000 Subject: [PATCH 18/50] remove unused includes --- python/mrc/_pymrc/tests/test_asyncio_runnable.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index 18ef222bc..e20715c4e 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -36,7 +36,6 @@ #include #include #include -#include #include #include From 222edcdc60b55dd7f1d0bfb571d91dc60bfe4364 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 31 Oct 2023 21:21:59 +0000 Subject: [PATCH 19/50] move exception catcher to MRC --- cpp/mrc/CMakeLists.txt | 1 + .../mrc/exceptions/exception_catcher.hpp | 34 ++++++++++++++++ .../public/exceptions/exception_catcher.cpp | 33 ++++++++++++++++ .../_pymrc/include/pymrc/asyncio_runnable.hpp | 39 +------------------ 4 files changed, 70 insertions(+), 37 deletions(-) create mode 100644 cpp/mrc/include/mrc/exceptions/exception_catcher.hpp create mode 100644 cpp/mrc/src/public/exceptions/exception_catcher.cpp diff --git a/cpp/mrc/CMakeLists.txt b/cpp/mrc/CMakeLists.txt index 93909e8c6..46cb31787 100644 --- a/cpp/mrc/CMakeLists.txt +++ b/cpp/mrc/CMakeLists.txt @@ -124,6 +124,7 @@ add_library(libmrc src/public/cuda/sync.cpp src/public/edge/edge_adapter_registry.cpp src/public/edge/edge_builder.cpp + src/public/exceptions/exception_catcher.cpp src/public/manifold/manifold.cpp src/public/memory/buffer_view.cpp src/public/memory/codable/buffer.cpp diff --git a/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp new file mode 100644 index 000000000..cf1c48f75 --- /dev/null +++ b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp @@ -0,0 +1,34 @@ +#include +#include +#include + +namespace mrc { + +/** + * @brief A utility for catching out-of-stack exceptions in a thread-safe manner such that they + * can be checked and throw from a parent thread. + */ +class ExceptionCatcher +{ + public: + /** + * @brief "catches" an exception to the catcher + */ + void push_exception(std::exception_ptr ex); + + /** + * @brief checks to see if any exceptions have been "caught" by the catcher. + */ + bool has_exception(); + + /** + * @brief rethrows the next exception (in the order in which it was "caught"). + */ + void rethrow_next_exception(); + + private: + std::mutex m_mutex{}; + std::queue m_exceptions{}; +}; + +} // namespace mrc diff --git a/cpp/mrc/src/public/exceptions/exception_catcher.cpp b/cpp/mrc/src/public/exceptions/exception_catcher.cpp new file mode 100644 index 000000000..49769436d --- /dev/null +++ b/cpp/mrc/src/public/exceptions/exception_catcher.cpp @@ -0,0 +1,33 @@ +#include + +namespace mrc { + +void ExceptionCatcher::push_exception(std::exception_ptr ex) +{ + auto lock = std::lock_guard(m_mutex); + m_exceptions.push(ex); +} + +bool ExceptionCatcher::has_exception() +{ + auto lock = std::lock_guard(m_mutex); + return not m_exceptions.empty(); +} + +void ExceptionCatcher::rethrow_next_exception() +{ + auto lock = std::lock_guard(m_mutex); + + if (m_exceptions.empty()) + { + return; + } + + auto ex = m_exceptions.front(); + + m_exceptions.pop(); + + std::rethrow_exception(ex); +} + +} // namespace mrc diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 1b8dcfcc0..5979cad12 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -35,41 +36,6 @@ namespace mrc::pymrc { -class ExceptionCatcher -{ - public: - void set_exception(std::exception_ptr ex) - { - auto lock = std::lock_guard(m_mutex); - m_exceptions.push(ex); - } - - bool has_exception() - { - auto lock = std::lock_guard(m_mutex); - return not m_exceptions.empty(); - } - - void rethrow_next_exception() - { - auto lock = std::lock_guard(m_mutex); - - if (m_exceptions.empty()) - { - return; - } - - auto ex = m_exceptions.front(); - m_exceptions.pop(); - - std::rethrow_exception(ex); - } - - private: - std::mutex m_mutex{}; - std::queue m_exceptions{}; -}; - template class BoostFutureAwaiter { @@ -367,8 +333,7 @@ coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, } } catch (...) { - // TODO(cwharris): communicate error back to the runnable's main main task - catcher.set_exception(std::current_exception()); + catcher.push_exception(std::current_exception()); } // Return the slot to the task buffer From fc49a12c7fc0d6046c679c31b6f54b1b365f44b8 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 31 Oct 2023 22:33:09 +0000 Subject: [PATCH 20/50] rename and document BoostFutureAwaiter -> BoostFutureAwaitableOperation --- .../mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 5979cad12..13c4d4c0a 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -36,14 +36,21 @@ namespace mrc::pymrc { +/** + * @brief A wrapper for executing a function as an async boost fiber, the result of which is a + * C++20 coroutine awaiter. + */ template -class BoostFutureAwaiter +class BoostFutureAwaitableOperation { class Awaiter; public: - BoostFutureAwaiter(std::function fn) : m_fn(std::move(fn)) {} + BoostFutureAwaitableOperation(std::function fn) : m_fn(std::move(fn)) {} + /** + * @brief Calls the wrapped function as an asyncboost fiber and returns a C++20 coroutine awaiter. + */ template auto operator()(ArgsT&&... args) -> Awaiter { @@ -118,7 +125,7 @@ class BoostFutureReader : public IReadable } private: - BoostFutureAwaiter m_awaiter; + BoostFutureAwaitableOperation m_awaiter; }; template @@ -143,7 +150,7 @@ class BoostFutureWriter : public IWritable } private: - BoostFutureAwaiter m_awaiter; + BoostFutureAwaitableOperation m_awaiter; }; template From 5f5f7ae469034c3a8c53be58c58f110a445c95c3 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 04:01:48 +0000 Subject: [PATCH 21/50] remove unneccessary ireader and iwriter interfaces --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 13c4d4c0a..fdb01e05d 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -104,22 +104,14 @@ class BoostFutureAwaitableOperation }; template -class IReadable -{ - public: - virtual ~IReadable() = default; - virtual coroutines::Task async_read(T& value) = 0; -}; - -template -class BoostFutureReader : public IReadable +class BoostFutureReader { public: template BoostFutureReader(FuncT&& fn) : m_awaiter(std::forward(fn)) {} - coroutines::Task async_read(T& value) override + coroutines::Task async_read(T& value) { co_return co_await m_awaiter(std::ref(value)); } @@ -129,22 +121,14 @@ class BoostFutureReader : public IReadable }; template -class IWritable -{ - public: - virtual ~IWritable() = default; - virtual coroutines::Task async_write(T&& value) = 0; -}; - -template -class BoostFutureWriter : public IWritable +class BoostFutureWriter { public: template BoostFutureWriter(FuncT&& fn) : m_awaiter(std::forward(fn)) {} - coroutines::Task async_write(T&& value) override + coroutines::Task async_write(T&& value) { co_return co_await m_awaiter(std::move(value)); } @@ -213,7 +197,7 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, // co_return; // } - auto build_writable_receiver() -> std::shared_ptr> + auto build_writable_receiver() -> std::shared_ptr> { return std::make_shared>([this](T&& value) { return this->get_writable_edge()->await_write(std::move(value)); @@ -240,7 +224,7 @@ class AsyncioRunnable : public CoroutineRunnableSink, coroutines::Task<> main_task(std::shared_ptr scheduler); coroutines::Task<> process_one(InputT&& value, - std::shared_ptr> writer, + std::shared_ptr> writer, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher); @@ -314,7 +298,7 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, - std::shared_ptr> writer, + std::shared_ptr> writer, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher) From 48dad3d86685d61c56a145fc322435107b642344 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 04:07:02 +0000 Subject: [PATCH 22/50] simplify asyncio_runnable --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index fdb01e05d..401e7df01 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -143,7 +143,10 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, public mrc::node::SinkChannelOwner { protected: - CoroutineRunnableSink() + CoroutineRunnableSink() : + m_reader([this](T& value) { + return this->get_readable_edge()->await_read(value); + }) { // Set the default channel this->set_channel(std::make_unique>()); @@ -151,16 +154,12 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, auto build_readable_generator(std::stop_token stop_token) -> mrc::coroutines::AsyncGenerator { - auto read_awaiter = BoostFutureReader([this](T& value) { - return this->get_readable_edge()->await_read(value); - }); - while (!stop_token.stop_requested()) { T value; // Pull a message off of the upstream channel - auto status = co_await read_awaiter.async_read(std::ref(value)); + auto status = co_await m_reader.async_read(std::ref(value)); if (status != mrc::channel::Status::success) { @@ -172,6 +171,9 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, co_return; } + + private: + BoostFutureReader m_reader; }; template @@ -184,25 +186,19 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, { // Set the default channel this->set_channel(std::make_unique>()); - } - - // auto build_readable_generator(std::stop_token stop_token) - // -> mrc::coroutines::AsyncGenerator - // { - // while (!stop_token.stop_requested()) - // { - // co_yield mrc::coroutines::detail::VoidValue{}; - // } - - // co_return; - // } - auto build_writable_receiver() -> std::shared_ptr> - { - return std::make_shared>([this](T&& value) { + m_writer = std::make_shared>([this](T&& value) { return this->get_writable_edge()->await_write(std::move(value)); }); } + + auto get_writable_receiver() -> std::shared_ptr> + { + return m_writer; + } + + private: + std::shared_ptr> m_writer; }; template @@ -257,7 +253,7 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr::build_readable_generator(m_stop_source.get_token()); - auto output_receiver = CoroutineRunnableSource::build_writable_receiver(); + auto output_receiver = CoroutineRunnableSource::get_writable_receiver(); // Create the task buffer to limit the number of running tasks task_buffer_t task_buffer{{.capacity = m_concurrency}}; From 521abc211d46e8b69cf7e17ee4c278c759530cdc Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 04:34:00 +0000 Subject: [PATCH 23/50] simplify asyncio_runnable --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 401e7df01..7209986a3 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -152,24 +152,9 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, this->set_channel(std::make_unique>()); } - auto build_readable_generator(std::stop_token stop_token) -> mrc::coroutines::AsyncGenerator + coroutines::Task async_read(T& value) { - while (!stop_token.stop_requested()) - { - T value; - - // Pull a message off of the upstream channel - auto status = co_await m_reader.async_read(std::ref(value)); - - if (status != mrc::channel::Status::success) - { - break; - } - - co_yield std::move(value); - } - - co_return; + co_return co_await m_reader.async_read(std::ref(value)); } private: @@ -252,33 +237,30 @@ template coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr scheduler) { // Get the generator and receiver - auto input_generator = CoroutineRunnableSink::build_readable_generator(m_stop_source.get_token()); auto output_receiver = CoroutineRunnableSource::get_writable_receiver(); // Create the task buffer to limit the number of running tasks task_buffer_t task_buffer{{.capacity = m_concurrency}}; - size_t i = 0; - - auto iter = co_await input_generator.begin(); - coroutines::TaskContainer outstanding_tasks(scheduler); ExceptionCatcher catcher{}; - while (not catcher.has_exception() and iter != input_generator.end()) - { - // Weird bug, cant directly move the value into the process_one call - auto data = std::move(*iter); + while (not m_stop_source.stop_requested() and not catcher.has_exception()) { + + InputT data; + + auto read_status = co_await this->async_read(data); + + if (read_status != mrc::channel::Status::success) + { + break; + } // Wait for an available slot in the task buffer - co_await task_buffer.write(i); + co_await task_buffer.write(0); outstanding_tasks.start(this->process_one(std::move(data), output_receiver, task_buffer, scheduler, catcher)); - - // Advance the iterator - co_await ++iter; - ++i; } // Close the buffer From a0d136e6a5079246bc00f62181d820a986f4816f Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 04:36:20 +0000 Subject: [PATCH 24/50] simplify asyncio_runnable --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 7209986a3..719f92b90 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -167,23 +167,22 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, public mrc::node::SourceChannelOwner { protected: - CoroutineRunnableSource() + CoroutineRunnableSource() : + m_writer([this](T&& value) { + return this->get_writable_edge()->await_write(std::move(value)); + }) { // Set the default channel this->set_channel(std::make_unique>()); - - m_writer = std::make_shared>([this](T&& value) { - return this->get_writable_edge()->await_write(std::move(value)); - }); } - auto get_writable_receiver() -> std::shared_ptr> + coroutines::Task async_write(T&& value) { - return m_writer; + co_return co_await m_writer.async_write(std::move(value)); } private: - std::shared_ptr> m_writer; + BoostFutureWriter m_writer; }; template @@ -205,7 +204,6 @@ class AsyncioRunnable : public CoroutineRunnableSink, coroutines::Task<> main_task(std::shared_ptr scheduler); coroutines::Task<> process_one(InputT&& value, - std::shared_ptr> writer, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher); @@ -236,9 +234,6 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) template coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr scheduler) { - // Get the generator and receiver - auto output_receiver = CoroutineRunnableSource::get_writable_receiver(); - // Create the task buffer to limit the number of running tasks task_buffer_t task_buffer{{.capacity = m_concurrency}}; @@ -246,8 +241,8 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptrasync_read(data); @@ -260,7 +255,7 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptrprocess_one(std::move(data), output_receiver, task_buffer, scheduler, catcher)); + outstanding_tasks.start(this->process_one(std::move(data), task_buffer, scheduler, catcher)); } // Close the buffer @@ -276,7 +271,6 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, - std::shared_ptr> writer, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher) @@ -295,7 +289,7 @@ coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, // Weird bug, cant directly move the value into the async_write call auto data = std::move(*iter); - co_await writer->async_write(std::move(data)); + co_await this->async_write(std::move(data)); // Advance the iterator co_await ++iter; From e1da11204f73630a263a37d62c84532e40a79857 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 04:40:08 +0000 Subject: [PATCH 25/50] simplify asyncio_runnable --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 54 ++++--------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 719f92b90..18000aa67 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -103,40 +103,6 @@ class BoostFutureAwaitableOperation std::function m_fn; }; -template -class BoostFutureReader -{ - public: - template - BoostFutureReader(FuncT&& fn) : m_awaiter(std::forward(fn)) - {} - - coroutines::Task async_read(T& value) - { - co_return co_await m_awaiter(std::ref(value)); - } - - private: - BoostFutureAwaitableOperation m_awaiter; -}; - -template -class BoostFutureWriter -{ - public: - template - BoostFutureWriter(FuncT&& fn) : m_awaiter(std::forward(fn)) - {} - - coroutines::Task async_write(T&& value) - { - co_return co_await m_awaiter(std::move(value)); - } - - private: - BoostFutureAwaitableOperation m_awaiter; -}; - template class CoroutineRunnableSink : public mrc::node::WritableProvider, public mrc::node::ReadableAcceptor, @@ -144,7 +110,7 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, { protected: CoroutineRunnableSink() : - m_reader([this](T& value) { + m_read_async([this](T& value) { return this->get_readable_edge()->await_read(value); }) { @@ -152,13 +118,13 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, this->set_channel(std::make_unique>()); } - coroutines::Task async_read(T& value) + coroutines::Task read_async(T& value) { - co_return co_await m_reader.async_read(std::ref(value)); + co_return co_await m_read_async(std::ref(value)); } private: - BoostFutureReader m_reader; + BoostFutureAwaitableOperation m_read_async; }; template @@ -168,7 +134,7 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, { protected: CoroutineRunnableSource() : - m_writer([this](T&& value) { + m_write_async([this](T&& value) { return this->get_writable_edge()->await_write(std::move(value)); }) { @@ -176,13 +142,13 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, this->set_channel(std::make_unique>()); } - coroutines::Task async_write(T&& value) + coroutines::Task write_async(T&& value) { - co_return co_await m_writer.async_write(std::move(value)); + co_return co_await m_write_async(std::move(value)); } private: - BoostFutureWriter m_writer; + BoostFutureAwaitableOperation m_write_async; }; template @@ -245,7 +211,7 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptrasync_read(data); + auto read_status = co_await this->read_async(data); if (read_status != mrc::channel::Status::success) { @@ -289,7 +255,7 @@ coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, // Weird bug, cant directly move the value into the async_write call auto data = std::move(*iter); - co_await this->async_write(std::move(data)); + co_await this->write_async(std::move(data)); // Advance the iterator co_await ++iter; From 2126de974de1c245c3db99637e55ae77bdfc4e1b Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 05:00:58 +0000 Subject: [PATCH 26/50] rename coroutinerunnable sink/source to async sink/source, add docs --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 18000aa67..3b45cf280 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -103,13 +103,16 @@ class BoostFutureAwaitableOperation std::function m_fn; }; +/** + * @brief A MRC Sink which receives from a channel with an awaitable internal interface. + */ template -class CoroutineRunnableSink : public mrc::node::WritableProvider, - public mrc::node::ReadableAcceptor, - public mrc::node::SinkChannelOwner +class AsyncSink : public mrc::node::WritableProvider, + public mrc::node::ReadableAcceptor, + public mrc::node::SinkChannelOwner { protected: - CoroutineRunnableSink() : + AsyncSink() : m_read_async([this](T& value) { return this->get_readable_edge()->await_read(value); }) @@ -127,13 +130,16 @@ class CoroutineRunnableSink : public mrc::node::WritableProvider, BoostFutureAwaitableOperation m_read_async; }; +/** + * @brief A MRC Source which produces to a channel with an awaitable internal interface. + */ template -class CoroutineRunnableSource : public mrc::node::WritableAcceptor, - public mrc::node::ReadableProvider, - public mrc::node::SourceChannelOwner +class AsyncSource : public mrc::node::WritableAcceptor, + public mrc::node::ReadableProvider, + public mrc::node::SourceChannelOwner { protected: - CoroutineRunnableSource() : + AsyncSource() : m_write_async([this](T&& value) { return this->get_writable_edge()->await_write(std::move(value)); }) @@ -151,9 +157,12 @@ class CoroutineRunnableSource : public mrc::node::WritableAcceptor, BoostFutureAwaitableOperation m_write_async; }; +/** + * @brief A MRC Runnable base class which hosts it's own asyncio loop and exposes a flatmap hook + */ template -class AsyncioRunnable : public CoroutineRunnableSink, - public CoroutineRunnableSource, +class AsyncioRunnable : public AsyncSink, + public AsyncSource, public mrc::runnable::RunnableWithContext<> { using state_t = mrc::runnable::Runnable::State; From 1bf51c8adaa66cf89b4f519088a189ba45be0373 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 14:45:33 +0000 Subject: [PATCH 27/50] adjust comments --- python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 3b45cf280..873c6b7aa 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -104,7 +104,7 @@ class BoostFutureAwaitableOperation }; /** - * @brief A MRC Sink which receives from a channel with an awaitable internal interface. + * @brief A MRC Sink which receives from a channel using an awaitable interface. */ template class AsyncSink : public mrc::node::WritableProvider, @@ -131,7 +131,7 @@ class AsyncSink : public mrc::node::WritableProvider, }; /** - * @brief A MRC Source which produces to a channel with an awaitable internal interface. + * @brief A MRC Source which produces to a channel using an awaitable interface. */ template class AsyncSource : public mrc::node::WritableAcceptor, From 760c314d5cbdaf876e084dc89c0ee648270d1bfe Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 14:46:57 +0000 Subject: [PATCH 28/50] remove concurrency from asyncioscheduler --- python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 2 +- python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 873c6b7aa..e17164432 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -196,7 +196,7 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) // auto& scheduler = ctx.scheduler(); // TODO(MDD): Eventually we should get this from the context object. For now, just create it directly - auto scheduler = std::make_shared(m_concurrency); + auto scheduler = std::make_shared(); // Now use the scheduler to run the main task until it is complete scheduler->run_until_complete(this->main_task(scheduler)); diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index 95ab6d909..a84dfea84 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -35,8 +35,6 @@ namespace mrc::pymrc { class AsyncioScheduler : public mrc::coroutines::Scheduler { public: - AsyncioScheduler(size_t concurrency) {} - std::string description() const override { return "AsyncioScheduler"; From b835b2455ca56d1f32ee27a63388b7a32f8b36e9 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 15:22:18 +0000 Subject: [PATCH 29/50] move ownership of event loop out of asyncioscheduler --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 40 ++++++++-- .../include/pymrc/asyncio_scheduler.hpp | 74 +------------------ 2 files changed, 38 insertions(+), 76 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index e17164432..d83b4f477 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -18,6 +18,7 @@ #pragma once #include "pymrc/asyncio_scheduler.hpp" +#include "pymrc/utilities/object_wrappers.hpp" #include #include @@ -193,13 +194,42 @@ class AsyncioRunnable : public AsyncSink, template void AsyncioRunnable::run(mrc::runnable::Context& ctx) { - // auto& scheduler = ctx.scheduler(); + { + py::gil_scoped_acquire gil; + + auto asyncio = py::module_::import("asyncio"); + + auto loop = [](auto& asyncio) -> PyObjectHolder { + try + { + return asyncio.attr("get_running_loop")(); + } catch (...) + { + return py::none(); + } + }(asyncio); + + if (not loop.is_none()) + { + throw std::runtime_error("asyncio loop already running, but runnable is expected to create it."); + } - // TODO(MDD): Eventually we should get this from the context object. For now, just create it directly - auto scheduler = std::make_shared(); + // Need to create a loop + LOG(INFO) << "AsyncioRunnable::run() > Creating new event loop"; - // Now use the scheduler to run the main task until it is complete - scheduler->run_until_complete(this->main_task(scheduler)); + // Gets (or more likely, creates) an event loop and runs it forever until stop is called + loop = asyncio.attr("new_event_loop")(); + + // Set the event loop as the current event loop + asyncio.attr("set_event_loop")(loop); + + // TODO(MDD): Eventually we should get this from the context object. For now, just create it directly + auto scheduler = std::make_shared(loop); + + auto py_awaitable = coro::BoostFibersMainPyAwaitable(this->main_task(scheduler)); + + loop.attr("run_until_complete")(std::move(py_awaitable)); + } // Need to drop the output edges mrc::node::SourceProperties::release_edge_connection(); diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index a84dfea84..b0e486c83 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -35,6 +35,8 @@ namespace mrc::pymrc { class AsyncioScheduler : public mrc::coroutines::Scheduler { public: + AsyncioScheduler(PyObjectHolder loop) : m_loop(std::move(loop)) {} + std::string description() const override { return "AsyncioScheduler"; @@ -50,10 +52,8 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler py::gil_scoped_acquire gil; - auto& loop = this->get_loop(); - // TODO(MDD): Check whether or not we need thread safe version - loop.attr("call_soon_threadsafe")(py::cpp_function([this, handle = std::move(coroutine)]() { + m_loop.attr("call_soon_threadsafe")(py::cpp_function([this, handle = std::move(coroutine)]() { if (handle.done()) { LOG(WARNING) << "AsyncioScheduler::resume() > Attempted to resume a completed coroutine"; @@ -66,59 +66,6 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler })); } - mrc::pymrc::PyHolder& init_loop() - { - CHECK_EQ(PyGILState_Check(), 1) << "Must have the GIL when calling AsyncioScheduler::init_loop()"; - - std::unique_lock lock(m_mutex); - - if (m_loop) - { - return m_loop; - } - - auto asyncio_mod = py::module_::import("asyncio"); - - auto loop = [asyncio_mod]() -> py::object { - try - { - return asyncio_mod.attr("get_running_loop")(); - } catch (...) - { - return py::none(); - } - }(); - - if (not loop.is_none()) - { - throw std::runtime_error("asyncio loop already running, but runnable is expected to create it."); - } - - // Need to create a loop - LOG(INFO) << "AsyncioScheduler::run() > Creating new event loop"; - - // Gets (or more likely, creates) an event loop and runs it forever until stop is called - m_loop = asyncio_mod.attr("new_event_loop")(); - - // Set the event loop as the current event loop - asyncio_mod.attr("set_event_loop")(m_loop); - - return m_loop; - } - - // Runs the task until its complete - void run_until_complete(coroutines::Task<>&& task) - { - mrc::pymrc::AcquireGIL gil; - - auto& loop = this->init_loop(); - - LOG(INFO) << "AsyncioScheduler::run() > Calling run_until_complete() on main_task()"; - - // Use the BoostFibersMainPyAwaitable to allow fibers to be progressed - loop.attr("run_until_complete")(mrc::pymrc::coro::BoostFibersMainPyAwaitable(std::move(task))); - } - private: std::coroutine_handle<> schedule_operation(Operation* operation) override { @@ -127,21 +74,6 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler return std::noop_coroutine(); } - mrc::pymrc::PyHolder& get_loop() - { - if (!m_loop) - { - throw std::runtime_error("Must call init_loop() before get_loop()"); - } - - // TODO(MDD): Check that we are on the same thread as the loop - return m_loop; - } - - std::mutex m_mutex; - - std::atomic_size_t m_outstanding{0}; - mrc::pymrc::PyHolder m_loop; }; From 8be2867aaf7b43faad0b572af15cc4abd795ff6f Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 15:24:22 +0000 Subject: [PATCH 30/50] add info log --- python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index d83b4f477..117106863 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -228,6 +228,8 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) auto py_awaitable = coro::BoostFibersMainPyAwaitable(this->main_task(scheduler)); + LOG(INFO) << "AsyncioRunnable::run() > Calling run_until_complete() on main_task()"; + loop.attr("run_until_complete")(std::move(py_awaitable)); } From efd7fe9b422f5c396ddf55c9b691621772a7448a Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 16:08:50 +0000 Subject: [PATCH 31/50] fix bug related to r-value reference and suspended coroutine --- python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 117106863..f9b481a7d 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -179,7 +179,7 @@ class AsyncioRunnable : public AsyncSink, coroutines::Task<> main_task(std::shared_ptr scheduler); - coroutines::Task<> process_one(InputT&& value, + coroutines::Task<> process_one(InputT value, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher); @@ -277,7 +277,7 @@ coroutines::Task<> AsyncioRunnable::main_task(std::shared_ptr -coroutines::Task<> AsyncioRunnable::process_one(InputT&& value, +coroutines::Task<> AsyncioRunnable::process_one(InputT value, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher) From e509bbbe87bfff5afb042bd2b1038f4c665cf4a6 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 16:55:18 +0000 Subject: [PATCH 32/50] add python-coroutine-based test for asynciorunnable --- .../_pymrc/tests/test_asyncio_runnable.cpp | 100 ++++++++++++++++-- 1 file changed, 94 insertions(+), 6 deletions(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index e20715c4e..265876f32 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -21,6 +21,7 @@ #include "pymrc/executor.hpp" #include "pymrc/pipeline.hpp" #include "pymrc/types.hpp" +#include "pymrc/utilities/object_wrappers.hpp" #include "mrc/coroutines/async_generator.hpp" #include "mrc/node/rx_sink.hpp" @@ -34,6 +35,9 @@ #include #include +#include +#include +#include #include #include #include @@ -55,30 +59,114 @@ class MyAsyncioRunnable : public pymrc::AsyncioRunnable mrc::coroutines::AsyncGenerator on_data(int&& value) override { co_yield value; - co_yield value * 2; }; }; -TEST_F(TestAsyncioRunnable, Execute) +// TEST_F(TestAsyncioRunnable, YieldMultipleValues) +// { +// std::atomic counter = 0; +// pymrc::Pipeline p; + +// pybind11::module_::import("mrc.core.coro"); + +// auto init = [&counter](mrc::segment::IBuilder& seg) { +// auto src = seg.make_source("src", [](rxcpp::subscriber& s) { +// if (s.is_subscribed()) +// { +// s.on_next(5); +// s.on_next(10); +// } + +// s.on_completed(); +// }); + +// auto internal = seg.construct_object("internal"); + +// auto sink = seg.make_sink("sink", [&counter](unsigned int x) { +// counter.fetch_add(x, std::memory_order_relaxed); +// }); + +// seg.make_edge(src, internal); +// seg.make_edge(internal, sink); +// }; + +// p.make_segment("seg1"s, init); +// p.make_segment("seg2"s, init); + +// auto options = std::make_shared(); +// options->topology().user_cpuset("0"); +// // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. +// options->engine_factories().set_default_engine_type(runnable::EngineType::Thread); + +// pymrc::Executor exec{options}; +// exec.register_pipeline(p); + +// exec.start(); +// exec.join(); + +// EXPECT_EQ(counter, 30); +// } + +class PythonFlatmapAsyncioRunnable : public pymrc::AsyncioRunnable +{ + public: + PythonFlatmapAsyncioRunnable(pymrc::PyObjectHolder operation) : m_operation(std::move(operation)) {} + + mrc::coroutines::AsyncGenerator on_data(int&& value) override + { + py::gil_scoped_acquire acquire; + + auto coroutine = m_operation(py::cast(value)); + + pymrc::PyObjectHolder result; + { + py::gil_scoped_release release; + + result = co_await pymrc::coro::PyTaskToCppAwaitable(std::move(coroutine)); + } + + auto result_casted = py::cast(result); + + py::gil_scoped_release release; + + co_yield result_casted; + }; + + private: + pymrc::PyObjectHolder m_operation; +}; + +TEST_F(TestAsyncioRunnable, UseAsyncioTasks) { + py::object globals = py::globals(); + py::exec( + "\ +async def fn(value): \ + return value \ + ", + globals); + + pymrc::PyObjectHolder fn = static_cast(globals["fn"]); + + ASSERT_FALSE(fn.is_none()); + std::atomic counter = 0; pymrc::Pipeline p; pybind11::module_::import("mrc.core.coro"); - auto init = [&counter](mrc::segment::IBuilder& seg) { + auto init = [&counter, &fn](mrc::segment::IBuilder& seg) { auto src = seg.make_source("src", [](rxcpp::subscriber& s) { if (s.is_subscribed()) { s.on_next(5); s.on_next(10); - s.on_next(7); } s.on_completed(); }); - auto internal = seg.construct_object("internal"); + auto internal = seg.construct_object("internal", fn); auto sink = seg.make_sink("sink", [&counter](unsigned int x) { counter.fetch_add(x, std::memory_order_relaxed); @@ -102,5 +190,5 @@ TEST_F(TestAsyncioRunnable, Execute) exec.start(); exec.join(); - EXPECT_EQ(counter, 132); + EXPECT_EQ(counter, 30); } \ No newline at end of file From 2b2f26eb094dcad6c1dcaf934fcf028fb44f9e8b Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 18:29:11 +0000 Subject: [PATCH 33/50] fix test_asyncio_runnable test class to support multiple test cases --- .../_pymrc/tests/test_asyncio_runnable.cpp | 97 ++++++++----------- 1 file changed, 39 insertions(+), 58 deletions(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index 265876f32..5d15a9a3c 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -52,65 +52,46 @@ namespace pymrc = mrc::pymrc; using namespace std::string_literals; using namespace py::literals; -PYMRC_TEST_CLASS(AsyncioRunnable); - -class MyAsyncioRunnable : public pymrc::AsyncioRunnable +class TestWithPythonInterpreter : public ::testing::Test { - mrc::coroutines::AsyncGenerator on_data(int&& value) override - { - co_yield value; - }; -}; - -// TEST_F(TestAsyncioRunnable, YieldMultipleValues) -// { -// std::atomic counter = 0; -// pymrc::Pipeline p; - -// pybind11::module_::import("mrc.core.coro"); - -// auto init = [&counter](mrc::segment::IBuilder& seg) { -// auto src = seg.make_source("src", [](rxcpp::subscriber& s) { -// if (s.is_subscribed()) -// { -// s.on_next(5); -// s.on_next(10); -// } - -// s.on_completed(); -// }); - -// auto internal = seg.construct_object("internal"); + public: + virtual void interpreter_setup() = 0; -// auto sink = seg.make_sink("sink", [&counter](unsigned int x) { -// counter.fetch_add(x, std::memory_order_relaxed); -// }); + protected: + void SetUp() override; -// seg.make_edge(src, internal); -// seg.make_edge(internal, sink); -// }; + void TearDown() override; -// p.make_segment("seg1"s, init); -// p.make_segment("seg2"s, init); + private: + static bool m_initialized; +}; -// auto options = std::make_shared(); -// options->topology().user_cpuset("0"); -// // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. -// options->engine_factories().set_default_engine_type(runnable::EngineType::Thread); +bool TestWithPythonInterpreter::m_initialized; -// pymrc::Executor exec{options}; -// exec.register_pipeline(p); +void TestWithPythonInterpreter::SetUp() +{ + if (!m_initialized) + { + m_initialized = true; + pybind11::initialize_interpreter(); + interpreter_setup(); + } +} -// exec.start(); -// exec.join(); +void TestWithPythonInterpreter::TearDown() {} -// EXPECT_EQ(counter, 30); -// } +class __attribute__((visibility("default"))) TestAsyncioRunnable : public TestWithPythonInterpreter +{ + void interpreter_setup() override + { + pybind11::module_::import("mrc.core.coro"); + } +}; -class PythonFlatmapAsyncioRunnable : public pymrc::AsyncioRunnable +class PythonCallbackAsyncioRunnable : public pymrc::AsyncioRunnable { public: - PythonFlatmapAsyncioRunnable(pymrc::PyObjectHolder operation) : m_operation(std::move(operation)) {} + PythonCallbackAsyncioRunnable(pymrc::PyObjectHolder operation) : m_operation(std::move(operation)) {} mrc::coroutines::AsyncGenerator on_data(int&& value) override { @@ -140,10 +121,12 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasks) { py::object globals = py::globals(); py::exec( - "\ -async def fn(value): \ - return value \ - ", + R"( + async def fn(value): + import asyncio + await asyncio.sleep(0) + return value * 2 + )", globals); pymrc::PyObjectHolder fn = static_cast(globals["fn"]); @@ -153,8 +136,6 @@ async def fn(value): \ std::atomic counter = 0; pymrc::Pipeline p; - pybind11::module_::import("mrc.core.coro"); - auto init = [&counter, &fn](mrc::segment::IBuilder& seg) { auto src = seg.make_source("src", [](rxcpp::subscriber& s) { if (s.is_subscribed()) @@ -166,9 +147,9 @@ async def fn(value): \ s.on_completed(); }); - auto internal = seg.construct_object("internal", fn); + auto internal = seg.construct_object("internal", fn); - auto sink = seg.make_sink("sink", [&counter](unsigned int x) { + auto sink = seg.make_sink("sink", [&counter](int x) { counter.fetch_add(x, std::memory_order_relaxed); }); @@ -190,5 +171,5 @@ async def fn(value): \ exec.start(); exec.join(); - EXPECT_EQ(counter, 30); -} \ No newline at end of file + EXPECT_EQ(counter, 60); +} From df8feb862d91a078a51d72ca65a5da596f958b5f Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 19:39:54 +0000 Subject: [PATCH 34/50] refactor scheduler to be simpler --- cpp/mrc/CMakeLists.txt | 1 - cpp/mrc/include/mrc/coroutines/scheduler.hpp | 79 ++----------------- cpp/mrc/src/public/coroutines/scheduler.cpp | 71 ----------------- .../include/pymrc/asyncio_scheduler.hpp | 64 +++++++++------ 4 files changed, 45 insertions(+), 170 deletions(-) delete mode 100644 cpp/mrc/src/public/coroutines/scheduler.cpp diff --git a/cpp/mrc/CMakeLists.txt b/cpp/mrc/CMakeLists.txt index 46cb31787..f2f1e63cc 100644 --- a/cpp/mrc/CMakeLists.txt +++ b/cpp/mrc/CMakeLists.txt @@ -115,7 +115,6 @@ add_library(libmrc src/public/core/logging.cpp src/public/core/thread.cpp src/public/coroutines/event.cpp - src/public/coroutines/scheduler.cpp src/public/coroutines/sync_wait.cpp src/public/coroutines/task_container.cpp src/public/coroutines/thread_local_context.cpp diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index 0dc7660ea..f6960cb63 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -17,6 +17,8 @@ #pragma once +#include "mrc/coroutines/task.hpp" + #include #include #include @@ -27,87 +29,16 @@ namespace mrc::coroutines { /** * @brief Scheduler base class - * - * Allows all schedulers to be discovered via the mrc::this_thread::current_scheduler() */ class Scheduler : public std::enable_shared_from_this { public: - struct Operation - { - Operation(Scheduler& scheduler); - - constexpr static auto await_ready() noexcept -> bool - { - return false; - } - - std::coroutine_handle<> await_suspend(std::coroutine_handle<> awaiting_coroutine) noexcept; - - constexpr static auto await_resume() noexcept -> void {} - - Scheduler& m_scheduler; - std::coroutine_handle<> m_awaiting_coroutine; - Operation* m_next{nullptr}; - }; - - Scheduler(); virtual ~Scheduler() = default; - /** - * @brief Description of Scheduler - */ - virtual std::string description() const = 0; - - /** - * Schedules the currently executing coroutine to be run on this thread pool. This must be - * called from within the coroutines function body to schedule the coroutine on the thread pool. - * @throw std::runtime_error If the thread pool is `shutdown()` scheduling new tasks is not permitted. - * @return The operation to switch from the calling scheduling thread to the executor thread - * pool thread. - */ - [[nodiscard]] virtual auto schedule() -> Operation; - - /** - * Schedules any coroutine handle that is ready to be resumed. - * @param handle The coroutine handle to schedule. - */ - virtual auto resume(std::coroutine_handle<> coroutine) -> void = 0; - - /** - * Yields the current task to the end of the queue of waiting tasks. - */ - [[nodiscard]] auto yield() -> Operation; - - /** - * If the calling thread controlled by a Scheduler, return a pointer to the Scheduler - */ - static auto from_current_thread() noexcept -> Scheduler*; - - /** - * If the calling thread is owned by a thread_pool, return the thread index (rank) of the current thread with - * respect the threads in the pool; otherwise, return the std::hash of std::this_thread::get_id - */ - static auto get_thread_id() noexcept -> std::size_t; - - protected: - virtual auto on_thread_start(std::size_t) -> void; - - private: - /** - * @brief When co_await schedule() is called, this function will be executed by the awaiter. Each scheduler - * implementation should determine how and when to execute the operation. - * - * @param operation The schedule() awaitable pointer - * @return std::coroutine_handle<> Return a coroutine handle to which will be - * used as the return value for await_suspend(). - */ - virtual std::coroutine_handle<> schedule_operation(Operation* operation) = 0; - - mutable std::mutex m_mutex; + virtual void resume(std::coroutine_handle<> handle) noexcept = 0; - thread_local static Scheduler* m_thread_local_scheduler; - thread_local static std::size_t m_thread_id; + [[nodiscard]] virtual Task<> schedule() = 0; + [[nodiscard]] virtual Task<> yield() = 0; }; } // namespace mrc::coroutines diff --git a/cpp/mrc/src/public/coroutines/scheduler.cpp b/cpp/mrc/src/public/coroutines/scheduler.cpp deleted file mode 100644 index da3b7b35d..000000000 --- a/cpp/mrc/src/public/coroutines/scheduler.cpp +++ /dev/null @@ -1,71 +0,0 @@ -/** - * SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "mrc/coroutines/scheduler.hpp" - -#include - -#include -#include - -namespace mrc::coroutines { - -thread_local Scheduler* Scheduler::m_thread_local_scheduler{nullptr}; -thread_local std::size_t Scheduler::m_thread_id{0}; - -Scheduler::Operation::Operation(Scheduler& scheduler) : m_scheduler(scheduler) {} - -std::coroutine_handle<> Scheduler::Operation::await_suspend(std::coroutine_handle<> awaiting_coroutine) noexcept -{ - m_awaiting_coroutine = awaiting_coroutine; - return m_scheduler.schedule_operation(this); -} - -Scheduler::Scheduler() = default; - -auto Scheduler::schedule() -> Operation -{ - return Operation{*this}; -} - -auto Scheduler::yield() -> Operation -{ - return schedule(); -} - -auto Scheduler::from_current_thread() noexcept -> Scheduler* -{ - return m_thread_local_scheduler; -} - -auto Scheduler::get_thread_id() noexcept -> std::size_t -{ - if (m_thread_local_scheduler == nullptr) - { - return std::hash()(std::this_thread::get_id()); - } - return m_thread_id; -} - -auto Scheduler::on_thread_start(std::size_t thread_id) -> void -{ - DVLOG(10) << "scheduler: " << description() << " initializing"; - m_thread_id = thread_id; - m_thread_local_scheduler = this; -} - -} // namespace mrc::coroutines diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index b0e486c83..03e7f115f 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -19,6 +19,7 @@ #include "pymrc/coro.hpp" #include "pymrc/utilities/acquire_gil.hpp" +#include "pymrc/utilities/object_wrappers.hpp" #include #include @@ -28,52 +29,67 @@ #include #include +#include namespace py = pybind11; namespace mrc::pymrc { + +/** + * @brief A MRC Scheduler which allows resuming C++20 coroutines on an Asyncio event loop. + */ class AsyncioScheduler : public mrc::coroutines::Scheduler { - public: - AsyncioScheduler(PyObjectHolder loop) : m_loop(std::move(loop)) {} - - std::string description() const override + private: + class ContinueOnLoopOperation { - return "AsyncioScheduler"; - } + public: + ContinueOnLoopOperation(PyObjectHolder loop) : m_loop(std::move(loop)) {} - void resume(std::coroutine_handle<> coroutine) override - { - if (coroutine.done()) + static bool await_ready() noexcept { - LOG(WARNING) << "AsyncioScheduler::resume() > Attempted to resume a completed coroutine"; - return; + return false; } - py::gil_scoped_acquire gil; + void await_suspend(std::coroutine_handle<> handle) noexcept + { + AsyncioScheduler::resume(m_loop, handle); + } - // TODO(MDD): Check whether or not we need thread safe version - m_loop.attr("call_soon_threadsafe")(py::cpp_function([this, handle = std::move(coroutine)]() { - if (handle.done()) - { - LOG(WARNING) << "AsyncioScheduler::resume() > Attempted to resume a completed coroutine"; - return; - } + static void await_resume() noexcept {} - py::gil_scoped_release nogil; + private: + PyObjectHolder m_loop; + }; + static void resume(PyObjectHolder loop, std::coroutine_handle<> handle) noexcept + { + pybind11::gil_scoped_acquire acquire; + loop.attr("call_soon_threadsafe")(pybind11::cpp_function([handle]() { // + pybind11::gil_scoped_release release; handle.resume(); })); } - private: - std::coroutine_handle<> schedule_operation(Operation* operation) override + public: + AsyncioScheduler(PyObjectHolder loop) : m_loop(std::move(loop)) {} + + void resume(std::coroutine_handle<> handle) noexcept override + { + AsyncioScheduler::resume(m_loop, handle); + } + + [[nodiscard]] coroutines::Task<> schedule() override { - this->resume(std::move(operation->m_awaiting_coroutine)); + co_await ContinueOnLoopOperation(m_loop); + } - return std::noop_coroutine(); + [[nodiscard]] coroutines::Task<> yield() override + { + co_await ContinueOnLoopOperation(m_loop); } + private: mrc::pymrc::PyHolder m_loop; }; From ce77e060b662c46925b8609f917dc77e4fd0198d Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 21:23:22 +0000 Subject: [PATCH 35/50] fix includes --- python/mrc/_pymrc/tests/test_asyncio_runnable.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index 5d15a9a3c..e9c52a0b5 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -15,12 +15,10 @@ * limitations under the License. */ -#include "test_pymrc.hpp" - #include "pymrc/asyncio_runnable.hpp" +#include "pymrc/coro.hpp" #include "pymrc/executor.hpp" #include "pymrc/pipeline.hpp" -#include "pymrc/types.hpp" #include "pymrc/utilities/object_wrappers.hpp" #include "mrc/coroutines/async_generator.hpp" @@ -35,6 +33,7 @@ #include #include +#include #include #include #include @@ -46,6 +45,7 @@ #include #include #include +#include namespace py = pybind11; namespace pymrc = mrc::pymrc; From e7331067350a1bc029719bfeae39ddec202dae62 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 1 Nov 2023 22:48:49 +0000 Subject: [PATCH 36/50] add boost future awaitable operation test --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 2 +- .../_pymrc/tests/test_asyncio_runnable.cpp | 59 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index f9b481a7d..a7bb4b266 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -91,7 +91,7 @@ class BoostFutureAwaitableOperation std::move(continuation)); } - auto await_resume() noexcept + auto await_resume() { return m_future.get(); } diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index e9c52a0b5..d28fcf7ba 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -22,6 +22,8 @@ #include "pymrc/utilities/object_wrappers.hpp" #include "mrc/coroutines/async_generator.hpp" +#include "mrc/coroutines/sync_wait.hpp" +#include "mrc/coroutines/task.hpp" #include "mrc/node/rx_sink.hpp" #include "mrc/node/rx_source.hpp" #include "mrc/options/engine_groups.hpp" @@ -31,6 +33,7 @@ #include "mrc/segment/builder.hpp" #include "mrc/segment/object.hpp" +#include #include #include #include @@ -44,6 +47,8 @@ #include #include #include +#include +#include #include #include @@ -163,7 +168,7 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasks) auto options = std::make_shared(); options->topology().user_cpuset("0"); // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. - options->engine_factories().set_default_engine_type(runnable::EngineType::Thread); + options->engine_factories().set_default_engine_type(mrc::runnable::EngineType::Thread); pymrc::Executor exec{options}; exec.register_pipeline(p); @@ -173,3 +178,55 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasks) EXPECT_EQ(counter, 60); } + +template +auto run_operation(OperationT& operation) -> mrc::coroutines::Task +{ + auto stop_source = std::stop_source(); + + auto coro = [](auto& operation, auto stop_source) -> mrc::coroutines::Task { + try + { + auto value = co_await operation(); + stop_source.request_stop(); + co_return value; + } catch (...) + { + stop_source.request_stop(); + throw; + } + }(operation, stop_source); + + coro.resume(); + + while (not stop_source.stop_requested()) + { + if (boost::fibers::has_ready_fibers()) + { + boost::this_fiber::yield(); + } + } + + co_return co_await coro; +} + +TEST_F(TestAsyncioRunnable, BoostFutureAwaitableOperationCanReturn) +{ + auto operation = mrc::pymrc::BoostFutureAwaitableOperation([]() { + using namespace std::chrono_literals; + boost::this_fiber::sleep_for(10ms); + return 5; + }); + + ASSERT_EQ(mrc::coroutines::sync_wait(run_operation(operation)), 5); +} + +TEST_F(TestAsyncioRunnable, BoostFutureAwaitableOperationCanThrow) +{ + auto operation = mrc::pymrc::BoostFutureAwaitableOperation([]() { + throw std::runtime_error("oops"); + return 5; + }); + + ASSERT_THROW(mrc::coroutines::sync_wait(run_operation(operation)), std::runtime_error); +} From ce7d371bfa168708b97267e863cb0e5aeee3203d Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 15:59:41 +0000 Subject: [PATCH 37/50] fix tests --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 1 + .../_pymrc/tests/test_asyncio_runnable.cpp | 41 +++---------------- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index a7bb4b266..4c0f0d89d 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -231,6 +231,7 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) LOG(INFO) << "AsyncioRunnable::run() > Calling run_until_complete() on main_task()"; loop.attr("run_until_complete")(std::move(py_awaitable)); + loop.attr("close")(); } // Need to drop the output edges diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index d28fcf7ba..c1921bb46 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ +#include "test_pymrc.hpp" #include "pymrc/asyncio_runnable.hpp" #include "pymrc/coro.hpp" #include "pymrc/executor.hpp" @@ -37,6 +38,8 @@ #include #include #include +#include +#include #include #include #include @@ -57,41 +60,7 @@ namespace pymrc = mrc::pymrc; using namespace std::string_literals; using namespace py::literals; -class TestWithPythonInterpreter : public ::testing::Test -{ - public: - virtual void interpreter_setup() = 0; - - protected: - void SetUp() override; - - void TearDown() override; - - private: - static bool m_initialized; -}; - -bool TestWithPythonInterpreter::m_initialized; - -void TestWithPythonInterpreter::SetUp() -{ - if (!m_initialized) - { - m_initialized = true; - pybind11::initialize_interpreter(); - interpreter_setup(); - } -} - -void TestWithPythonInterpreter::TearDown() {} - -class __attribute__((visibility("default"))) TestAsyncioRunnable : public TestWithPythonInterpreter -{ - void interpreter_setup() override - { - pybind11::module_::import("mrc.core.coro"); - } -}; +PYMRC_TEST_CLASS(AsyncioRunnable); class PythonCallbackAsyncioRunnable : public pymrc::AsyncioRunnable { @@ -124,6 +93,8 @@ class PythonCallbackAsyncioRunnable : public pymrc::AsyncioRunnable TEST_F(TestAsyncioRunnable, UseAsyncioTasks) { + pybind11::module_::import("mrc.core.coro"); + py::object globals = py::globals(); py::exec( R"( From 98e093add6d4e17b4b1febf0859d343193423cf0 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 16:03:31 +0000 Subject: [PATCH 38/50] add comments to scheduler and asyncio_scheduler --- cpp/mrc/include/mrc/coroutines/scheduler.hpp | 10 ++++++++++ python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index f6960cb63..ea6e9fdbd 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -35,9 +35,19 @@ class Scheduler : public std::enable_shared_from_this public: virtual ~Scheduler() = default; + /** + * @brief Resumes a coroutine according to the scheduler's implementation. + */ virtual void resume(std::coroutine_handle<> handle) noexcept = 0; + /** + * @brief Suspends the current function and resumes it according to the scheduler's implementation. + */ [[nodiscard]] virtual Task<> schedule() = 0; + + /** + * @brief Suspends the current function and resumes it according to the scheduler's implementation. + */ [[nodiscard]] virtual Task<> yield() = 0; }; diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index 03e7f115f..0875b5dc4 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -74,16 +74,26 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler public: AsyncioScheduler(PyObjectHolder loop) : m_loop(std::move(loop)) {} + + /** + * @brief Resumes a coroutine on the scheduler's Asyncio event loop + */ void resume(std::coroutine_handle<> handle) noexcept override { AsyncioScheduler::resume(m_loop, handle); } + /** + * @brief Suspends the current function and resumes it on the scheduler's Asyncio event loop + */ [[nodiscard]] coroutines::Task<> schedule() override { co_await ContinueOnLoopOperation(m_loop); } + /** + * @brief Suspends the current function and resumes it on the scheduler's Asyncio event loop + */ [[nodiscard]] coroutines::Task<> yield() override { co_await ContinueOnLoopOperation(m_loop); From e6509970a007d920e9667d80d70538ef0189f601 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 16:14:51 +0000 Subject: [PATCH 39/50] comment asyncio_runnable's functions --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index 4c0f0d89d..e8453e3fc 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -122,6 +122,9 @@ class AsyncSink : public mrc::node::WritableProvider, this->set_channel(std::make_unique>()); } + /** + * @brief Asynchronously reads a value from the sink's channel + */ coroutines::Task read_async(T& value) { co_return co_await m_read_async(std::ref(value)); @@ -149,6 +152,9 @@ class AsyncSource : public mrc::node::WritableAcceptor, this->set_channel(std::make_unique>()); } + /** + * @brief Asynchronously writes a value to the source's channel + */ coroutines::Task write_async(T&& value) { co_return co_await m_write_async(std::move(value)); @@ -174,16 +180,33 @@ class AsyncioRunnable : public AsyncSink, ~AsyncioRunnable() override = default; private: + /** + * @brief Runnable's entrypoint. + */ void run(mrc::runnable::Context& ctx) override; + + /** + * @brief Runnable's state control, for stopping from MRC. + */ void on_state_update(const state_t& state) final; + /** + * @brief The top-level coroutine which is run while the asyncio event loop is running. + */ coroutines::Task<> main_task(std::shared_ptr scheduler); + /** + * @brief The per-value coroutine run asynchronously alongside other calls. + */ coroutines::Task<> process_one(InputT value, task_buffer_t& task_buffer, std::shared_ptr on, ExceptionCatcher& catcher); + /** + * @brief Value's read from the sink's channel are fed to this function and yields from the + * resulting generator are written to the source's channel. + */ virtual mrc::coroutines::AsyncGenerator on_data(InputT&& value) = 0; std::stop_source m_stop_source; From 4b1999d46357da43ad31a4431a6890fd2c84d5ea Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 16:55:26 +0000 Subject: [PATCH 40/50] add exception test case for asyncio_runnable --- .../_pymrc/include/pymrc/asyncio_runnable.hpp | 17 +++- .../_pymrc/tests/test_asyncio_runnable.cpp | 85 +++++++++++++++++-- 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp index e8453e3fc..965cf551c 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp @@ -217,6 +217,8 @@ class AsyncioRunnable : public AsyncSink, template void AsyncioRunnable::run(mrc::runnable::Context& ctx) { + std::exception_ptr exception; + { py::gil_scoped_acquire gil; @@ -253,13 +255,25 @@ void AsyncioRunnable::run(mrc::runnable::Context& ctx) LOG(INFO) << "AsyncioRunnable::run() > Calling run_until_complete() on main_task()"; - loop.attr("run_until_complete")(std::move(py_awaitable)); + try + { + loop.attr("run_until_complete")(std::move(py_awaitable)); + } catch (...) + { + exception = std::current_exception(); + } + loop.attr("close")(); } // Need to drop the output edges mrc::node::SourceProperties::release_edge_connection(); mrc::node::SinkProperties::release_edge_connection(); + + if (exception != nullptr) + { + std::rethrow_exception(exception); + } } template @@ -345,7 +359,6 @@ void AsyncioRunnable::on_state_update(const state_t& state) break; case state_t::Kill: - m_stop_source.request_stop(); break; diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index c1921bb46..9b3d17d21 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -16,6 +16,7 @@ */ #include "test_pymrc.hpp" + #include "pymrc/asyncio_runnable.hpp" #include "pymrc/coro.hpp" #include "pymrc/executor.hpp" @@ -38,8 +39,6 @@ #include #include #include -#include -#include #include #include #include @@ -48,7 +47,9 @@ #include #include +#include #include +#include #include #include #include @@ -60,7 +61,26 @@ namespace pymrc = mrc::pymrc; using namespace std::string_literals; using namespace py::literals; -PYMRC_TEST_CLASS(AsyncioRunnable); +class __attribute__((visibility("default"))) TestAsyncioRunnable : public ::testing::Test +{ + public: + static void SetUpTestSuite() + { + m_interpreter = std::make_unique(); + pybind11::gil_scoped_acquire acquire; + pybind11::module_::import("mrc.core.coro"); + } + + static void TearDownTestSuite() + { + m_interpreter.reset(); + } + + private: + static std::unique_ptr m_interpreter; +}; + +std::unique_ptr TestAsyncioRunnable::m_interpreter; class PythonCallbackAsyncioRunnable : public pymrc::AsyncioRunnable { @@ -93,8 +113,6 @@ class PythonCallbackAsyncioRunnable : public pymrc::AsyncioRunnable TEST_F(TestAsyncioRunnable, UseAsyncioTasks) { - pybind11::module_::import("mrc.core.coro"); - py::object globals = py::globals(); py::exec( R"( @@ -150,6 +168,63 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasks) EXPECT_EQ(counter, 60); } +TEST_F(TestAsyncioRunnable, UseAsyncioTasksThrows) +{ + // pybind11::module_::import("mrc.core.coro"); + + py::object globals = py::globals(); + py::exec( + R"( + async def fn(value): + raise RuntimeError("oops") + )", + globals); + + pymrc::PyObjectHolder fn = static_cast(globals["fn"]); + + ASSERT_FALSE(fn.is_none()); + + std::atomic counter = 0; + pymrc::Pipeline p; + + auto init = [&counter, &fn](mrc::segment::IBuilder& seg) { + auto src = seg.make_source("src", [](rxcpp::subscriber& s) { + if (s.is_subscribed()) + { + s.on_next(5); + s.on_next(10); + } + + s.on_completed(); + }); + + auto internal = seg.construct_object("internal", fn); + + auto sink = seg.make_sink("sink", [&counter](int x) { + counter.fetch_add(x, std::memory_order_relaxed); + }); + + seg.make_edge(src, internal); + seg.make_edge(internal, sink); + }; + + p.make_segment("seg1"s, init); + p.make_segment("seg2"s, init); + + auto options = std::make_shared(); + options->topology().user_cpuset("0"); + // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. + options->engine_factories().set_default_engine_type(mrc::runnable::EngineType::Thread); + + pymrc::Executor exec{options}; + exec.register_pipeline(p); + + exec.start(); + exec.join(); + + EXPECT_EQ(counter, 60); +} + template auto run_operation(OperationT& operation) -> mrc::coroutines::Task { From 1591e3d82e21b9484c9da702356ae9d0c33bc449 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 17:30:10 +0000 Subject: [PATCH 41/50] add asyncgenerator failure test to asyncio_runnable --- python/mrc/_pymrc/include/pymrc/coro.hpp | 15 ++++- .../_pymrc/tests/test_asyncio_runnable.cpp | 59 ++++++++++++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index 5c80398cc..fa9b865da 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -174,13 +174,22 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable PyTaskToCppAwaitable(mrc::pymrc::PyObjectHolder&& task) : m_task(std::move(task)) { pybind11::gil_scoped_acquire acquire; - if (pybind11::module_::import("inspect").attr("iscoroutine")(m_task).cast()) + + auto asyncio = pybind11::module_::import("asyncio"); + auto inspect = pybind11::module_::import("inspect"); + + if (not asyncio.attr("isfuture")(m_task).cast()) { - m_task = pybind11::module_::import("asyncio").attr("create_task")(m_task); + if (not asyncio.attr("iscoroutine")(m_task).cast()) + { + throw std::runtime_error(MRC_CONCAT_STR("PyTaskToCppAwaitable expected task or coroutine but got " << pybind11::repr(m_task).cast())); + } + + m_task = asyncio.attr("create_task")(m_task); } } - static bool await_ready() noexcept // NOLINT(readability-convert-member-functions-to-static) + static bool await_ready() noexcept { // Always suspend return false; diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index 9b3d17d21..e71cc5243 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -168,6 +168,62 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasks) EXPECT_EQ(counter, 60); } +TEST_F(TestAsyncioRunnable, UseAsyncioGeneratorThrows) +{ + // pybind11::module_::import("mrc.core.coro"); + + py::object globals = py::globals(); + py::exec( + R"( + async def fn(value): + yield value + )", + globals); + + pymrc::PyObjectHolder fn = static_cast(globals["fn"]); + + ASSERT_FALSE(fn.is_none()); + + std::atomic counter = 0; + pymrc::Pipeline p; + + auto init = [&counter, &fn](mrc::segment::IBuilder& seg) { + auto src = seg.make_source("src", [](rxcpp::subscriber& s) { + if (s.is_subscribed()) + { + s.on_next(5); + s.on_next(10); + } + + s.on_completed(); + }); + + auto internal = seg.construct_object("internal", fn); + + auto sink = seg.make_sink("sink", [&counter](int x) { + counter.fetch_add(x, std::memory_order_relaxed); + }); + + seg.make_edge(src, internal); + seg.make_edge(internal, sink); + }; + + p.make_segment("seg1"s, init); + p.make_segment("seg2"s, init); + + auto options = std::make_shared(); + options->topology().user_cpuset("0"); + // AsyncioRunnable only works with the Thread engine due to asyncio loops being thread-specific. + options->engine_factories().set_default_engine_type(mrc::runnable::EngineType::Thread); + + pymrc::Executor exec{options}; + exec.register_pipeline(p); + + exec.start(); + + ASSERT_THROW(exec.join(), std::runtime_error); +} + TEST_F(TestAsyncioRunnable, UseAsyncioTasksThrows) { // pybind11::module_::import("mrc.core.coro"); @@ -220,9 +276,8 @@ TEST_F(TestAsyncioRunnable, UseAsyncioTasksThrows) exec.register_pipeline(p); exec.start(); - exec.join(); - EXPECT_EQ(counter, 60); + ASSERT_THROW(exec.join(), std::runtime_error); } template From c2942c272db4962795e0a14f2cde66220c1fbc34 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 17:30:52 +0000 Subject: [PATCH 42/50] formatting --- python/mrc/_pymrc/include/pymrc/coro.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index fa9b865da..bc380a5e6 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -182,7 +182,8 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable { if (not asyncio.attr("iscoroutine")(m_task).cast()) { - throw std::runtime_error(MRC_CONCAT_STR("PyTaskToCppAwaitable expected task or coroutine but got " << pybind11::repr(m_task).cast())); + throw std::runtime_error(MRC_CONCAT_STR("PyTaskToCppAwaitable expected task or coroutine but got " + << pybind11::repr(m_task).cast())); } m_task = asyncio.attr("create_task")(m_task); From 00253e600afbaa0319341bbfc3563acc5921f6f1 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 17:31:07 +0000 Subject: [PATCH 43/50] remove inspect --- python/mrc/_pymrc/include/pymrc/coro.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/python/mrc/_pymrc/include/pymrc/coro.hpp b/python/mrc/_pymrc/include/pymrc/coro.hpp index bc380a5e6..5b50f14a0 100644 --- a/python/mrc/_pymrc/include/pymrc/coro.hpp +++ b/python/mrc/_pymrc/include/pymrc/coro.hpp @@ -176,7 +176,6 @@ class PYBIND11_EXPORT PyTaskToCppAwaitable pybind11::gil_scoped_acquire acquire; auto asyncio = pybind11::module_::import("asyncio"); - auto inspect = pybind11::module_::import("inspect"); if (not asyncio.attr("isfuture")(m_task).cast()) { From a4e49f2b483974b5f224ae3989d75745161a603e Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 18:04:03 +0000 Subject: [PATCH 44/50] fix includes --- python/mrc/_pymrc/tests/test_asyncio_runnable.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp index e71cc5243..46a139a04 100644 --- a/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp +++ b/python/mrc/_pymrc/tests/test_asyncio_runnable.cpp @@ -15,8 +15,6 @@ * limitations under the License. */ -#include "test_pymrc.hpp" - #include "pymrc/asyncio_runnable.hpp" #include "pymrc/coro.hpp" #include "pymrc/executor.hpp" From 7fa1f794bb613c1ce63b9a771f3750f10e05e623 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 18:16:37 +0000 Subject: [PATCH 45/50] fix scheduler comment styles --- cpp/mrc/include/mrc/coroutines/scheduler.hpp | 4 ++-- python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index ea6e9fdbd..a7a58a4a1 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -42,12 +42,12 @@ class Scheduler : public std::enable_shared_from_this /** * @brief Suspends the current function and resumes it according to the scheduler's implementation. - */ + */ [[nodiscard]] virtual Task<> schedule() = 0; /** * @brief Suspends the current function and resumes it according to the scheduler's implementation. - */ + */ [[nodiscard]] virtual Task<> yield() = 0; }; diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index 0875b5dc4..45cf08103 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -85,7 +85,7 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler /** * @brief Suspends the current function and resumes it on the scheduler's Asyncio event loop - */ + */ [[nodiscard]] coroutines::Task<> schedule() override { co_await ContinueOnLoopOperation(m_loop); @@ -93,7 +93,7 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler /** * @brief Suspends the current function and resumes it on the scheduler's Asyncio event loop - */ + */ [[nodiscard]] coroutines::Task<> yield() override { co_await ContinueOnLoopOperation(m_loop); From 519d0e799729fbdf21a37648e6cd5180b5a10159 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 18:17:28 +0000 Subject: [PATCH 46/50] fix asyncio_scheduler styles --- python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp index 45cf08103..3d9e563b9 100644 --- a/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp +++ b/python/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp @@ -65,7 +65,7 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler static void resume(PyObjectHolder loop, std::coroutine_handle<> handle) noexcept { pybind11::gil_scoped_acquire acquire; - loop.attr("call_soon_threadsafe")(pybind11::cpp_function([handle]() { // + loop.attr("call_soon_threadsafe")(pybind11::cpp_function([handle]() { pybind11::gil_scoped_release release; handle.resume(); })); @@ -74,7 +74,6 @@ class AsyncioScheduler : public mrc::coroutines::Scheduler public: AsyncioScheduler(PyObjectHolder loop) : m_loop(std::move(loop)) {} - /** * @brief Resumes a coroutine on the scheduler's Asyncio event loop */ From 229dd3ad5f3b605beeb4ea5e94dd0866a372dd93 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 20:12:25 +0000 Subject: [PATCH 47/50] fix styles --- cpp/mrc/include/mrc/coroutines/scheduler.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/mrc/include/mrc/coroutines/scheduler.hpp b/cpp/mrc/include/mrc/coroutines/scheduler.hpp index a7a58a4a1..0e296924a 100644 --- a/cpp/mrc/include/mrc/coroutines/scheduler.hpp +++ b/cpp/mrc/include/mrc/coroutines/scheduler.hpp @@ -48,7 +48,7 @@ class Scheduler : public std::enable_shared_from_this /** * @brief Suspends the current function and resumes it according to the scheduler's implementation. */ - [[nodiscard]] virtual Task<> yield() = 0; + [[nodiscard]] virtual Task<> yield() = 0; }; } // namespace mrc::coroutines From 5dbfbe019339103bea4626b1749176a774cd54dd Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 20:53:45 +0000 Subject: [PATCH 48/50] add copyright header to exception_catcher.hpp --- .../mrc/exceptions/exception_catcher.hpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp index cf1c48f75..bb0d6ec3a 100644 --- a/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp +++ b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp @@ -1,7 +1,26 @@ +/** + * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #include #include #include +#pragma once + namespace mrc { /** From 6c070c85a0b51f95a71284950eab819c6a89f934 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 21:05:36 +0000 Subject: [PATCH 49/50] styles --- cpp/mrc/include/mrc/exceptions/exception_catcher.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp index bb0d6ec3a..98c4a7d6d 100644 --- a/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp +++ b/cpp/mrc/include/mrc/exceptions/exception_catcher.hpp @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - + #include #include #include From f2369db83c43f5bccf589ccb3b58d98edb58ca10 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 2 Nov 2023 21:16:08 +0000 Subject: [PATCH 50/50] fix exception_catch.cpp copyright header --- .../src/public/exceptions/exception_catcher.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cpp/mrc/src/public/exceptions/exception_catcher.cpp b/cpp/mrc/src/public/exceptions/exception_catcher.cpp index 49769436d..c139436f7 100644 --- a/cpp/mrc/src/public/exceptions/exception_catcher.cpp +++ b/cpp/mrc/src/public/exceptions/exception_catcher.cpp @@ -1,3 +1,20 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #include namespace mrc {