Repository navigation
Conversation
Use the new API in: * Modules/_abc.c * Modules/_asynciomodule.c * Modules/_hashopenssl.c * Modules/_ssl.c
Documentation build overview
41 files changed ·
|
vstinner
changed the title
PyFrozenSetWriter C APIPyFrozenSetWriter C API
|
Updating |
| // Now that the frozenset is fully initialized, it's safe to track it in | ||
| // the GC. Track the frozenset if at least one item is tracked by the GC. | ||
| Py_ssize_t pos = 0; | ||
| setentry *entry; | ||
| while (set_next(frozenset, &pos, &entry)) { | ||
| PyObject *item = entry->key; | ||
| if (_PyObject_IS_GC(item) && _PyObject_GC_IS_TRACKED(item)) { | ||
| _PyObject_GC_TRACK(frozenset); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe we can skip iterating on the items and just always track the frozenset in the GC. Most set functions already do that: always call _PyObject_GC_TRACK() without checking items.
|
cc @corona10 |
|
This C API is just an abstraction on top of existing |
|
The edits to |
There multiple advantages of this new API. The current The internal frozenset is not tracked by the GC until Finish() is called (when the frozenset is fully initialized). So it's not possible to observe a frozenset being modified in Python (which must not be possible, frozenset are immutable, right?). See for example #159730 for recent examples of fixed bugs. Also, I think that's a bad design to have a PySet_Add() function to modify an immutable (frozenset) object. We should avoid such API. So I propose deprecating calling PySet_Add() on a frozenset (replaced with this new Writer API). See issues: |
|
When you're done making the requested changes, leave the comment: |
Add PyFrozenSetWriter_UpdateArray() and PyFrozenSetWriter_UpdateArraySteal() functions. Add tests on PyFrozenSetWriter_AddSteal(). Typoe: rename frozsetwriter_check_consistency() to frozensetwriter_check_consistency().
|
Updates:
|
|
|
||
|
|
||
| void | ||
| PyFrozenSetWriter_Discard(PyFrozenSetWriter *writer) |
There was a problem hiding this comment.
Can you name this something else? Sets already have a discard method with a completely different meaning. No need to create confusion by giving this the same name.
Some cleanups.
|
I ran a benchmark on Linux ( => PyFrozenSetWriter_UpdateArray() is the fastest method to create a frozenset.
I suppose that PyFrozenSetWriter_Update() is faster than PyFrozenSet_New() because PyFrozenSet_New() iterates on items to try to untrack the frozenset in the GC, whereas PyFrozenSetWriter_Finish() calls Note: I didn't test Free Threading since PySet_Add() and PyFrozenSetWriter avoid locking (critical section) anyway. Benchmark script: DetailsPatch adding _testcapi functions: Detailsdiff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 033ee3234cf..b2a881b4b4a 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2889,6 +2889,139 @@ uptime_bsd(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
#endif
+static PyObject*
+bench_frozenset_new(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *iterable;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &iterable, &loops)) {
+ return NULL;
+ }
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyObject *result = PyFrozenSet_New(iterable);
+ if (result == NULL) {
+ return NULL;
+ }
+ Py_DECREF(result);
+ }
+ (void)PyTime_PerfCounterRaw(&t2);
+
+ return PyFloat_FromDouble(PyTime_AsSecondsDouble(t2 - t1));
+}
+
+
+static PyObject*
+bench_writer_add(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *tuple;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &tuple, &loops)) {
+ return NULL;
+ }
+
+ Py_ssize_t len = PyTuple_GET_SIZE(tuple);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyFrozenSetWriter *writer = PyFrozenSetWriter_Create(0);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ for (Py_ssize_t j=0; j < len; j++) {
+ PyObject *item = PyTuple_GET_ITEM(tuple, j);
+ if (PyFrozenSetWriter_Add(writer, item) < 0) {
+ PyFrozenSetWriter_Discard(writer);
+ return NULL;
+ }
+ }
+
+ PyObject *result = PyFrozenSetWriter_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_update(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *iterable;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &iterable, &loops)) {
+ return NULL;
+ }
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyFrozenSetWriter *writer = PyFrozenSetWriter_Create(0);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ if (PyFrozenSetWriter_Update(writer, iterable) < 0) {
+ PyFrozenSetWriter_Discard(writer);
+ return NULL;
+ }
+
+ PyObject *result = PyFrozenSetWriter_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_update_array(PyObject *Py_UNUSED(self), PyObject *args)
+{
+ PyObject *tuple;
+ Py_ssize_t loops;
+ if (!PyArg_ParseTuple(args, "On", &tuple, &loops)) {
+ return NULL;
+ }
+ PyObject **items = &PyTuple_GET_ITEM(tuple, 0);
+ Py_ssize_t len = PyTuple_GET_SIZE(tuple);
+
+ PyTime_t t1, t2;
+ (void)PyTime_PerfCounterRaw(&t1);
+ for (Py_ssize_t i=0; i < loops; i++) {
+ PyFrozenSetWriter *writer = PyFrozenSetWriter_Create(0);
+ if (writer == NULL) {
+ return NULL;
+ }
+
+ if (PyFrozenSetWriter_UpdateArray(writer, items, len) < 0) {
+ PyFrozenSetWriter_Discard(writer);
+ return NULL;
+ }
+
+ PyObject *result = PyFrozenSetWriter_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 +3121,10 @@ static PyMethodDef TestMethods[] = {
#ifdef HAVE_SYSCTLBYNAME
{"uptime_bsd", uptime_bsd, METH_NOARGS},
#endif
+ {"bench_frozenset_new", bench_frozenset_new, METH_VARARGS},
+ {"bench_writer_add", bench_writer_add, METH_VARARGS},
+ {"bench_writer_update", bench_writer_update, METH_VARARGS},
+ {"bench_writer_update_array", bench_writer_update_array, METH_VARARGS},
{NULL, NULL} /* sentinel */
};
|
I was wrong: PyFrozenSet_New(iterable) iterates on items to untrack the frozenset if an item is not tracked by the GC. |
Use the new API in:
PyFrozenWriterC API to create afrozensetobject #160147