Generalized ptr_wise_atomic_{memmove,memcpy} as part of public API

Copied from Generalized ptr_wise_atomic_{memmove,memcpy} as part of public API · Issue #106 · capi-workgroup/decisions

Inspired by cpython/Objects/listobject.c at main · kumaraditya303/cpython · GitHub, I propose to extend ptr_wise_atomic_memmove beyond PyListObjects. Since the proposed function ought to accept any Python C object type convertible from PyObject, here’s a sketch:

void
PyMem_PtrWiseAtomicMemmove(PyObject *a, PyObject **dest, PyObject **src, Py_ssize_t n)
{
#ifndef Py_GIL_DISABLED
    memmove(dest, src, n * sizeof(PyObject *));
#else
    // List built-in types with internal locks. May be more
    if (PyDict_Check(a) || PyList_Check(a) || PyDict_Check(a))
        _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a);
    if (_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) {
        // No other threads can read this list concurrently
        memmove(dest, src, n * sizeof(PyObject *));
        return;
    }
    if (dest < src) {
        for (Py_ssize_t i = 0; i != n; i++) {
            _Py_atomic_store_ptr_release(&dest[i], src[i]);
        }
    }
    else {
        // copy backwards to avoid overwriting src before it's read
        for (Py_ssize_t i = n; i != 0; i--) {
            _Py_atomic_store_ptr_release(&dest[i - 1], src[i - 1]);
        }
    }
#endif
}

void
PyMem_PtrWiseAtomicMemcpy(PyObject *a, PyObject **dest, PyObject **src, Py_ssize_t n)
{
#ifndef Py_GIL_DISABLED
    memcpy(dest, src, n * sizeof(PyObject *));
#else
    // List built-in types with internal locks. May be more
    if (PyDict_Check(a) || PyList_Check(a) || PyDict_Check(a))
        _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a);
    if (_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) {
        // No other threads can read this list concurrently
        memcpy(dest, src, n * sizeof(PyObject *));
        return;
    }
    if (dest < src) {
        for (Py_ssize_t i = 0; i != n; i++) {
            _Py_atomic_store_ptr_release(&dest[i], src[i]);
        }
    }
    else {
        // copy backwards to avoid overwriting src before it's read
        for (Py_ssize_t i = n; i != 0; i--) {
            _Py_atomic_store_ptr_release(&dest[i - 1], src[i - 1]);
        }
    }
#endif
}

This function seems to be very specific. I’m not sure that it’s a good idea to propose a general API for that.

Which projects are supposed to use such function? For what purpose?

1 Like

This function seems to be very specific. I’m not sure that it’s a good idea to propose a general API for that.

Which projects are supposed to use such function? For what purpose?

Maybe a public API is too broad of an proposal…
Perhaps prefix it with PyUnstable_ for now. As for the purpose, I took inspiration from gh-149816: fix thread safety of deletion of list slice by kumaraditya303 · Pull Request #149936 · python/cpython · GitHub, where I wanted to improve thread safety of memmoves and memsets within the free-threaded interpreter

Is there a project other than CPython that would use these?
I’d be interested in the documentation for these functions. What’s the difference between the two? What do you need to ensure before you call them?

These functions assume the following:

  • The number of bytes to be moved or copied to another object pointer is a multiple of sizeof(PyObject*)
  • On interpreters with GIL, they’re mere aliases for standard memcpy and memmove. On free-threaded builds, the functions are the same, simply swapping out which standard mem_ standard library function is called. Furthermore, both check if the PyObject is owned by the thread calling this function and references of the same object is not shared between multiple threads. If both conditions are true, the standard mem_ function is invoked, otherwise it falls back to atomic byte-by-byte moving or storing with release semantics.

In the past, there’s been reluctance to expose the atomic operations as public API on the basis that it’s something that people should be getting from standard library headers and not from Python.

If we’re going to argue against that position, then this doesn’t feel like the first thing I’d pick to expose.

1 Like

If everyone else feels that these should not be part of the public API, even if prefixed with PyUnstable_ or _Py, then it’s fine by me.

My main intent was to touch internal operations

1 Like

@clin1234 Maybe you should consider adding this as a private/internal API, which would be useful for CPython and built-in extension implementations.

See Add `PyTuple_FromPair` · Issue #145247 · python/cpython · GitHub