Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: free-threaded Python (3.13.0+) support #165

Merged
merged 24 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f335468
Rebase to master
jmao-denver Sep 23, 2024
a1fb456
More thread-safety change/refactoring
jmao-denver Sep 23, 2024
e93774a
Fix a race between PO cleanup thread and Python
jmao-denver Sep 27, 2024
e65f8ff
Add free-threaded build in GH action
jmao-denver Oct 21, 2024
6a0f8fe
Fix dll names for FT/debug builds
jmao-denver Oct 24, 2024
2cb3314
Cleanup and more comments
jmao-denver Oct 24, 2024
fe97888
Rollback temp change and add clarifying comments
jmao-denver Oct 24, 2024
ab38dcb
Complete release action for FT builds
jmao-denver Oct 24, 2024
16ba89a
Merge ft builds into build.yml
jmao-denver Oct 24, 2024
40ef816
Use GITHUB_PATH
jmao-denver Oct 29, 2024
a5718c1
Squash windows-t build, add Java ver in matrix
jmao-denver Oct 29, 2024
9aa8250
Fix for invalid syntax for powershell
jmao-denver Oct 29, 2024
752a0a7
Add upload action for JVM core/log
jmao-denver Oct 29, 2024
e1f841b
debug build.yml
jmao-denver Oct 29, 2024
0165921
Apply suggestions from code review
jmao-denver Oct 29, 2024
31b9010
Restore a block of code deleted by mistake
jmao-denver Oct 29, 2024
ccc3253
debug Windows FT build
jmao-denver Oct 29, 2024
d972d96
Add comments, use version directives
jmao-denver Oct 30, 2024
4058638
Code cleanup
jmao-denver Oct 31, 2024
173592d
Fix a bug in convert() for java array type
jmao-denver Nov 2, 2024
eea87e7
Remove duplicate comment
jmao-denver Nov 4, 2024
c9d7bfa
Add clarifying comment
jmao-denver Nov 4, 2024
1c51917
Respond to review comments
jmao-denver Nov 6, 2024
99f130a
Naming change to be more applicable in FT mode
jmao-denver Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 97 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ jobs:
distribution: 'temurin'
java-version: '8'

- run: pip install setuptools
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
- run: ${{ matrix.info.cmd }}

