Skip to content

Commit

Permalink
[shortfin] Fix issues with free threaded Python builds. (#226)
Browse files Browse the repository at this point in the history
* 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
iree-org/iree#18609, which was causing an assert
to be hit in this specific RESOURCE_EXHAUSTED scenario.
  • Loading branch information
stellaraccident authored Sep 26, 2024
1 parent a9fecda commit fe5af6b
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 4 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci_linux_x64_nogil-libshortfin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
20 changes: 20 additions & 0 deletions libshortfin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"))'
```
1 change: 1 addition & 0 deletions libshortfin/build_tools/python_lsan_suppressions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ leak:google/_upb
leak:import_find_and_load
leak:pyo3::pyclass::create_type_object
leak:ufunc
leak:pydantic_core
7 changes: 5 additions & 2 deletions libshortfin/python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions libshortfin/python/_shortfin/asyncio_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions libshortfin/tests/examples/fastapi_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit fe5af6b

Please sign in to comment.