Skip to content

Commit 83077a2

Browse files
committed
Merge branch 'super_ptr_wise' into patch-4
2 parents cf6235f + e911243 commit 83077a2

8 files changed

Lines changed: 266 additions & 68 deletions

File tree

Include/internal/pycore_object.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,35 @@ static inline Py_ALWAYS_INLINE void _Py_INCREF_MORTAL(PyObject *op)
10391039
* references. */
10401040
PyAPI_FUNC(int) _PyObject_VisitType(PyObject *op, visitproc visit, void *arg);
10411041

1042+
// Pointer-by-pointer memmove for PyObject** arrays that is safe for shared
1043+
// objects in Py_GIL_DISABLED builds. Locking is the caller's responsibility.
1044+
static inline void
1045+
_Py_ptr_wise_atomic_memmove(PyObject *a, PyObject **dest, PyObject **src,
1046+
Py_ssize_t n)
1047+
{
1048+
#ifndef Py_GIL_DISABLED
1049+
(void)a;
1050+
memmove(dest, src, n * sizeof(PyObject *));
1051+
#else
1052+
if (_Py_IsOwnedByCurrentThread(a) && !_PyObject_GC_IS_SHARED(a)) {
1053+
// No other threads can read this object's array concurrently
1054+
memmove(dest, src, n * sizeof(PyObject *));
1055+
return;
1056+
}
1057+
if (dest < src) {
1058+
for (Py_ssize_t i = 0; i != n; i++) {
1059+
_Py_atomic_store_ptr_release(&dest[i], src[i]);
1060+
}
1061+
}
1062+
else {
1063+
// copy backwards to avoid overwriting src before it's read
1064+
for (Py_ssize_t i = n; i != 0; i--) {
1065+
_Py_atomic_store_ptr_release(&dest[i - 1], src[i - 1]);
1066+
}
1067+
}
1068+
#endif
1069+
}
1070+
10421071
#ifdef __cplusplus
10431072
}
10441073
#endif

Modules/Setup.stdlib.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@
172172
@MODULE_XXSUBTYPE_TRUE@xxsubtype xxsubtype.c
173173
@MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
174174
@MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c
175-
@MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c _testinternalcapi/complex.c _testinternalcapi/interpreter.c _testinternalcapi/tuple.c
175+
@MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/test_ptr_wise_memmove.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c _testinternalcapi/complex.c _testinternalcapi/interpreter.c _testinternalcapi/tuple.c
176176
@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/heaptype.c _testcapi/abstract.c _testcapi/unicode.c _testcapi/dict.c _testcapi/set.c _testcapi/list.c _testcapi/tuple.c _testcapi/getargs.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/complex.c _testcapi/numbers.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/buffer.c _testcapi/pyatomic.c _testcapi/run.c _testcapi/file.c _testcapi/codec.c _testcapi/immortal.c _testcapi/gc.c _testcapi/hash.c _testcapi/time.c _testcapi/bytes.c _testcapi/object.c _testcapi/modsupport.c _testcapi/monitoring.c _testcapi/config.c _testcapi/import.c _testcapi/frame.c _testcapi/type.c _testcapi/function.c _testcapi/module.c _testcapi/weakref.c
177177
@MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/import.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/slots.c _testlimitedcapi/sys.c _testlimitedcapi/threadstate.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c _testlimitedcapi/file.c _testlimitedcapi/weakref.c
178178
@MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c

Modules/_elementtree.c

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "Python.h"
1919
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
2020
#include "pycore_dict.h" // _PyDict_CopyAsDict()
21+
#include "pycore_object.h" // _Py_ptr_wise_atomic_memmove()
2122
#include "pycore_pyhash.h" // _Py_HashSecret
2223
#include "pycore_tuple.h" // _PyTuple_FromPair
2324
#include "pycore_weakref.h" // FT_CLEAR_WEAKREFS()
@@ -1863,34 +1864,6 @@ element_subscr(PyObject *op, PyObject *item)
18631864
}
18641865
}
18651866

