From fb7e72b16b59dc057a3ac106ba8f3e1de57132c5 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 11 Apr 2023 10:55:18 +0300 Subject: [PATCH 01/12] WIP --- Include/internal/pycore_interp.h | 5 +- Makefile.pre.in | 1 + Objects/genericaliasobject.c | 134 +++++++++++++++++++++++++------ Python/pylifecycle.c | 7 ++ 4 files changed, 122 insertions(+), 25 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index d64a68cd2da54e..6f6181ff4aa242 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -28,7 +28,7 @@ extern "C" { #include "pycore_global_objects.h" // struct _Py_interp_static_objects #include "pycore_object_state.h" // struct _py_object_state #include "pycore_tuple.h" // struct _Py_tuple_state -#include "pycore_typeobject.h" // struct type_cache +#include "pycore_typeobject.h" // struct types_state #include "pycore_unicodeobject.h" // struct _Py_unicode_state #include "pycore_warnings.h" // struct _warnings_runtime_state @@ -136,6 +136,9 @@ struct _is { created and then deleted again. */ PySliceObject *slice_cache; + // Dict with existing generic alias objects: + PyObject* genericalias_cache; + struct _Py_tuple_state tuple; struct _Py_list_state list; struct _Py_dict_state dict_state; diff --git a/Makefile.pre.in b/Makefile.pre.in index fefa5d4e8147bf..042036b133a0ab 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1685,6 +1685,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_frame.h \ $(srcdir)/Include/internal/pycore_function.h \ $(srcdir)/Include/internal/pycore_genobject.h \ + $(srcdir)/Include/internal/pycore_genericalias.h \ $(srcdir)/Include/internal/pycore_getopt.h \ $(srcdir)/Include/internal/pycore_gil.h \ $(srcdir)/Include/internal/pycore_global_objects.h \ diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 888cb16edd1b46..fd04121d63ec5f 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -3,6 +3,8 @@ #include "Python.h" #include "pycore_object.h" #include "pycore_unionobject.h" // _Py_union_type_or, _PyGenericAlias_Check +#include "pycore_genericalias.h" // _PyGenericAlias_Init, _PyGenericAlias_Fini +#include "pycore_initconfig.h" // _PyStatus_OK() #include "structmember.h" // PyMemberDef #include @@ -781,23 +783,66 @@ static PyGetSetDef ga_properties[] = { {0} }; -/* A helper function to create GenericAlias' args tuple and set its attributes. - * Returns 1 on success, 0 on failure. - */ -static inline int -setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { - if (!PyTuple_Check(args)) { - args = PyTuple_Pack(1, args); - if (args == NULL) { - return 0; - } +/* Cache helpers. */ + +static PyObject* +get_cache_key(PyObject *origin, PyObject *args, bool starred) { + PyObject *key = PyTuple_Pack(3, origin, args, PyBool_FromLong(starred)); + if (key == NULL) { + return NULL; } - else { - Py_INCREF(args); + return key; +} + +static PyObject* +get_cache_entry(PyInterpreterState *interp, PyObject *key) { + Py_hash_t hash; + hash = PyObject_Hash(key); + if (hash == -1) { + return NULL; + } + + PyObject* res = _PyDict_GetItem_KnownHash(interp->genericalias_cache, + key, hash); + if (!res || PyErr_Occurred()) { + PyErr_Clear(); + return NULL; + } + + Py_DECREF(key); + return Py_NewRef(res); +} + +static int +try_set_cache_entry(PyInterpreterState *interp, + PyObject *key, gaobject *alias) { + Py_hash_t hash; + hash = PyObject_Hash(key); + if (hash == -1) { + return -1; } + int res = _PyDict_SetItem_KnownHash(interp->genericalias_cache, + key, Py_NewRef((PyObject *)alias), hash); + + Py_DECREF(key); + return res; +} + +/* A helper function to create GenericAlias' args tuple. */ +static PyObject* +normalize_args(PyObject *args) { + if (!PyTuple_Check(args)) { + return PyTuple_Pack(1, args); + } + + return args; +} + +static inline void +setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { alias->origin = Py_NewRef(origin); - alias->args = args; + alias->args = Py_NewRef(args); alias->parameters = NULL; alias->weakreflist = NULL; @@ -807,8 +852,6 @@ setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { else { alias->vectorcall = NULL; } - - return 1; } static PyObject * @@ -821,15 +864,15 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; } PyObject *origin = PyTuple_GET_ITEM(args, 0); - PyObject *arguments = PyTuple_GET_ITEM(args, 1); - gaobject *self = (gaobject *)type->tp_alloc(type, 0); - if (self == NULL) { + PyObject *arguments = normalize_args(PyTuple_GET_ITEM(args, 1)); + if (arguments == NULL) { return NULL; } - if (!setup_ga(self, origin, arguments)) { - Py_DECREF(self); + gaobject *self = (gaobject *)type->tp_alloc(type, 0); + if (self == NULL) { return NULL; } + setup_ga(self, origin, arguments); return (PyObject *)self; } @@ -923,7 +966,6 @@ ga_iter(PyObject *self) { // TODO: // - argument clinic? -// - cache? PyTypeObject Py_GenericAliasType = { PyVarObject_HEAD_INIT(&PyType_Type, 0) .tp_name = "types.GenericAlias", @@ -953,14 +995,58 @@ PyTypeObject Py_GenericAliasType = { PyObject * Py_GenericAlias(PyObject *origin, PyObject *args) { + args = normalize_args(args); + if (args == NULL) { + return NULL; + } + + PyInterpreterState *interp = _PyInterpreterState_GET(); + PyObject *key = get_cache_key(origin, args, false); + if (key != NULL) { + PyObject *cached = get_cache_entry(interp, key); + if (cached != NULL) { + return cached; + } + } + gaobject *alias = (gaobject*) PyType_GenericAlloc( (PyTypeObject *)&Py_GenericAliasType, 0); if (alias == NULL) { return NULL; } - if (!setup_ga(alias, origin, args)) { - Py_DECREF(alias); - return NULL; + setup_ga(alias, origin, args); + if (key != NULL) { + // Try setting cache, but if it fails - just go on: + (void)try_set_cache_entry(interp, key, alias); } + return (PyObject *)alias; } + +/* runtime lifecycle */ + +PyStatus +_PyGenericAlias_Init(PyInterpreterState *interp) { + PyObject *cache; + + if (PyType_Ready(&Py_GenericAliasType) < 0) { + return _PyStatus_ERR("Can't initialize 'types.GenericAlias' type"); + } + + cache = PyDict_New(); + if (cache == NULL) { + return _PyStatus_ERR("Can't initialize 'types.GenericAlias' cache"); + } + + interp->genericalias_cache = cache; + return _PyStatus_OK(); +}; + +void +_PyGenericAlias_Fini(PyInterpreterState *interp) { + if (interp->genericalias_cache != NULL) { + PyObject *cache = interp->genericalias_cache; + PyObject_GC_Del(cache); + interp->genericalias_cache = NULL; + } +}; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index d6627bc6b7e86b..84bdeeb68b6e23 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -29,6 +29,7 @@ #include "pycore_tuple.h" // _PyTuple_InitTypes() #include "pycore_typeobject.h" // _PyTypes_InitTypes() #include "pycore_unicodeobject.h" // _PyUnicode_InitTypes() +#include "pycore_genericalias.h" // _PyGenericAlias_Init() #include "opcode.h" extern PyStatus _PyIO_InitTypes(PyInterpreterState *interp); @@ -722,6 +723,11 @@ pycore_init_types(PyInterpreterState *interp) if (_PyStatus_EXCEPTION(status)) { return status; } + + status = _PyGenericAlias_Init(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } return _PyStatus_OK(); } @@ -1665,6 +1671,7 @@ flush_std_files(void) static void finalize_interp_types(PyInterpreterState *interp) { + _PyGenericAlias_Fini(interp); _PyUnicode_FiniTypes(interp); _PySys_Fini(interp); _PyExc_Fini(interp); From a96f8527040ca12ddef982f1f9e23cba9160db7e Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 11 Apr 2023 10:55:28 +0300 Subject: [PATCH 02/12] WIP --- Include/internal/pycore_genericalias.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 Include/internal/pycore_genericalias.h diff --git a/Include/internal/pycore_genericalias.h b/Include/internal/pycore_genericalias.h new file mode 100644 index 00000000000000..9c94d0f22dd528 --- /dev/null +++ b/Include/internal/pycore_genericalias.h @@ -0,0 +1,12 @@ +#ifdef __cplusplus +extern "C" { +#endif + +/* runtime lifecycle */ + +extern PyStatus _PyGenericAlias_Init(PyInterpreterState *interp); +extern void _PyGenericAlias_Fini(PyInterpreterState *interp); + +#ifdef __cplusplus +} +#endif From 4cc38ade36125a9f1cd8769ce03f63b6fe4cbc11 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 13 Apr 2023 14:09:26 +0300 Subject: [PATCH 03/12] WIP --- Objects/genericaliasobject.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index fd04121d63ec5f..ae81579e67fccf 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -787,11 +787,7 @@ static PyGetSetDef ga_properties[] = { static PyObject* get_cache_key(PyObject *origin, PyObject *args, bool starred) { - PyObject *key = PyTuple_Pack(3, origin, args, PyBool_FromLong(starred)); - if (key == NULL) { - return NULL; - } - return key; + return PyTuple_Pack(3, origin, args, PyBool_FromLong(starred)); } static PyObject* @@ -822,11 +818,8 @@ try_set_cache_entry(PyInterpreterState *interp, return -1; } - int res = _PyDict_SetItem_KnownHash(interp->genericalias_cache, - key, Py_NewRef((PyObject *)alias), hash); - - Py_DECREF(key); - return res; + return _PyDict_SetItem_KnownHash(interp->genericalias_cache, + key, Py_NewRef((PyObject *)alias), hash); } /* A helper function to create GenericAlias' args tuple. */ @@ -1002,11 +995,14 @@ Py_GenericAlias(PyObject *origin, PyObject *args) PyInterpreterState *interp = _PyInterpreterState_GET(); PyObject *key = get_cache_key(origin, args, false); - if (key != NULL) { - PyObject *cached = get_cache_entry(interp, key); - if (cached != NULL) { - return cached; - } + if (key == NULL) { + return NULL; + } + + PyObject *cached = get_cache_entry(interp, key); + if (cached != NULL) { + Py_DECREF(key); + return cached; } gaobject *alias = (gaobject*) PyType_GenericAlloc( @@ -1018,6 +1014,7 @@ Py_GenericAlias(PyObject *origin, PyObject *args) if (key != NULL) { // Try setting cache, but if it fails - just go on: (void)try_set_cache_entry(interp, key, alias); + Py_DECREF(key); } return (PyObject *)alias; From 8c51f5024a22773c03de0ae34cbdcd47c5788cfe Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 14 Apr 2023 17:43:33 +0300 Subject: [PATCH 04/12] First prototype --- Include/internal/pycore_genericalias.h | 3 + Lib/test/test_genericalias.py | 33 ++++- Objects/genericaliasobject.c | 176 +++++++++++++++---------- PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 + 5 files changed, 142 insertions(+), 74 deletions(-) diff --git a/Include/internal/pycore_genericalias.h b/Include/internal/pycore_genericalias.h index 9c94d0f22dd528..3b83a1ffc657da 100644 --- a/Include/internal/pycore_genericalias.h +++ b/Include/internal/pycore_genericalias.h @@ -1,3 +1,5 @@ +#ifndef Py_INTERNAL_GENERICALIAS_H +#define Py_INTERNAL_GENERICALIAS_H #ifdef __cplusplus extern "C" { #endif @@ -10,3 +12,4 @@ extern void _PyGenericAlias_Fini(PyInterpreterState *interp); #ifdef __cplusplus } #endif +#endif /* !Py_INTERNAL_GENERICALIAS_H */ diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 9b59d1e3e0aad2..5929632667f393 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -56,6 +56,8 @@ import typing from typing import Unpack +from test import support + from typing import TypeVar T = TypeVar('T') K = TypeVar('K') @@ -94,8 +96,11 @@ class BaseTest(unittest.TestCase): """Test basics.""" - generic_types = [type, tuple, list, dict, set, frozenset, enumerate, - defaultdict, deque, + c_generic_types = [ + tuple, list, dict, set, frozenset, enumerate, + defaultdict, deque, + ] + generic_types = [*c_generic_types, SequenceMatcher, dircmp, FileInput, @@ -173,6 +178,30 @@ def default(): else: self.assertEqual(alias(iter((1, 2, 3))), t((1, 2, 3))) + @support.cpython_only + def test_c_genericaliases_are_cached(self): + for t in self.c_generic_types: + if t is None: + continue + tname = t.__name__ + with self.subTest(f"Testing {tname}"): + self.assertIs(t[int], t[int]) + + @support.cpython_only + def test_generic_alias_unpacks_are_cached(self): + self.assertIs((*tuple[int, str],)[0], (*tuple[int, str],)[0]) + self.assertIs((*tuple[T, ...],)[0], (*tuple[T, ...],)[0]) + self.assertIsNot((*tuple[int, str],)[0], tuple[int, str]) + + @support.cpython_only + def test_genericalias_constructor_is_no_cached(self): + for t in self.generic_types: + if t is None: + continue + tname = t.__name__ + with self.subTest(f"Testing {tname}"): + self.assertIsNot(GenericAlias(t, [int]), GenericAlias(t, [int])) + def test_unbound_methods(self): t = list[int] a = t() diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index ae81579e67fccf..dbf98c151b750e 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -25,6 +25,14 @@ typedef struct { PyObject *obj; /* Set to NULL when iterator is exhausted */ } gaiterobject; +// Forward references: +// ----- + +PyObject * +_Py_GenericAliasStarred(PyObject *origin, PyObject *args, bool starred); + +// ----- + static void ga_dealloc(PyObject *self) { @@ -529,8 +537,8 @@ ga_getitem(PyObject *self, PyObject *item) return NULL; } - PyObject *res = Py_GenericAlias(alias->origin, newargs); - ((gaobject *)res)->starred = alias->starred; + PyObject *res = _Py_GenericAliasStarred(alias->origin, newargs, + alias->starred); Py_DECREF(newargs); return res; @@ -783,59 +791,36 @@ static PyGetSetDef ga_properties[] = { {0} }; -/* Cache helpers. */ - -static PyObject* -get_cache_key(PyObject *origin, PyObject *args, bool starred) { - return PyTuple_Pack(3, origin, args, PyBool_FromLong(starred)); -} - -static PyObject* -get_cache_entry(PyInterpreterState *interp, PyObject *key) { - Py_hash_t hash; - hash = PyObject_Hash(key); - if (hash == -1) { - return NULL; - } - - PyObject* res = _PyDict_GetItem_KnownHash(interp->genericalias_cache, - key, hash); - if (!res || PyErr_Occurred()) { - PyErr_Clear(); - return NULL; - } - - Py_DECREF(key); - return Py_NewRef(res); -} +/* +A helper function to create GenericAlias' args tuple. +Returns -1 on error, +Returns 0 on no action, +Returns 1 if args were modified. +*/ static int -try_set_cache_entry(PyInterpreterState *interp, - PyObject *key, gaobject *alias) { - Py_hash_t hash; - hash = PyObject_Hash(key); - if (hash == -1) { - return -1; - } +normalize_args(PyObject **args) { + PyObject *normalized; - return _PyDict_SetItem_KnownHash(interp->genericalias_cache, - key, Py_NewRef((PyObject *)alias), hash); -} + if (!PyTuple_Check(*args)) { + normalized = PyTuple_Pack(1, *args); + if (normalized == NULL) { + return -1; + } -/* A helper function to create GenericAlias' args tuple. */ -static PyObject* -normalize_args(PyObject *args) { - if (!PyTuple_Check(args)) { - return PyTuple_Pack(1, args); + *args = normalized; + return 1; } - return args; + Py_INCREF(*args); + return 0; } static inline void -setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { +setup_ga(gaobject *alias, PyObject *origin, PyObject *args, bool starred) { + // Make sure to call `normalize_args` before calling `setup_ga`. alias->origin = Py_NewRef(origin); - alias->args = Py_NewRef(args); + alias->args = args; alias->parameters = NULL; alias->weakreflist = NULL; @@ -845,6 +830,8 @@ setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { else { alias->vectorcall = NULL; } + + alias->starred = starred; } static PyObject * @@ -857,15 +844,16 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; } PyObject *origin = PyTuple_GET_ITEM(args, 0); - PyObject *arguments = normalize_args(PyTuple_GET_ITEM(args, 1)); - if (arguments == NULL) { + PyObject *arguments = PyTuple_GET_ITEM(args, 1); + int arg_res = normalize_args(&arguments); + if (arg_res < 0) { return NULL; } gaobject *self = (gaobject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; } - setup_ga(self, origin, arguments); + setup_ga(self, origin, arguments, false); return (PyObject *)self; } @@ -880,11 +868,11 @@ ga_iternext(gaiterobject *gi) { return NULL; } gaobject *alias = (gaobject *)gi->obj; - PyObject *starred_alias = Py_GenericAlias(alias->origin, alias->args); + PyObject *starred_alias = _Py_GenericAliasStarred(alias->origin, + alias->args, true); if (starred_alias == NULL) { return NULL; } - ((gaobject *)starred_alias)->starred = true; Py_SETREF(gi->obj, NULL); return starred_alias; } @@ -985,42 +973,86 @@ PyTypeObject Py_GenericAliasType = { .tp_vectorcall_offset = offsetof(gaobject, vectorcall), }; -PyObject * -Py_GenericAlias(PyObject *origin, PyObject *args) -{ - args = normalize_args(args); - if (args == NULL) { +static PyObject * +_Py_GenericAlias_impl_nocache(PyObject *origin, PyObject *args, bool starred) { + // args must be normalized at this point. + gaobject *alias = (gaobject*) PyType_GenericAlloc( + (PyTypeObject *)&Py_GenericAliasType, 0); + if (alias == NULL) { return NULL; } + setup_ga(alias, origin, args, starred); + return (PyObject *)alias; +} +static PyObject * +_Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { + Py_hash_t hash; + PyObject *star = NULL; + PyObject *key = NULL; + PyObject *result = NULL; PyInterpreterState *interp = _PyInterpreterState_GET(); - PyObject *key = get_cache_key(origin, args, false); - if (key == NULL) { + + int arg_res = normalize_args(&args); + if (arg_res < 0) { return NULL; } - PyObject *cached = get_cache_entry(interp, key); - if (cached != NULL) { - Py_DECREF(key); - return cached; + star = PyBool_FromLong(starred); + if (star == NULL) { + goto error; + } + key = PyTuple_Pack(3, origin, args, star); + if (key == NULL) { + goto error; } - gaobject *alias = (gaobject*) PyType_GenericAlloc( - (PyTypeObject *)&Py_GenericAliasType, 0); - if (alias == NULL) { - return NULL; + hash = PyObject_Hash(key); + if (hash == -1) { + goto error; } - setup_ga(alias, origin, args); - if (key != NULL) { - // Try setting cache, but if it fails - just go on: - (void)try_set_cache_entry(interp, key, alias); - Py_DECREF(key); + + result = _PyDict_GetItem_KnownHash(interp->genericalias_cache, key, hash); + if (result) { + Py_INCREF(result); + goto done; + } + if (PyErr_Occurred()) { + goto error; } - return (PyObject *)alias; + result = _Py_GenericAlias_impl_nocache(origin, args, starred); + if (result == NULL) { + goto error; + } + + if (_PyDict_SetItem_KnownHash(interp->genericalias_cache, + key, result, hash) < 0) { + goto error; + } else { + goto done; + } + +error: + Py_XDECREF(result); +done: + Py_XDECREF(key); + Py_XDECREF(star); + return result; +} + +PyObject * +Py_GenericAlias(PyObject *origin, PyObject *args) { + return _Py_GenericAlias_impl(origin, args, false); +} + +// TODO: make this API public? +PyObject * +_Py_GenericAliasStarred(PyObject *origin, PyObject *args, bool starred) { + return _Py_GenericAlias_impl(origin, args, starred); } -/* runtime lifecycle */ +/* Runtime lifecycle */ PyStatus _PyGenericAlias_Init(PyInterpreterState *interp) { diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 8aafcb786a6064..6e434b551d6064 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -224,6 +224,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 07476f30833372..ed37f49c1904dd 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -573,6 +573,9 @@ Include\internal + + Include\internal + Include\internal From 32a8d5e572bbc9ed1983438d680ca42188a30d0a Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 16 Apr 2023 14:14:26 +0300 Subject: [PATCH 05/12] Clear global cache, support unhashable type args --- Include/internal/pycore_genericalias.h | 4 +++ Lib/test/libregrtest/utils.py | 7 +++++ Lib/test/test_genericalias.py | 23 +++++++++++++--- Modules/Setup.stdlib.in | 2 +- Modules/_testcapi/genericalias.c | 36 ++++++++++++++++++++++++++ Modules/_testcapi/parts.h | 1 + Modules/_testcapimodule.c | 5 +++- Objects/genericaliasobject.c | 32 ++++++++++++++--------- 8 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 Modules/_testcapi/genericalias.c diff --git a/Include/internal/pycore_genericalias.h b/Include/internal/pycore_genericalias.h index 3b83a1ffc657da..ead45465dca668 100644 --- a/Include/internal/pycore_genericalias.h +++ b/Include/internal/pycore_genericalias.h @@ -4,6 +4,10 @@ extern "C" { #endif +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + /* runtime lifecycle */ extern PyStatus _PyGenericAlias_Init(PyInterpreterState *interp); diff --git a/Lib/test/libregrtest/utils.py b/Lib/test/libregrtest/utils.py index fb13fa0e243ba7..b773c4b7945924 100644 --- a/Lib/test/libregrtest/utils.py +++ b/Lib/test/libregrtest/utils.py @@ -210,6 +210,13 @@ def clear_caches(): else: fractions._hash_algorithm.cache_clear() + try: + _testcapi = sys.modules['_testcapi'] + except KeyError: + pass + else: + _testcapi.genericalias_cache_clear() + def get_build_info(): # Get most important configure and build options as a list of strings. diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 5929632667f393..c6a32717365f33 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -181,18 +181,32 @@ def default(): @support.cpython_only def test_c_genericaliases_are_cached(self): for t in self.c_generic_types: - if t is None: - continue - tname = t.__name__ - with self.subTest(f"Testing {tname}"): + with self.subTest(t=t): self.assertIs(t[int], t[int]) + self.assertEqual(t[int], t[int]) + self.assertIsNot(t[int], t[str]) + + @support.cpython_only + def test_c_genericaliases_uncachable_still_work(self): + for t in self.c_generic_types: + with self.subTest(t=t): + # Cache does not work for these args, + # but no error is present + self.assertIsNot(t[{}], t[{}]) + self.assertEqual(t[{}], t[{}]) @support.cpython_only def test_generic_alias_unpacks_are_cached(self): self.assertIs((*tuple[int, str],)[0], (*tuple[int, str],)[0]) + self.assertIsNot((*tuple[str, int],)[0], (*tuple[int, str],)[0]) self.assertIs((*tuple[T, ...],)[0], (*tuple[T, ...],)[0]) self.assertIsNot((*tuple[int, str],)[0], tuple[int, str]) + @support.cpython_only + def test_generic_alias_unpacks_uncachable_still_work(self): + self.assertIsNot((*tuple[{}],)[0], (*tuple[{}],)[0]) + self.assertEqual((*tuple[{}],)[0], (*tuple[{}],)[0]) + @support.cpython_only def test_genericalias_constructor_is_no_cached(self): for t in self.generic_types: @@ -224,6 +238,7 @@ def test_class_methods(self): def test_no_chaining(self): t = list[int] + list[int] with self.assertRaises(TypeError): t[int] diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index fe1b9f8f5380c1..36d800fc127b24 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -169,7 +169,7 @@ @MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c -@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c +@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c _testcapi/genericalias.c @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c # Some testing modules MUST be built as shared libraries. diff --git a/Modules/_testcapi/genericalias.c b/Modules/_testcapi/genericalias.c new file mode 100644 index 00000000000000..4067d5c60a65ba --- /dev/null +++ b/Modules/_testcapi/genericalias.c @@ -0,0 +1,36 @@ +#include "parts.h" + +#define Py_BUILD_CORE +// Needed to include both +// `Include/internal/pycore_gc.h` and +// `Include/cpython/objimpl.h` +#undef _PyGC_FINALIZED +#include "pycore_interp.h" // PyInterpreterState + +static PyObject * +genericalias_cache_clear(PyObject *self, PyObject *Py_UNUSED(args)) +{ + PyThreadState *tstate = PyThreadState_Get(); + PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate); + assert(interp != NULL); + assert(interp->genericalias_cache != NULL); + + PyDict_Clear(interp->genericalias_cache); // needs full PyInterpreterState + + Py_RETURN_NONE; +} + +static PyMethodDef test_methods[] = { + {"genericalias_cache_clear", genericalias_cache_clear, METH_NOARGS}, + {NULL}, +}; + +int +_PyTestCapi_Init_GenericAlias(PyObject *mod) +{ + if (PyModule_AddFunctions(mod, test_methods) < 0) { + return -1; + } + + return 0; +} diff --git a/Modules/_testcapi/parts.h b/Modules/_testcapi/parts.h index 60ec81dad2ba9e..17e734910b32a5 100644 --- a/Modules/_testcapi/parts.h +++ b/Modules/_testcapi/parts.h @@ -39,6 +39,7 @@ int _PyTestCapi_Init_Structmember(PyObject *module); int _PyTestCapi_Init_Exceptions(PyObject *module); int _PyTestCapi_Init_Code(PyObject *module); int _PyTestCapi_Init_PyOS(PyObject *module); +int _PyTestCapi_Init_GenericAlias(PyObject *mod); #ifdef LIMITED_API_AVAILABLE int _PyTestCapi_Init_VectorcallLimited(PyObject *module); diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 557a6d46ed4632..ea9f449db4d3dc 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3343,7 +3343,7 @@ test_gc_visit_objects_basic(PyObject *Py_UNUSED(self), } state.target = obj; state.found = 0; - + PyUnstable_GC_VisitObjects(gc_visit_callback_basic, &state); Py_DECREF(obj); if (!state.found) { @@ -4189,6 +4189,9 @@ PyInit__testcapi(void) if (_PyTestCapi_Init_PyOS(m) < 0) { return NULL; } + if (_PyTestCapi_Init_GenericAlias(m) < 0) { + return NULL; + } #ifndef LIMITED_API_AVAILABLE PyModule_AddObjectRef(m, "LIMITED_API_AVAILABLE", Py_False); diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index dbf98c151b750e..50b096080d2647 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -988,6 +988,7 @@ _Py_GenericAlias_impl_nocache(PyObject *origin, PyObject *args, bool starred) { static PyObject * _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { Py_hash_t hash; + bool unhashable = false; PyObject *star = NULL; PyObject *key = NULL; PyObject *result = NULL; @@ -1009,33 +1010,40 @@ _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { hash = PyObject_Hash(key); if (hash == -1) { - goto error; - } - - result = _PyDict_GetItem_KnownHash(interp->genericalias_cache, key, hash); - if (result) { - Py_INCREF(result); - goto done; - } - if (PyErr_Occurred()) { - goto error; + // `key` contains unhashable objects, stop right here. + // Just return the object itself without setting cache. + PyErr_Clear(); + unhashable = true; + } else { + result = _PyDict_GetItem_KnownHash(interp->genericalias_cache, + key, hash); + if (result) { + Py_INCREF(result); + goto finally; + } + if (PyErr_Occurred()) { + goto error; + } } result = _Py_GenericAlias_impl_nocache(origin, args, starred); if (result == NULL) { goto error; } + if (unhashable) { + goto finally; + } if (_PyDict_SetItem_KnownHash(interp->genericalias_cache, key, result, hash) < 0) { goto error; } else { - goto done; + goto finally; } error: Py_XDECREF(result); -done: +finally: Py_XDECREF(key); Py_XDECREF(star); return result; From 87eb78456083a20ffab881d487717c9b4ee75990 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 16 Apr 2023 14:19:12 +0300 Subject: [PATCH 06/12] Typo --- Lib/test/test_genericalias.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index c6a32717365f33..38026748deee5c 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -238,7 +238,6 @@ def test_class_methods(self): def test_no_chaining(self): t = list[int] - list[int] with self.assertRaises(TypeError): t[int] From 3b92438c8341f8c0ca58056d09eeb79eb0d6085d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 16 Apr 2023 14:20:24 +0300 Subject: [PATCH 07/12] Add more metadata --- PCbuild/_testcapi.vcxproj | 1 + PCbuild/_testcapi.vcxproj.filters | 3 +++ 2 files changed, 4 insertions(+) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 439cd687fda61d..84c4ac9c6d7f67 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -110,6 +110,7 @@ + diff --git a/PCbuild/_testcapi.vcxproj.filters b/PCbuild/_testcapi.vcxproj.filters index 0e42e4982c21ff..d0889ff3849df8 100644 --- a/PCbuild/_testcapi.vcxproj.filters +++ b/PCbuild/_testcapi.vcxproj.filters @@ -60,6 +60,9 @@ Source Files + + Source Files + From 2d73969be181d82046edeb313f1f30a06365c1a3 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 19 Apr 2023 02:10:30 +0300 Subject: [PATCH 08/12] Address review, remove leak --- Objects/genericaliasobject.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 50b096080d2647..e1a52f6cbb3ebd 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -1016,9 +1016,10 @@ _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { unhashable = true; } else { result = _PyDict_GetItem_KnownHash(interp->genericalias_cache, - key, hash); + key, hash); if (result) { Py_INCREF(result); + Py_DECREF(args); goto finally; } if (PyErr_Occurred()) { @@ -1043,6 +1044,9 @@ _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { error: Py_XDECREF(result); + if (arg_res == 1) { + Py_DECREF(args); + } finally: Py_XDECREF(key); Py_XDECREF(star); From 1771aba3303c16dd957aa01ceecec36843eb2dcd Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 19 Apr 2023 03:05:54 +0300 Subject: [PATCH 09/12] Address review, remove leak --- Objects/genericaliasobject.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index e1a52f6cbb3ebd..5e85212164e07c 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -851,6 +851,7 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } gaobject *self = (gaobject *)type->tp_alloc(type, 0); if (self == NULL) { + Py_DECREF(arguments); return NULL; } setup_ga(self, origin, arguments, false); @@ -1037,6 +1038,7 @@ _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { if (_PyDict_SetItem_KnownHash(interp->genericalias_cache, key, result, hash) < 0) { + Py_DECREF(origin); goto error; } else { goto finally; @@ -1044,9 +1046,7 @@ _Py_GenericAlias_impl(PyObject *origin, PyObject *args, bool starred) { error: Py_XDECREF(result); - if (arg_res == 1) { - Py_DECREF(args); - } + Py_DECREF(args); finally: Py_XDECREF(key); Py_XDECREF(star); From 15fbd8bdb9832fa59c5d8f7275afdbb339e46cff Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 20 Apr 2023 16:00:38 +0300 Subject: [PATCH 10/12] Address review by @sunmy2019 --- Objects/genericaliasobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 5e85212164e07c..7e588e6391340d 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -1087,7 +1087,7 @@ void _PyGenericAlias_Fini(PyInterpreterState *interp) { if (interp->genericalias_cache != NULL) { PyObject *cache = interp->genericalias_cache; - PyObject_GC_Del(cache); + Py_DECREF(cache); interp->genericalias_cache = NULL; } }; From 97062c0209284405c2d77d3374bd6036e52b44f7 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 20 Apr 2023 16:40:57 +0300 Subject: [PATCH 11/12] Address review by @sunmy2019 --- Lib/test/test_types.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index af095632a36fcb..098c38143574c7 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -985,6 +985,11 @@ def __module__(self): def test_or_type_operator_reference_cycle(self): if not hasattr(sys, 'gettotalrefcount'): self.skipTest('Cannot get total reference count.') + try: + import _testcapi + except ImportError: + self.skipTest('Cannot clear types.GenericAlias cache.') + gc.collect() before = sys.gettotalrefcount() for _ in range(30): @@ -993,6 +998,7 @@ def test_or_type_operator_reference_cycle(self): T.blah = U del T del U + _testcapi.genericalias_cache_clear() gc.collect() leeway = 15 self.assertLessEqual(sys.gettotalrefcount() - before, leeway, From bcf99a627401ff68e72084deab3725f3c8339e3f Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 21 Apr 2023 16:50:57 +0300 Subject: [PATCH 12/12] Address review --- Lib/test/test_genericalias.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 38026748deee5c..a393a4c5319253 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -216,6 +216,12 @@ def test_genericalias_constructor_is_no_cached(self): with self.subTest(f"Testing {tname}"): self.assertIsNot(GenericAlias(t, [int]), GenericAlias(t, [int])) + @support.cpython_only + def test_c_union_arg_order(self): + self.assertIsNot(int | str, str | int) + self.assertIsNot(int | str, int | None) + self.assertIsNot(int | str, int | str | None) + def test_unbound_methods(self): t = list[int] a = t()