- uses: actions/upload-artifact@v4
Expand Down Expand Up @@ -161,9 +160,105 @@ jobs:
path: /tmp/dist/*.whl
retention-days: 1

bdist-wheels-t:
runs-on: ${{ matrix.info.machine }}
strategy:
fail-fast: false
matrix:
info:
- { machine: 'ubuntu-20.04', python: '3.13t', arch: 'amd64', cmd: '.github/env/Linux/bdist-wheel.sh' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that the latest ubuntu LTS is 24.04.1. Not sure how much it matters here.

- { machine: 'macos-13', python: '3.13t', arch: 'amd64', cmd: '.github/env/macOS/bdist-wheel.sh' }
- { machine: 'macos-latest', python: '3.13t', arch: 'arm64', cmd: '.github/env/macOS/bdist-wheel.sh' }
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved

steps:
- uses: actions/checkout@v4

- uses: astral-sh/setup-uv@v3
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
- run: |
uv python install ${{ matrix.info.python }}
uv venv --python ${{ matrix.info.python }}
source .venv/bin/activate
uv pip install pip
echo $JAVA_HOME
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
echo PATH=$PATH >> $GITHUB_ENV
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved

- run: ${{ matrix.info.cmd }}

- uses: actions/upload-artifact@v4
with:
name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }}
path: dist/*.whl
retention-days: 1

bdist-wheels-windows-t:
runs-on: ${{ matrix.info.machine }}
strategy:
fail-fast: false
matrix:
info:
- { machine: 'windows-2022', python: '3.13t', arch: 'amd64', cmd: '.\.github\env\Windows\bdist-wheel.ps1' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this could be rolled back into bdist-wheels-t, and maybe even bdist-wheels-t could be rolled back into bdist-wheels, once setup-python has -t support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am watching actions/setup-python#771


steps:
- uses: actions/checkout@v4

- uses: astral-sh/setup-uv@v3
- run: |
uv python install ${{ matrix.info.python }}
uv venv --python ${{ matrix.info.python }}
.venv\Scripts\Activate.ps1
uv pip install pip
echo PATH=%PATH% >> $GITHUB_ENV
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
${{ matrix.info.cmd }}
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved

- uses: actions/upload-artifact@v4
with:
name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }}
path: dist/*.whl
retention-days: 1

bdist-wheels-linux-arm64-t:
runs-on: ${{ matrix.info.machine }}
strategy:
fail-fast: false
matrix:
info:
- { machine: 'ubuntu-20.04', python: '3.13t', arch: 'aarch64', cmd: '.github/env/Linux/bdist-wheel.sh' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see note about the latest LTS version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20.04 goes EOL 4/23/2025. We may consider bumping to at least 22.04 which.

Copy link
Contributor Author

@jmao-denver jmao-denver Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devinrsmith is almost certain changing to 22.04 will make the wheels not installation due to some incompatibility in the GLIBC. Will file a follow-up ticket.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are currently targeting manylinux_2_17, which makes it compatible with GLIBC version 2.17+; afaict, the building of these artifacts needs to happen on a system w/ GLIBC version 2.17, and updating to 22.04 or later breaks this. We should have a ticket about GLIBC version support, and when we will move on past 2.17.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pypa/cibuildwheel#1772 has relevant upstream comments about the topic.


steps:
- uses: actions/checkout@v4

- name: Set up QEMU
uses: docker/setup-qemu-action@v3

- name: Build wheels
uses: pypa/[email protected]
Comment on lines +218 to +222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting, this is different workflow for building aarch64 wheels than we use for the other aarch64 wheels, which are built via aarch64 emulation in docker. If we think think this is a better way, we may consider migrating the other ones over to cibuildwheel in the future (and potentially amd64?).

The nice thing about the docker emulation is that users can build everything locally without needing to rely on CI.

Copy link
Contributor Author

@jmao-denver jmao-denver Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still uses emulation but the nice thing about CBW under aarch64 simulation is that it can build both amd64 and aarch64 wheels. Followup ticket: #173

env:
CIBW_FREE_THREADED_SUPPORT: true
CIBW_ARCHS_LINUX: "aarch64"
CIBW_BUILD: "cp313t-*"
CIBW_SKIP: "cp313t-musllinux_aarch64"
CIBW_BEFORE_ALL_LINUX: >
yum install -y java-11-openjdk-devel &&
yum install -y wget &&
wget https://www.apache.org/dist/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz -P /tmp &&
tar xf /tmp/apache-maven-3.8.8-bin.tar.gz -C /opt &&
ln -s /opt/apache-maven-3.8.8/bin/mvn /usr/bin/mvn
CIBW_ENVIRONMENT: JAVA_HOME=/etc/alternatives/jre_11_openjdk
CIBW_REPAIR_WHEEL_COMMAND_LINUX: 'auditwheel repair --exclude libjvm.so -w {dest_dir} {wheel}'

with:
package-dir: .
output-dir: dist

- uses: actions/upload-artifact@v4
with:
name: build-${{ matrix.info.python }}-${{ matrix.info.machine }}-${{ matrix.info.arch }}
path: dist/*.whl
retention-days: 1

collect-artifacts:
runs-on: ubuntu-22.04
needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64']
needs: ['sdist', 'bdist-wheel', 'bdist-wheel-universal2-hack', 'bdist-wheels-linux-arm64', 'bdist-wheels-t', 'bdist-wheels-windows-t', 'bdist-wheels-linux-arm64-t']
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/checkout@v4
- uses: actions/download-artifact@v4
Expand Down
29 changes: 29 additions & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,32 @@ jobs:

- name: Run Test
run: python setup.py test

test-free-threaded:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see windows-specific and mac-specific code, but I don't see CI checks for the platforms. There do appear to be windows runners. https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners
It isn't the highest priority, but I am noting it.

runs-on: ubuntu-22.04
strategy:
matrix:
python: ['3.13t']
java: ['8', '11', '17', '21', '23']
steps:
- uses: actions/checkout@v4

- uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: ${{ matrix.java }}

- uses: astral-sh/setup-uv@v3
- run: |
uv python install ${{ matrix.python }}
uv venv --python ${{ matrix.python }}
source .venv/bin/activate
uv pip install pip
echo $JAVA_HOME
echo PATH=$PATH >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it is still using GITHUB_ENV?


- run: pip install "setuptools < 72"

- name: Run Free-threaded Test
run: python setup.py test

30 changes: 22 additions & 8 deletions jpyutil.py
devinrsmith marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -303,18 +303,18 @@ def _find_python_dll_file(fail=False):
logger.debug("Searching for Python shared library file")

# Prepare list of search directories

search_dirs = [sys.prefix]

# installed_base/lib needs to be added to the search path for Python 3.13t files
installed_base = sysconfig.get_config_var('installed_base')
if installed_base:
search_dirs.append(os.path.join(installed_base, "lib"))

extra_search_dirs = [sysconfig.get_config_var(name) for name in PYTHON_LIB_DIR_CONFIG_VAR_NAMES]
for extra_dir in extra_search_dirs:
if extra_dir and extra_dir not in search_dirs and os.path.exists(extra_dir):
search_dirs.append(extra_dir)

if platform.system() == 'Windows':
extra_search_dirs = _get_existing_subdirs(search_dirs, "DLLs")
search_dirs = extra_search_dirs + search_dirs

jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
multi_arch_sub_dir = sysconfig.get_config_var('multiarchsubdir')
if multi_arch_sub_dir:
while multi_arch_sub_dir.startswith('/'):
Expand All @@ -326,18 +326,32 @@ def _find_python_dll_file(fail=False):

# Prepare list of possible library file names

# account for Python debug builds
try:
sys.gettotalrefcount()
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
debug_build = True
except AttributeError:
debug_build = False

# account for Python 3.13+ with GIL disabled
dll_suffix = ''
if sys.version_info >= (3, 13):
if not sys._is_gil_enabled():
dll_suffix = 't'
dll_suffix += 'd' if debug_build else ''

vmaj = str(sys.version_info.major)
vmin = str(sys.version_info.minor)

if platform.system() == 'Windows':
versions = (vmaj + vmin, vmaj, '')
versions = (vmaj + vmin, vmaj, vmaj + vmin + dll_suffix, '')
file_names = ['python' + v + '.dll' for v in versions]
elif platform.system() == 'Darwin':
versions = (vmaj + "." + vmin, vmaj, '')
versions = (vmaj + "." + vmin, vmaj, vmaj + "." + vmin + dll_suffix, '')
file_names = ['libpython' + v + '.dylib' for v in versions] + \
['libpython' + v + '.so' for v in versions]
else:
versions = (vmaj + "." + vmin, vmaj, '')
versions = (vmaj + "." + vmin, vmaj, vmaj + "." + vmin + dll_suffix, '')
file_names = ['libpython' + v + '.so' for v in versions]

logger.debug("Potential Python shared library file names: %s" % repr(file_names))
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
os.path.join(src_test_py_dir, 'jpy_java_embeddable_test.py'),
os.path.join(src_test_py_dir, 'jpy_obj_test.py'),
os.path.join(src_test_py_dir, 'jpy_eval_exec_test.py'),
os.path.join(src_test_py_dir, 'jpy_mt_eval_exec_test.py'),
]

# e.g. jdk_home_dir = '/home/marta/jdk1.7.0_15'
Expand Down
24 changes: 21 additions & 3 deletions src/main/c/jni/org_jpy_PyLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,18 @@ void dumpDict(const char* dictName, PyObject* dict)

size = PyDict_Size(dict);
printf(">> dumpDict: %s.size = %ld\n", dictName, size);
#ifdef Py_GIL_DISABLED
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
Py_BEGIN_CRITICAL_SECTION(dict);
#endif
while (PyDict_Next(dict, &pos, &key, &value)) {
const char* name;
name = JPy_AS_UTF8(key);
printf(">> dumpDict: %s[%ld].name = '%s'\n", dictName, i, name);
i++;
}
#ifdef Py_GIL_DISABLED
Py_END_CRITICAL_SECTION();
#endif
}

/**
Expand All @@ -521,7 +527,7 @@ PyObject *getMainGlobals() {
}

pyGlobals = PyModule_GetDict(pyMainModule); // borrowed ref
JPy_INCREF(pyGlobals);
JPy_XINCREF(pyGlobals);

return pyGlobals;
}
Expand Down Expand Up @@ -557,7 +563,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentGlobals

JPy_BEGIN_GIL_STATE

#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12
#if PY_VERSION_HEX < 0x030D0000
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
globals = PyEval_GetGlobals(); // borrowed ref
JPy_XINCREF(globals);
#else
Expand Down Expand Up @@ -588,7 +594,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentLocals

JPy_BEGIN_GIL_STATE

#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12
#if PY_VERSION_HEX < 0x030D0000
locals = PyEval_GetLocals(); // borrowed ref
JPy_XINCREF(locals);
#else
Expand Down Expand Up @@ -1124,7 +1130,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_incRef
if (Py_IsInitialized()) {
JPy_BEGIN_GIL_STATE

#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
#endif
JPy_DIAG_PRINT(JPy_DIAG_F_MEM, "Java_org_jpy_PyLib_incRef: pyObject=%p, refCount=%d, type='%s'\n", pyObject, refCount, Py_TYPE(pyObject)->tp_name);
JPy_INCREF(pyObject);

Expand All @@ -1150,7 +1160,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRef
if (Py_IsInitialized()) {
JPy_BEGIN_GIL_STATE

#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
#endif
if (refCount <= 0) {
JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRef: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount);
} else {
Expand Down Expand Up @@ -1183,7 +1197,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRefs
buf = (*jenv)->GetLongArrayElements(jenv, objIds, &isCopy);
for (i = 0; i < len; i++) {
pyObject = (PyObject*) buf[i];
#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
#endif
if (refCount <= 0) {
JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRefs: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount);
} else {
Expand Down
2 changes: 2 additions & 0 deletions src/main/c/jpy_jmethod.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,7 @@ JPy_JMethod* JOverloadedMethod_FindMethod0(JNIEnv* jenv, JPy_JOverloadedMethod*
overloadedMethod->declaringClass->javaName, JPy_AS_UTF8(overloadedMethod->name), overloadCount, argCount);

for (i = 0; i < overloadCount; i++) {
// borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently
currMethod = (JPy_JMethod*) PyList_GetItem(overloadedMethod->methodList, i);

if (currMethod->isVarArgs && matchValueMax > 0 && !bestMethod->isVarArgs) {
Expand Down Expand Up @@ -950,6 +951,7 @@ int JOverloadedMethod_AddMethod(JPy_JOverloadedMethod* overloadedMethod, JPy_JMe
// we need to insert this before the first varargs method
Py_ssize_t size = PyList_Size(overloadedMethod->methodList);
for (ii = 0; ii < size; ii++) {
// borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently
PyObject *check = PyList_GetItem(overloadedMethod->methodList, ii);
if (((JPy_JMethod *) check)->isVarArgs) {
// this is the first varargs method, so we should go before it
Expand Down
16 changes: 14 additions & 2 deletions src/main/c/jpy_jobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,30 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef)
}


// we check the type translations dictionary for a callable for this java type name,
// we check the type translations dictionary for a callable for this java type name,
// and apply the returned callable to the wrapped object
callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName);
#ifdef Py_GIL_DISABLED
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
// if return is 1, callable is new reference
if (PyDict_GetItemStringRef(JPy_Type_Translations, type->javaName, &callable) != 1) {
callable = NULL;
}
#else
callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
JPy_XINCREF(callable);
#endif

if (callable != NULL) {
if (PyCallable_Check(callable)) {
callableResult = PyObject_CallFunction(callable, "OO", type, obj);
JPy_XDECREF(callable);
jmao-denver marked this conversation as resolved.
Show resolved Hide resolved
if (callableResult == NULL) {
Py_RETURN_NONE;
} else {
return callableResult;
}
}
}
JPy_XDECREF(callable);

return (PyObject *)obj;
}
Expand All @@ -103,6 +114,7 @@ int JObj_init_internal(JNIEnv* jenv, JPy_JObj* self, PyObject* args, PyObject* k

type = ((PyObject*) self)->ob_type;

// borrowed ref, no need to replace with PyDict_GetItemStringRef because tp_dict won't be changed concurrently
constructor = PyDict_GetItemString(type->tp_dict, JPy_JTYPE_ATTR_NAME_JINIT);
if (constructor == NULL) {
PyErr_SetString(PyExc_RuntimeError, "no constructor found (missing JType attribute '" JPy_JTYPE_ATTR_NAME_JINIT "')");
Expand Down
Loading
Loading