8000 bpo-37596: compile to reproducible frozen sets by FFY00 · Pull Request #27769 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

bpo-37596: compile to reproducible frozen sets #27769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Include/setobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,19 @@ typedef struct {
#define PySet_GET_SIZE(so) (assert(PyAnySet_Check(so)),(((PySetObject *)(so))->used))

PyAPI_DATA(PyObject *) _PySet_Dummy;
PyAPI_DATA(PyTypeObject) _PyReproducibleFrozenSet_Type;

PyAPI_FUNC(PyObject *) _PyReproducibleFrozenSet_New(PyObject *);

PyAPI_FUNC(int) _PySet_NextEntry(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash);
PyAPI_FUNC(int) _PySet_Update(PyObject *set, PyObject *iterable);

#define _PyReproducibleFrozenSet_CheckExact(ob) \
Py_IS_TYPE(ob, &_PyReproducibleFrozenSet_Type)
#define _PyReproducibleFrozenSet_Check(ob) \
(Py_IS_TYPE(ob, &_PyReproducibleFrozenSet_Type) || \
PyType_IsSubtype(Py_TYPE(ob), &_PyReproducibleFrozenSet_Type))

Comment on lines +70 to +82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions should not be public. I think we really should not expose this type to be used or checked from outside CPython

#endif /* Section excluded by Py_LIMITED_API */

PyAPI_DATA(PyTypeObject) PySet_Type;
Expand Down
65 changes: 63 additions & 2 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash)

static int set_table_resize(PySetObject *, Py_ssize_t);

PyTypeObject PyReproducibleFrozenSet_Type;

static int
set_add_entry(PySetObject *so, PyObject *key, Py_hash_t hash)
{
Expand All @@ -118,9 +120,15 @@ set_add_entry(PySetObject *so, PyObject *key, Py_hash_t hash)
restart:

mask = so->mask;
i = (size_t)hash & mask;
freeslot = NULL;
perturb = hash;

if (_PyReproducibleFrozenSet_CheckExact(so)) {
i = 0;
perturb = 0;
} else {
i = (size_t)hash & mask;
perturb = hash;
}
Comment on lines +125 to +131
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this impacts the performance of normal set operations, and given that is a basic type of the interpreter I am not very convinced about it. This is one of the reasons I said:

and a new type, which I am not very fond of)

Because a new type will require a new set of independent functions on the required path to not impact the regular set and frozenset and that is much more code than what I am confortable with.

On the other hand, others may have other views on this, so I am happy to be convinced otherwise :)


while (1) {
entry = &so->table[i];
Expand Down Expand Up @@ -2233,6 +2241,53 @@ PyTypeObject PyFrozenSet_Type = {
};


PyTypeObject _PyReproducibleFrozenSet_Type = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For new types, please use C99 named fields like

static PyTypeObject CustomType = {
PyVarObject_HEAD_INIT(NULL, 0)
.tp_name = "custom2.Custom",
.tp_doc = "Custom objects",
.tp_basicsize = sizeof(CustomObject),
.tp_itemsize = 0,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_new = Custom_new,
.tp_init = (initproc) Custom_init,
.tp_dealloc = (destructor) Custom_dealloc,
.tp_members = Custom_members,
.tp_methods = Custom_methods,
};

PyVarObject_HEAD_INIT(&PyType_Type, 0)
"reproducible_frozenset", /* tp_name */
sizeof(PySetObject), /* tp_basicsize */
0, /* tp_itemsize */
/* methods */
(destructor)set_dealloc, /* tp_dealloc */
0, /* tp_vectorcall_offset */
0, /* tp_getattr */
0, /* tp_setattr */
0, /* tp_as_async */
(reprfunc)set_repr, /* tp_repr */
&frozenset_as_number, /* tp_as_number */
&set_as_sequence, /* tp_as_sequence */
0, /* tp_as_mapping */
frozenset_hash, /* tp_hash */
0, /* tp_call */
0, /* tp_str */
PyObject_GenericGetAttr, /* tp_getattro */
0, /* tp_setattro */
0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_BASETYPE |
_Py_TPFLAGS_MATCH_SELF, /* tp_flags */
frozenset_doc, /* tp_doc */
(traverseproc)set_traverse, /* tp_traverse */
(inquiry)set_clear_internal, /* tp_clear */
(richcmpfunc)set_richcompare, /* tp_richcompare */
offsetof(PySetObject, weakreflist), /* tp_weaklistoffset */
(getiterfunc)set_iter, /* tp_iter */
0, 8000 /* tp_iternext */
0, /* tp_methods */
0, /* tp_members */
0, /* tp_getset */
&PyFrozenSet_Type, /* tp_base */
0, /* tp_dict */
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
0, /* tp_init */
PyType_GenericAlloc, /* tp_alloc */
frozenset_new, /* tp_new */
PyObject_GC_Del, /* tp_free */
.tp_vectorcall = frozenset_vectorcall,
};


/***** C API functions *************************************************/

PyObject *
Expand All @@ -2247,6 +2302,12 @@ PyFrozenSet_New(PyObject *iterable)
return make_new_set(&PyFrozenSet_Type, iterable);
}

PyObject *
_PyReproducibleFrozenSet_New(PyObject *iterable)
{
return make_new_set(&_PyReproducibleFrozenSet_Type, iterable);
}

Py_ssize_t
PySet_Size(PyObject *anyset)
{
Expand Down
4 changes: 2 additions & 2 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ merge_consts_recursive(struct compiler *c, PyObject *o)

// Instead of rewriting o, we create new frozenset and embed in the
// key tuple. Caller should get merged frozenset from the key tuple.
PyObject *new = PyFrozenSet_New(tuple);
PyObject *new = _PyReproducibleFrozenSet_New(tuple);
Py_DECREF(tuple);
if (new == NULL) {
Py_DECREF(key);
Expand Down Expand Up @@ -3910,7 +3910,7 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed,
ADDOP_LOAD_CONST_NEW(c, folded);
} else {
if (add == SET_ADD) {
Py_SETREF(folded, PyFrozenSet_New(folded));
Py_SETREF(folded, _PyReproducibleFrozenSet_New(folded));
if (folded == NULL) {
return 0;
}
Expand Down
5 changes: 3 additions & 2 deletions Python/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,12 +492,13 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
}
w_object((PyObject *)NULL, p);
}
else if (PyAnySet_CheckExact(v)) {
else if (PyAnySet_CheckExact(v) || _PyReproducibleFrozenSet_CheckExact(v)) {
PyObject *value;
Py_ssize_t pos = 0;
Py_hash_t hash;

if (PyFrozenSet_CheckExact(v))
if (PyFrozenSet_CheckExact(v) ||
_PyReproducibleFrozenSet_CheckExact(v))
W_TYPE(TYPE_FROZENSET, p);
else
W_TYPE(TYPE_SET, p);
Expand Down
0