Repository navigation
Conversation
Use the new API in: * Modules/_abc.c * Modules/_asynciomodule.c * Modules/_hashopenssl.c * Modules/_ssl.c
Documentation build overview
44 files changed ·
|
vstinner
changed the title
PyFrozenSetWriter C APIPyFrozenSetWriter C API
|
Updating |
|
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.
There was a problem hiding this comment.
Oh, it's unfortunate that PyFrozenSetWriter_Discard() name is close set.discard(), but has a different meaning (discard the temporary frozenset and destroy the writer) :-(
If possible, I would prefer to reuse Create/Finish/Discard names, since they are already used by 3 other "Writer" C API:
For now, I only propose functions to add items to the writer. If someone needs "more complex" use cases, I would suggest creating a mutable set, and then convert it to a frozenset (call PyFrozenNew_New(temp_set)).
If later a discard method is added, it can be called PyFrozenSetWriter_DiscardItem(writer, item).
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. |
|
I updated the PR.
|
Use the new API in:
PyFrozenWriterC API to create afrozensetobject #160147