Repository navigation
Conversation
|
If a list writer is used by multiple threads, |
|
This C API is just an abstraction on top of existing PyList functions. |
Documentation build overview
19 files changed ·
|
|
Another advantage of using this C API instead of |
Oh, this failure seems to be unrelated to my change: |
|
I ran a benchmark to compare
I ran the benchmark on Free Threaded build on Linux ( import pyperf, functools, _testcapi
runner = pyperf.Runner()
sizes = (0, 10, 100, 1_000)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_setitem_macro, test_list)
runner.bench_time_func(f'setitem macro {list_len:,}-list', func)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_setitem_func, test_list)
runner.bench_time_func(f'setitem func {list_len:,}-list', func)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_writer, test_list)
runner.bench_time_func(f'writer {list_len:,}-list', func)Results:
On Free Threading, Patch adding 3 bench functions to _testcapi: Detailsdiff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 033ee3234cf..a3a8d9989c0 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2889,6 +2889,112 @@ uptime_bsd(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
#endif
+static PyObject*
+bench_setitem_macro(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyObject *result = PyList_New(len);
+ if (result == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = Py_NewRef(PyList_GET_ITEM(list, j));
+ PyList_SET_ITEM(result, j, item);
+ }
+
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_setitem_func(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyObject *result = PyList_New(len);
+ if (result == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = Py_NewRef(PyList_GET_ITEM(list, j));
+ if (PyList_SetItem(result, j, item) < 0) {
+ Py_DECREF(result);
+ return NULL;
+ }
+ }
+
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_writer(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyListWriter *writer = PyListWriter_Create(len);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = PyList_GET_ITEM(list, j);
+ if (PyListWriter_Append(writer, item) < 0) {
+ PyListWriter_Discard(writer);
+ return NULL;
+ }
+ }
+
+ PyObject *result = PyListWriter_Finish(writer);
+ if (result == NULL) {
+ return NULL;
+ }
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
static PyMethodDef TestMethods[] = {
{"set_errno", set_errno, METH_VARARGS},
{"test_config", test_config, METH_NOARGS},
@@ -2988,6 +3094,9 @@ static PyMethodDef TestMethods[] = {
#ifdef HAVE_SYSCTLBYNAME
{"uptime_bsd", uptime_bsd, METH_NOARGS},
#endif
+ {"bench_setitem_macro", bench_setitem_macro, METH_VARARGS},
+ {"bench_setitem_func", bench_setitem_func, METH_VARARGS},
+ {"bench_writer", bench_writer, METH_VARARGS},
{NULL, NULL} /* sentinel */
};
|
|
I also ran benchmarks on
Well, it's not surprising that Benchmark script: Detailsimport pyperf, functools, _testcapi
runner = pyperf.Runner()
sizes = (0, 10, 100, 1_000)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_setitem_macro, test_list)
runner.bench_time_func(f'setitem macro {list_len:,}-list', func)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_writer_append, test_list)
runner.bench_time_func(f'writer append {list_len:,}-list', func)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_writer_extend, test_list)
runner.bench_time_func(f'writer extend {list_len:,}-list', func)
for list_len in sizes:
test_list = list(range(list_len))
func = functools.partial(_testcapi.bench_writer_extend_array, test_list)
runner.bench_time_func(f'writer extend array {list_len:,}-list', func)Patch adding C benchmarks: Detailsdiff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 033ee3234cf..c191d12c862 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2889,6 +2889,185 @@ uptime_bsd(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
#endif
+static PyObject*
+bench_setitem_macro(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyObject *result = PyList_New(len);
+ if (result == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = Py_NewRef(PyList_GET_ITEM(list, j));
+ PyList_SET_ITEM(result, j, item);
+ }
+
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_setitem_func(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyObject *result = PyList_New(len);
+ if (result == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = Py_NewRef(PyList_GET_ITEM(list, j));
+ if (PyList_SetItem(result, j, item) < 0) {
+ Py_DECREF(result);
+ return NULL;
+ }
+ }
+
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_writer_append(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyListWriter *writer = PyListWriter_Create(len);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = PyList_GET_ITEM(list, j);
+ if (PyListWriter_Append(writer, item) < 0) {
+ PyListWriter_Discard(writer);
+ return NULL;
+ }
+ }
+
+ PyObject *result = PyListWriter_Finish(writer);
+ if (result == NULL) {
+ return NULL;
+ }
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_writer_extend(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyListWriter *writer = PyListWriter_Create(len);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ if (PyListWriter_Extend(writer, list) < 0) {
+ PyListWriter_Discard(writer);
+ return NULL;
+ }
+
+ PyObject *result = PyListWriter_Finish(writer);
+ if (result == NULL) {
+ return NULL;
+ }
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_writer_extend_array(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *list;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &list, &loops)) {
+ return NULL;
+ }
+
+ PyObject **list_items = _PyList_CAST(list)->ob_item;
+ Py_ssize_t len = PyList_GET_SIZE(list);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyListWriter *writer = PyListWriter_Create(len);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ if (PyListWriter_ExtendArray(writer, list_items, len) < 0) {
+ PyListWriter_Discard(writer);
+ return NULL;
+ }
+
+ PyObject *result = PyListWriter_Finish(writer);
+ if (result == NULL) {
+ return NULL;
+ }
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
static PyMethodDef TestMethods[] = {
{"set_errno", set_errno, METH_VARARGS},
{"test_config", test_config, METH_NOARGS},
@@ -2988,6 +3167,11 @@ static PyMethodDef TestMethods[] = {
#ifdef HAVE_SYSCTLBYNAME
{"uptime_bsd", uptime_bsd, METH_NOARGS},
#endif
+ {"bench_setitem_macro", bench_setitem_macro, METH_VARARGS},
+ {"bench_setitem_func", bench_setitem_func, METH_VARARGS},
+ {"bench_writer_append", bench_writer_append, METH_VARARGS},
+ {"bench_writer_extend", bench_writer_extend, METH_VARARGS},
+ {"bench_writer_extend_array", bench_writer_extend_array, METH_VARARGS},
{NULL, NULL} /* sentinel */
};
|
|
+1 on the principle, this looks like a very useful addition. |
|
I implemented wrappers in PyO3 for Victor's API and ran a new round of benchmarks with an implementation using I have two ideas to improve Victor's initial proposal.
Here is a patch that removes the zero-initialization from this PR: Detailsdiff --git a/Objects/listobject.c b/Objects/listobject.c
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -4415,12 +4415,29 @@
return NULL;
}
- PyObject *list = PyList_New(size);
+ PyObject *list = PyList_New(0);
if (list == NULL) {
return NULL;
}
- Py_SET_SIZE(list, 0);
_PyObject_GC_UNTRACK(list);
+
+ // Keep the length at zero until items have been initialized.
+ if (size > 0) {
+#ifdef Py_GIL_DISABLED
+ _PyListArray *array = list_allocate_array(size);
+ PyObject **items = array != NULL ? array->ob_item : NULL;
+#else
+ PyObject **items = PyMem_New(PyObject *, size);
+#endif
+ if (items == NULL) {
+ Py_DECREF(list);
+ PyErr_NoMemory();
+ return NULL;
+ }
+ PyListObject *list_obj = _PyList_CAST(list);
+ list_obj->ob_item = items;
+ list_obj->allocated = size;
+ }
PyListWriter *writer = (PyListWriter*)list;
assert(listwriter_check_consistency(writer));
Here is a plot that illustrates the timings for all three approaches, along with the iterator approach, 
Note that this is a somewhat different workload than Victor's test above. PyO3 streams input items with a size hint, so a batched approach is most natural. Victor passes an existing array directly, and his I lean towards leaving in |
In release mode, no longer initialize items to NULL for better performance. The caller is responsible to initialize all items before calling PyListWriter_Finish().
Oh sure, there is no need to initialize items to NULL since the list is not tracked by the GC. I implemented this optimization.
I'm not sure how other C API Working Group members feel about functions stealing references. But ok, I added the function! |
|
I mark the PR as ready for review. |
|
@encukou: So what do you think of this APi? |
|
How important is this?
If there's a function that doesn't steal, it's fine to add an additional one with a Personally I'd prefer separating suffixes like this with an underscore ( |
It's quite important. PyListWriter implementation relies on the fact that the internal list objects cannot be seen by Python or other C code. The incomplete list object cannot be tracked by the GC since its items are uninitialized memory. If we remove this optimization, so initialize list items to NULL again, well, you might just use PyList_New() in this case (but suffer from the abi3t performance tax, which is the issue there).
We don't do that in other places (like in
I would prefer to add it to have an API easier and more convenient to use. For example, that's why we have By the way, I vaguely recall that #define _PyObject_CallArg1(func, arg) \
_PyObject_FastCall((func), (PyObject **)&(arg), 1)The commit message says:
|
|
I created capi-workgroup/decisions#118 for the C API Working Group. |
|
I re-ran benchmarks differently to produce comparison tables. I ran benchmarks on Free Threading with CPU isolation. In short, Also, in average, Comparison to the PyList_SET_ITEM() macro:
Comparison to the PyList_SetItem() function (abi3t):
PyListWriter_AppendSteal() compared to PyListWriter_Append():
Benchmark hidden because not significant (1): size=100 It seems like PyListWriter_AppendSteal() is mostly slower than PyListWriter_Append() which is surprising since PyListWriter_Append() calls PyListWriter_AppendSteal()! Well, I would say that the performance is basically the same, and the difference is just noise in the benchmark. PyListWriter_ExtendArraySteal() compared to PyListWriter_ExtendArray():
PyListWriter_ExtendArray() and PyListWriter_ExtendArraySteal() compared to the PyList_SET_ITEM() macro:
Files:
|
PyList_SetItemis ~2.7x slower per item on the free-threaded build #158660