1866-
// Pointer-by-pointer memmove for PyObject** arrays that is safe
1867-
// for shared ElementObjects in Py_GIL_DISABLED builds.
1868-
static void
1869-
ptr_wise_atomic_memmove(ElementObject *a, PyObject **dest, PyObject **src, Py_ssize_t n)
1870-
{
1871-
#ifndef Py_GIL_DISABLED
1872-
memmove(dest, src, n * sizeof(PyObject *));
1873-
#else
1874-
// XXX: maybe a critical section isn't needed for ElementObject?
1875-
// _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a);
1876-
if (_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) {
1877-
// No other threads can read this list concurrently
1878-
memmove(dest, src, n * sizeof(PyObject *));
1879-
return;
1880-
}
1881-
if (dest < src) {
1882-
for (Py_ssize_t i = 0; i != n; i++) {
1883-
_Py_atomic_store_ptr_release(&dest[i], src[i]);
1884-
}
1885-
}
1886-
else {
1887-
// copy backwards to avoid overwriting src before it's read
1888-
for (Py_ssize_t i = n; i != 0; i--) {
1889-
_Py_atomic_store_ptr_release(&dest[i - 1], src[i - 1]);
1890-
}
1891-
}
1892-
#endif
1893-
}
18941867

18951868
static int
18961869
element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
@@ -1967,8 +1940,8 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
19671940

19681941
PyList_SET_ITEM(recycle, i, self->extra->children[cur]);
19691942

