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 */
};
|
| Extend the list writer with the contents of *items*. | ||
|
|
||
| On success, return ``0``. | ||
| On error, set an exception and return ``-1``. |
There was a problem hiding this comment.
Should say what happens to the refcounts of items on error (also on success: does it steal references?).
There was a problem hiding this comment.
It behaves as PyTuple_FromArray(): new references are created. I will reuse PyTuple_FromArray() phrasing: "copy references from array to the new tuple".
|
+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 ( |
PyList_SetItemis ~2.7x slower per item on the free-threaded build #158660