-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
MAINT: use an atomic load/store and a mutex to initialize the argparse and runtime import caches #26780
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
MAINT: use an atomic load/store and a mutex to initialize the argparse and runtime import caches #26780
Changes from all commits
e55b25c
6386423
9a22d84
6ea18a4
3c620cb
2e2fa1d
b6af99a
dabfe59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
/* | ||
* Provides wrappers around C11 standard library atomics and MSVC intrinsics | ||
* to provide basic atomic load and store functionality. This is based on | ||
* code in CPython's pyatomic.h, pyatomic_std.h, and pyatomic_msc.h | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
|
||
#ifndef NUMPY_CORE_SRC_COMMON_NPY_ATOMIC_H_ | ||
#define NUMPY_CORE_SRC_COMMON_NPY_ATOMIC_H_ | ||
|
||
#include "numpy/npy_common.h" | ||
|
||
#if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_ATOMICS__) | ||
// TODO: support C++ atomics as well if this header is ever needed in C++ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code to do this is in the CPython header this is cribbed from. It would be dead code if I included it. |
||
#include <stdatomic.h> | ||
#include <stdint.h> | ||
#define STDC_ATOMICS | ||
#elif _MSC_VER | ||
#include <intrin.h> | ||
#define MSC_ATOMICS | ||
#if !defined(_M_X64) && !defined(_M_IX86) && !defined(_M_ARM64) | ||
#error "Unsupported MSVC build configuration, neither x86 or ARM" | ||
#endif | ||
#elif defined(__GNUC__) && (__GNUC__ > 4) | ||
#define GCC_ATOMICS | ||
#elif defined(__clang__) | ||
#if __has_builtin(__atomic_load) | ||
#define GCC_ATOMICS | ||
#endif | ||
#else | ||
#error "no supported atomic implementation for this platform/compiler" | ||
#endif | ||
|
||
|
||
static inline npy_uint8 | ||
npy_atomic_load_uint8(const npy_uint8 *obj) { | ||
#ifdef STDC_ATOMICS | ||
return (npy_uint8)atomic_load((const _Atomic(uint8_t)*)obj); | ||
#elif defined(MSC_ATOMICS) | ||
#if defined(_M_X64) || defined(_M_IX86) | ||
return *(volatile npy_uint8 *)obj; | ||
#else // defined(_M_ARM64) | ||
return (npy_uint8)__ldar8((unsigned __int8 volatile *)obj); | ||
#endif | ||
#elif defined(GCC_ATOMICS) | ||
return __atomic_load_n(obj, __ATOMIC_SEQ_CST); | ||
#endif | ||
} | ||
|
||
static inline void* | ||
npy_atomic_load_ptr(const void *obj) { | ||
#ifdef STDC_ATOMICS | ||
return atomic_load((const _Atomic(void *)*)obj); | ||
#elif defined(MSC_ATOMICS) | ||
#if SIZEOF_VOID_P == 8 | ||
#if defined(_M_X64) || defined(_M_IX86) | ||
return *(volatile uint64_t *)obj; | ||
#elif defined(_M_ARM64) | ||
return (uint64_t)__ldar64((unsigned __int64 volatile *)obj); | ||
#endif | ||
#else | ||
#if defined(_M_X64) || defined(_M_IX86) | ||
return *(volatile uint32_t *)obj; | ||
#elif defined(_M_ARM64) | ||
return (uint32_t)__ldar32((unsigned __int32 volatile *)obj); | ||
#endif | ||
#endif | ||
#elif defined(GCC_ATOMICS) | ||
return (void *)__atomic_load_n((void * const *)obj, __ATOMIC_SEQ_CST); | ||
#endif | ||
} | ||
|
||
static inline void | ||
npy_atomic_store_uint8(npy_uint8 *obj, npy_uint8 value) { | ||
#ifdef STDC_ATOMICS | ||
atomic_store((_Atomic(uint8_t)*)obj, value); | ||
#elif defined(MSC_ATOMICS) | ||
_InterlockedExchange8((volatile char *)obj, (char)value); | ||
#elif defined(GCC_ATOMICS) | ||
__atomic_store_n(obj, value, __ATOMIC_SEQ_CST); | ||
#endif | ||
} | ||
|
||
static inline void | ||
npy_atomic_store_ptr(void *obj, void *value) | ||
{ | ||
#ifdef STDC_ATOMICS | ||
atomic_store((_Atomic(void *)*)obj, value); | ||
#elif defined(MSC_ATOMICS) | ||
_InterlockedExchangePointer((void * volatile *)obj, (void *)value); | ||
#elif defined(GCC_ATOMICS) | ||
__atomic_store_n((void **)obj, value, __ATOMIC_SEQ_CST); | ||
#endif | ||
} | ||
|
||
#undef MSC_ATOMICS | ||
#undef STDC_ATOMICS | ||
#undef GCC_ATOMICS | ||
|
||
#endif // NUMPY_CORE_SRC_COMMON_NPY_NPY_ATOMIC_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#define NPY_NO_DEPRECATED_API NPY_API_VERSION | ||
#define _MULTIARRAYMODULE | ||
|
||
#include "numpy/ndarraytypes.h" | ||
#include "npy_import.h" | ||
#include "npy_atomic.h" | ||
|
||
|
||
NPY_VISIBILITY_HIDDEN npy_runtime_imports_struct npy_runtime_imports; | ||
|
||
NPY_NO_EXPORT int | ||
init_import_mutex(void) { | ||
npy_runtime_imports.import_mutex = PyThread_allocate_lock(); | ||
if (npy_runtime_imports.import_mutex == NULL) { | ||
PyErr_NoMemory(); | ||
return -1; | ||
} | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,70 @@ | |
|
||
#include <Python.h> | ||
|
||
/*! \brief Fetch and cache Python function. | ||
#include "numpy/npy_common.h" | ||
#include "npy_atomic.h" | ||
|
||
/* | ||
* Cached references to objects obtained via an import. All of these are | ||
* can be initialized at any time by npy_cache_import_runtime. | ||
*/ | ||
typedef struct npy_runtime_imports_struct { | ||
PyThread_type_lock import_mutex; | ||
PyObject *_add_dtype_helper; | ||
PyObject *_all; | ||
PyObject *_amax; | ||
PyObject *_amin; | ||
PyObject *_any; | ||
PyObject *array_function_errmsg_formatter; | ||
PyObject *array_ufunc_errmsg_formatter; | ||
PyObject *_clip; | ||
PyObject *_commastring; | ||
PyObject *_convert_to_stringdtype_kwargs; | ||
PyObject *_default_array_repr; | ||
PyObject *_default_array_str; | ||
PyObject *_dump; | ||
PyObject *_dumps; | ||
PyObject *_getfield_is_safe; | ||
PyObject *internal_gcd_func; | ||
PyObject *_mean; | ||
PyObject *NO_NEP50_WARNING; | ||
PyObject *npy_ctypes_check; | ||
PyObject *numpy_matrix; | ||
PyObject *_prod; | ||
PyObject *_promote_fields; | ||
PyObject *_std; | ||
PyObject *_sum; | ||
PyObject *_ufunc_doc_signature_formatter; | ||
PyObject *_var; | ||
PyObject *_view_is_safe; | ||
PyObject *_void_scalar_to_string; | ||
} npy_runtime_imports_struct; | ||
|
||
NPY_VISIBILITY_HIDDEN extern npy_runtime_imports_struct npy_runtime_imports; | ||
|
||
/*! \brief Import a Python object. | ||
|
||
* This function imports the Python function specified by | ||
* \a module and \a function, increments its reference count, and returns | ||
* the result. On error, returns NULL. | ||
* | ||
* @param module Absolute module name. | ||
* @param attr module attribute to cache. | ||
*/ | ||
static inline PyObject* | ||
npy_import(const char *module, const char *attr) | ||
{ | ||
PyObject *ret = NULL; | ||
PyObject *mod = PyImport_ImportModule(module); | ||
|
||
if (mod != NULL) { | ||
ret = PyObject_GetAttrString(mod, attr); | ||
Py_DECREF(mod); | ||
} | ||
return ret; | ||
} | ||
|
||
/*! \brief Fetch and cache Python object at runtime. | ||
* | ||
* Import a Python function and cache it for use. The function checks if | ||
* cache is NULL, and if not NULL imports the Python function specified by | ||
|
@@ -16,17 +79,24 @@ | |
* @param attr module attribute to cache. | ||
* @param cache Storage location for imported function. | ||
*/ | ||
static inline void | ||
npy_cache_import(const char *module, const char *attr, PyObject **cache) | ||
{ | ||
if (NPY_UNLIKELY(*cache == NULL)) { | ||
PyObject *mod = PyImport_ImportModule(module); | ||
|
||
if (mod != NULL) { | ||
*cache = PyObject_GetAttrString(mod, attr); | ||
Py_DECREF(mod); | ||
static inline int | ||
npy_cache_import_runtime(const char *module, const char *attr, PyObject **obj) { | ||
if (!npy_atomic_load_ptr(obj)) { | ||
PyObject* value = npy_import(module, attr); | ||
if (value == NULL) { | ||
return -1; | ||
} | ||
PyThread_acquire_lock(npy_runtime_imports.import_mutex, WAIT_LOCK); | ||
if (!npy_atomic_load_ptr(obj)) { | ||
npy_atomic_store_ptr(obj, Py_NewRef(value)); | ||
} | ||
PyThread_release_lock(npy_runtime_imports.import_mutex); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the release before the error check, or we will dead-lock on error! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! Also, the But I think it would be better not to hold the lock around the
So I'd suggest writing this as: if (!npy_atomic_load_ptr(obj)) {
PyObject *value = npy_import(module, attr);
if (value == NULL) {
return -1;
}
PyThread_acquire_lock(npy_runtime_imports.import_mutex, WAIT_LOCK);
if (!npy_atomic_load_ptr(obj)) {
npy_atomic_store_ptr(obj, Py_NewRef(value));
}
PyThread_release_lock(npy_runtime_imports.import_mutex);
Py_DECREF(value);
} Or you can get rid of the lock if you implement compare-exchange: if (!npy_atomic_load_ptr(obj)) {
PyObject *value = npy_import(module, attr);
if (value == NULL) {
return -1;
}
PyObject *exepected = NULL;
if (!npy_atomic_compare_exchange_ptr(obj, &expected, value)) {
Py_DECREF(value);
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I kept the lock since the next iteration will use PyMutex and I'd like to make sure this gets in for NumPy 2.1. Thankfully deadsnakes was updated over the weekend so we can test against a version with a public PyMutex now. |
||
Py_DECREF(value); | ||
} | ||
return 0; | ||
} | ||
|
||
NPY_NO_EXPORT int | ||
init_import_mutex(void); | ||
|
||
#endif /* NUMPY_CORE_SRC_COMMON_NPY_IMPORT_H_ */ |
Uh oh!
There was an error while loading. Please reload this page.