1970-
ptr_wise_atomic_memmove(
1971-
self,
1943+
_Py_ptr_wise_atomic_memmove(
1944+
(PyObject *)self,
19721945
self->extra->children + cur - i,
19731946
self->extra->children + cur + 1,
19741947
num_moved);
@@ -1977,8 +1950,8 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
19771950
/* Leftover "tail" after the last removed child */
19781951
cur = start + (size_t)slicelen * step;
19791952
if (cur < (size_t)self->extra->length) {
1980-
ptr_wise_atomic_memmove(
1981-
self,
1953+
_Py_ptr_wise_atomic_memmove(
1954+
(PyObject *)self,
19821955
self->extra->children + cur - slicelen,
19831956
self->extra->children + cur,
19841957
self->extra->length - cur);

Modules/_testinternalcapi.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,6 +3340,9 @@ module_exec(PyObject *module)
33403340
if (_PyTestInternalCapi_Init_PyTime(module) < 0) {
33413341
return 1;
33423342
}
3343+
if (_PyTestInternalCapi_Init_PtrWiseMemmove(module) < 0) {
3344+
return 1;
3345+
}
33433346
if (_PyTestInternalCapi_Init_Set(module) < 0) {
33443347
return 1;
33453348
}

Modules/_testinternalcapi/parts.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
int _PyTestInternalCapi_Init_Lock(PyObject *module);
1414
int _PyTestInternalCapi_Init_PyTime(PyObject *module);
15+
int _PyTestInternalCapi_Init_PtrWiseMemmove(PyObject *module);
1516
int _PyTestInternalCapi_Init_Set(PyObject *module);
1617
int _PyTestInternalCapi_Init_Complex(PyObject *module);
1718
int _PyTestInternalCapi_Init_CriticalSection(PyObject *module);
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// Tests for _Py_ptr_wise_atomic_memmove() in pycore_object.h
2+
3+
#include "parts.h"
4+
#include "pycore_object.h" // _Py_ptr_wise_atomic_memmove()
5+
#include "pycore_gc.h" // _PyObject_GC_SET_SHARED()
6+
7+
// Five distinguishable immortal singletons used as placeholder pointers.
8+
// These require no reference-count management when stored in a raw array.
9+
#define NPTRS 5
10+
static PyObject *test_objs[NPTRS];
11+
12+
static void
13+
setup_test_objs(void)
14+
{
15+
test_objs[0] = Py_None;
16+
test_objs[1] = Py_True;
17+
test_objs[2] = Py_False;
18+
test_objs[3] = Py_Ellipsis;
19+
test_objs[4] = Py_NotImplemented;
20+
}
21+
22+
// Fill buf[0..NPTRS-1] with test_objs in order.
23+
static void
24+
fill_buf(PyObject **buf)
25+
{
26+
for (int i = 0; i < NPTRS; i++) {
27+
buf[i] = test_objs[i];
28+
}
29+
}
30+
31+
// Return a fresh empty PyListObject whose GC SHARED bit is set in
32+
// Py_GIL_DISABLED builds. This forces _Py_ptr_wise_atomic_memmove()
33+
// to take the atomic (non-fast) path so we can exercise all branches.
34+
// In non-GIL builds the function always uses memmove, so no flag is needed.
35+
static PyObject *
36+
make_shared_container(void)
37+
{
38+
PyObject *a = PyList_New(0);
39+
if (a == NULL) {
40+
return NULL;
41+
}
42+
#ifdef Py_GIL_DISABLED
43+
_PyObject_GC_SET_SHARED(a);
44+
#endif
45+
return a;
46+
}
47+
48+
// Helper: create container, call the function, return it for cleanup.
49+
// Returns NULL (with exception set) on allocation failure.
50+
static PyObject *
51+
call_memmove(PyObject **dest, PyObject **src, Py_ssize_t n)
52+
{
53+
PyObject *a = make_shared_container();
54+
if (a != NULL) {
55+
_Py_ptr_wise_atomic_memmove(a, dest, src, n);
56+
}
57+
return a;
58+
}
59+
60+
61+
// dest < src: forward pointer-by-pointer copy.
62+
// buf = [0,1,2,3,4], copy src=&buf[2] n=3 to dest=&buf[0]
63+
// Expected result: buf = [2,3,4,3,4]
64+
static PyObject *
65+
test_memmove_dest_lt_src(PyObject *self, PyObject *Py_UNUSED(arg))
66+
{
67+
setup_test_objs();
68+
69+
PyObject *buf[NPTRS];
70+
fill_buf(buf);
71+
72+
PyObject *a = call_memmove(&buf[0], &buf[2], 3);
73+
if (a == NULL) {
74+
return NULL;
75+
}
76+
77+
assert(buf[0] == test_objs[2]);
78+
assert(buf[1] == test_objs[3]);
79+
assert(buf[2] == test_objs[4]);
80+
assert(buf[3] == test_objs[3]); // unchanged
81+
assert(buf[4] == test_objs[4]); // unchanged
82+
83+
Py_DECREF(a);
84+
Py_RETURN_NONE;
85+
}
86+
87+
// dest > src: backward pointer-by-pointer copy.
88+
// buf = [0,1,2,3,4], copy src=&buf[0] n=3 to dest=&buf[2]
89+
// Expected result: buf = [0,1,0,1,2]
90+
static PyObject *
91+
test_memmove_dest_gt_src(PyObject *self, PyObject *Py_UNUSED(arg))
92+
{
93+
setup_test_objs();
94+
95+
PyObject *buf[NPTRS];
96+
fill_buf(buf);
97+
98+
PyObject *a = call_memmove(&buf[2], &buf[0], 3);
99+
if (a == NULL) {
100+
return NULL;
101+
}
102+
103+
assert(buf[0] == test_objs[0]); // unchanged
104+
assert(buf[1] == test_objs[1]); // unchanged
105+
assert(buf[2] == test_objs[0]);
106+
assert(buf[3] == test_objs[1]);
107+
assert(buf[4] == test_objs[2]);
108+
109+
Py_DECREF(a);
110+
Py_RETURN_NONE;
111+
}
112+
113+
// dest == src: backward copy where every write is a no-op.
114+
// buf = [0,1,2,3,4], copy src=&buf[1] n=3 to dest=&buf[1]
115+
// Expected result: buf unchanged.
116+
static PyObject *
117+
test_memmove_dest_eq_src(PyObject *self, PyObject *Py_UNUSED(arg))
118+
{
119+
setup_test_objs();
120+
121+
PyObject *buf[NPTRS];
122+
fill_buf(buf);
123+
124+
PyObject *a = call_memmove(&buf[1], &buf[1], 3);
125+
if (a == NULL) {
126+
return NULL;
127+
}
128+
129+
for (int i = 0; i < NPTRS; i++) {
130+
assert(buf[i] == test_objs[i]);
131+
}
132+
133+
Py_DECREF(a);
134+
Py_RETURN_NONE;
135+
}
136+
137+
// Overlapping ranges, dest < src: forward copy is safe.
138+
// buf = [0,1,2,3,4], copy src=&buf[2] n=3 to dest=&buf[1]
139+
// Forward: buf[1]=buf[2]=2, buf[2]=buf[3]=3, buf[3]=buf[4]=4
140+
// Expected result: buf = [0,2,3,4,4]
141+
static PyObject *
142+
test_memmove_overlapping(PyObject *self, PyObject *Py_UNUSED(arg))
143+
{
144+
setup_test_objs();
145+
146+
PyObject *buf[NPTRS];
147+
fill_buf(buf);
148+
149+
PyObject *a = call_memmove(&buf[1], &buf[2], 3);
150+
if (a == NULL) {
151+
return NULL;
152+
}
153+
154+
assert(buf[0] == test_objs[0]); // unchanged
155+
assert(buf[1] == test_objs[2]);
156+
assert(buf[2] == test_objs[3]);
157+
assert(buf[3] == test_objs[4]);
158+
assert(buf[4] == test_objs[4]); // unchanged
159+
160+
Py_DECREF(a);
161+
Py_RETURN_NONE;
162+
}
163+
164+
// Single owner fast path (GIL_DISABLED only): object owned by this thread
165+
// and not GC-shared => memmove is used instead of atomic stores.
166+
// In non-GIL builds this always takes the memmove path, so the test
167+
// degenerates to a basic correctness check.
168+
static PyObject *
169+
test_memmove_single_owner(PyObject *self, PyObject *Py_UNUSED(arg))
170+
{
171+
setup_test_objs();
172+
173+
PyObject *a = PyList_New(0);
174+
if (a == NULL) {
175+
return NULL;
176+
}
177+
178+
#ifdef Py_GIL_DISABLED
179+
// A freelist-reused list may carry the SHARED bit set by a sibling test.
180+
// Clear it explicitly so this test exercises the single-owner fast path.
181+
_PyObject_CLEAR_GC_BITS(a, _PyGC_BITS_SHARED);
182+
assert(_Py_IsOwnedByCurrentThread(a) && !_PyObject_GC_IS_SHARED(a));
183+
#endif
184+
185+
PyObject *buf[NPTRS];
186+
fill_buf(buf);
187+
188+
_Py_ptr_wise_atomic_memmove(a, &buf[0], &buf[2], 3);
189+
190+
assert(buf[0] == test_objs[2]);
191+
assert(buf[1] == test_objs[3]);
192+
assert(buf[2] == test_objs[4]);
193+
194+
Py_DECREF(a);
195+
Py_RETURN_NONE;
196+
}
197+
198+
199+
static PyMethodDef test_methods[] = {
200+
{"test_memmove_dest_lt_src", test_memmove_dest_lt_src, METH_NOARGS},
201+
{"test_memmove_dest_gt_src", test_memmove_dest_gt_src, METH_NOARGS},
202+
{"test_memmove_dest_eq_src", test_memmove_dest_eq_src, METH_NOARGS},
203+
{"test_memmove_overlapping", test_memmove_overlapping, METH_NOARGS},
204+
{"test_memmove_single_owner", test_memmove_single_owner, METH_NOARGS},
205+
{NULL},
206+
};
207+
208+
int
209+
_PyTestInternalCapi_Init_PtrWiseMemmove(PyObject *mod)
210+
{
211+
return PyModule_AddFunctions(mod, test_methods);
212+
}

0 commit comments

Comments
 (0)