From fe5af6b06b4d15b3ffb34034eb80162508876f0f Mon Sep 17 00:00:00 2001 From: Stella Laurenzo Date: Thu, 26 Sep 2024 14:07:03 -0700 Subject: [PATCH] [shortfin] Fix issues with free threaded Python builds. (#226) * Fixes a method signature issue that manifested on 3.13 generally (not FT related). * Bumps nanobind to a recent HEAD commit to pick up free threaded support. * Adds nanobind option to enable free threading if building for a CPython with it enabled. * Adds a README stanza advising on how to acquire a free threaded CPython. * Makes fastapi tests skip if deps not met (not yet available / hard to install). * Adds an LSAN exclusion for something unrelated that has snuck in. * Sets FT CMAKE_BUILD_TYPE=Debug in the CI. Note that on the large NUMA system I was testing on, the CPU test which creates an executor on each NUMA node for all processes exceeded the default file handle ulimit, requiring it to be increased. I assume that the FT CPython internally uses more synchronization handles and it just happened to go over budget. This resulted in fixing https://github.com/iree-org/iree/pull/18609, which was causing an assert to be hit in this specific RESOURCE_EXHAUSTED scenario. --- .../ci_linux_x64_nogil-libshortfin.yml | 1 + libshortfin/README.md | 20 +++++++++++++++++++ .../build_tools/python_lsan_suppressions.txt | 1 + libshortfin/python/CMakeLists.txt | 7 +++++-- .../python/_shortfin/asyncio_bridge.py | 4 ++-- libshortfin/tests/examples/fastapi_test.py | 4 ++++ 6 files changed, 33 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci_linux_x64_nogil-libshortfin.yml b/.github/workflows/ci_linux_x64_nogil-libshortfin.yml index 8990edc49..cc3f49707 100644 --- a/.github/workflows/ci_linux_x64_nogil-libshortfin.yml +++ b/.github/workflows/ci_linux_x64_nogil-libshortfin.yml @@ -79,6 +79,7 @@ jobs: mkdir ${{ env.LIBSHORTFIN_DIR }}/build cd ${{ env.LIBSHORTFIN_DIR }}/build cmake -GNinja \ + -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_C_COMPILER=clang-18 \ -DCMAKE_CXX_COMPILER=clang++-18 \ -DCMAKE_LINKER_TYPE=LLD \ diff --git a/libshortfin/README.md b/libshortfin/README.md index 1cb06cfa8..c61a7e38e 100644 --- a/libshortfin/README.md +++ b/libshortfin/README.md @@ -101,3 +101,23 @@ recommended: * Compile dependencies with `-fvisibility=hidden` * Enable LTO builds of libshortfin * Set flags to enable symbol versioning + +# Miscellaneous Build Topics + +## Free threaded Python + +Support for free threaded Python builds (aka. "nogil") is in progress. It +is currently being tested via dev builds of CPython 3.13 with the +`--disable-gil` option set. There are multiple ways to acquire such an +environment. If using `pyenv`, here is a way: + +``` +# Build a 3.13-dev-nogil version. +PYTHON_CONFIGURE_OPTS='--disable-gil' \ +$(pyenv root)/plugins/python-build/bin/python-build 3.13-dev \ + $(pyenv root)/versions/3.13-dev-nogil + +# Test (should print "1"). +pyenv shell 3.13-dev-nogil +python -c 'import sysconfig; print(sysconfig.get_config_var("Py_GIL_DISABLED"))' +``` diff --git a/libshortfin/build_tools/python_lsan_suppressions.txt b/libshortfin/build_tools/python_lsan_suppressions.txt index ebf3039d2..498f1ea72 100644 --- a/libshortfin/build_tools/python_lsan_suppressions.txt +++ b/libshortfin/build_tools/python_lsan_suppressions.txt @@ -7,3 +7,4 @@ leak:google/_upb leak:import_find_and_load leak:pyo3::pyclass::create_type_object leak:ufunc +leak:pydantic_core diff --git a/libshortfin/python/CMakeLists.txt b/libshortfin/python/CMakeLists.txt index 7ca96a583..9104fcbc2 100644 --- a/libshortfin/python/CMakeLists.txt +++ b/libshortfin/python/CMakeLists.txt @@ -11,14 +11,17 @@ # Others. # nanobind +# Pinned to a pre 2.2.0 commit hash which includes free threaded support. +# TODO: Bump to 2.2.0 when available. FetchContent_Declare( nanobind GIT_REPOSITORY https://github.com/wjakob/nanobind.git - GIT_TAG 9641bb7151f04120013b812789b3ebdfa7e7324f # 2.1.0 + GIT_TAG 8ce0dee7f62add575f85c0de386a9c819e4d50af # HEAD > 2.1.0 ) FetchContent_MakeAvailable(nanobind) -nanobind_add_module(shortfin_python_extension NB_STATIC LTO +nanobind_add_module(shortfin_python_extension + NB_STATIC LTO FREE_THREADED array_binding.cc array_host_ops.cc lib_ext.cc diff --git a/libshortfin/python/_shortfin/asyncio_bridge.py b/libshortfin/python/_shortfin/asyncio_bridge.py index 07c83c779..4cb54449c 100644 --- a/libshortfin/python/_shortfin/asyncio_bridge.py +++ b/libshortfin/python/_shortfin/asyncio_bridge.py @@ -17,8 +17,8 @@ def get_debug(self): # Requirement of asyncio. return False - def create_task(self, coro): - return asyncio.Task(coro, loop=self) + def create_task(self, coro, *, name=None, context=None): + return asyncio.Task(coro, loop=self, name=name, context=context) def create_future(self): return asyncio.Future(loop=self) diff --git a/libshortfin/tests/examples/fastapi_test.py b/libshortfin/tests/examples/fastapi_test.py index 5640f0d4b..bd9c350ee 100644 --- a/libshortfin/tests/examples/fastapi_test.py +++ b/libshortfin/tests/examples/fastapi_test.py @@ -20,6 +20,10 @@ @pytest.fixture(scope="session") def server(): + try: + import fastapi + except ModuleNotFoundError as e: + pytest.skip(f"Required dep not available: {e}") runner = ServerRunner([]) yield runner print("Sending kill signal")