From 97c3ee547d26599115d438031a2f545fd51dbca8 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 13 Nov 2018 00:44:23 +0000 Subject: [PATCH 01/14] bpo-32492: 2.5x speed up in namedtuple attribute access using C fast path --- Include/descrobject.h | 8 +++ Lib/collections/__init__.py | 15 +++-- Modules/_collectionsmodule.c | 106 +++++++++++++++++++++++++++++++++++ Objects/descrobject.c | 9 --- 4 files changed, 125 insertions(+), 13 deletions(-) diff --git a/Include/descrobject.h b/Include/descrobject.h index 73bbb3fe54e5d0..07844f073c05f8 100644 --- a/Include/descrobject.h +++ b/Include/descrobject.h @@ -101,6 +101,14 @@ PyAPI_FUNC(PyObject *) PyDescr_NewWrapper(PyTypeObject *, PyAPI_FUNC(PyObject *) PyDictProxy_New(PyObject *); PyAPI_FUNC(PyObject *) PyWrapper_New(PyObject *, PyObject *); +typedef struct { + PyObject_HEAD + PyObject *prop_get; + PyObject *prop_set; + PyObject *prop_del; + PyObject *prop_doc; + int getter_doc; +} propertyobject; PyAPI_DATA(PyTypeObject) PyProperty_Type; #ifdef __cplusplus diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 4724b0edf33a7c..478591a81524ca 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -311,6 +311,11 @@ def __eq__(self, other): ### namedtuple ################################################################################ +try: + from _collections import _tuplegetter +except ImportError: + _tuplegetter = lambda index, doc: property(_itemgetter(index), doc=doc) + _nt_itemgetters = {} def namedtuple(typename, field_names, *, rename=False, defaults=None, module=None): @@ -454,12 +459,14 @@ def __getnewargs__(self): cache = _nt_itemgetters for index, name in enumerate(field_names): try: - itemgetter_object, doc = cache[index] + tuplegetter_object, doc = cache[index] except KeyError: - itemgetter_object = _itemgetter(index) doc = f'Alias for field number {index}' - cache[index] = itemgetter_object, doc - class_namespace[name] = property(itemgetter_object, doc=doc) + tuplegetter_object = _tuplegetter(index, doc=doc) + cache[index] = tuplegetter_object, doc + + tuplegetter_object.__doc__ = doc + class_namespace[name] = tuplegetter_object result = type(typename, (tuple,), class_namespace) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 267cf07f1f72a3..5e82059a76556c 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2328,6 +2328,107 @@ _count_elements(PyObject *self, PyObject *args) Py_RETURN_NONE; } +/* Helper functions for namedtuples */ + +typedef struct { + propertyobject po; + Py_ssize_t index; + PyObject* doc; +} _tuplegetterobject; + +static PyObject * +tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +{ + Py_ssize_t index; + PyObject *doc; + _tuplegetterobject *self; + + static char *kwlist[] = {"index", "doc", NULL}; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "n$O", kwlist, + &index, &doc)){ + return NULL; + } + + self = (_tuplegetterobject *)PyProperty_Type.tp_new(type, args, kwds); + if (self == NULL) { + return NULL; + } + self->index = index; + Py_INCREF(doc); + self->doc = doc; + return (PyObject *)self; +} + +static PyObject * +tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) +{ + PyObject *result; + if (obj == NULL || obj == Py_None) { + Py_INCREF(self); + return self; + } + result = PyTuple_GetItem(obj, ((_tuplegetterobject*)self)->index); + Py_XINCREF(result); + return result; +} + +static void +tuplegetter_dealloc(_tuplegetterobject *self) +{ + Py_XDECREF(self->doc); + PyProperty_Type.tp_dealloc((PyObject*)self); +} + + +static PyMemberDef tuplegetter_members[] = { + {"__doc__", T_OBJECT, offsetof(_tuplegetterobject, doc), 0}, + {0} +}; + +static PyTypeObject tuplegetter_type = { + PyVarObject_HEAD_INIT(NULL, 0) + "_collections.tuplegetter", /* tp_name */ + sizeof(_tuplegetterobject), /* tp_basicsize */ + 0, /* tp_itemsize */ + /* methods */ + (destructor)tuplegetter_dealloc, /* tp_dealloc */ + 0, /* tp_print */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_reserved */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + 0, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ + 0, /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + tuplegetter_members, /* tp_members */ + 0, /* tp_getset */ + &PyProperty_Type, /* tp_base */ + 0, /* tp_dict */ + tuplegetterdescr_get, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + tuplegetter_new, /* tp_new */ + 0, +}; + + /* module level code ********************************************************/ PyDoc_STRVAR(module_doc, @@ -2386,5 +2487,10 @@ PyInit__collections(void) Py_INCREF(&dequereviter_type); PyModule_AddObject(m, "_deque_reverse_iterator", (PyObject *)&dequereviter_type); + if (PyType_Ready(&tuplegetter_type) < 0) + return NULL; + Py_INCREF(&tuplegetter_type); + PyModule_AddObject(m, "_tuplegetter", (PyObject *)&tuplegetter_type); + return m; } diff --git a/Objects/descrobject.c b/Objects/descrobject.c index ca814bf78a69dd..4f8db2c58b4a94 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1256,15 +1256,6 @@ class property(object): */ -typedef struct { - PyObject_HEAD - PyObject *prop_get; - PyObject *prop_set; - PyObject *prop_del; - PyObject *prop_doc; - int getter_doc; -} propertyobject; - static PyObject * property_copy(PyObject *, PyObject *, PyObject *, PyObject *); From 5dc78ffc1f32e9b2a378d7b3fd970e219b92fc70 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 13 Nov 2018 01:03:19 +0000 Subject: [PATCH 02/14] Add News entry --- .../Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst b/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst new file mode 100644 index 00000000000000..7a758f800886cc --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst @@ -0,0 +1,2 @@ +Speed up :class:`namedtuple` attribute access by 2.5x using a C fast-path +for the name descriptors. Patch by Pablo Galindo. From 631ab2cb294e578ef7f505e5bb21514c50c88784 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 13 Nov 2018 01:54:56 +0000 Subject: [PATCH 03/14] fixup! bpo-32492: 2.5x speed up in namedtuple attribute access using C fast path --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5e82059a76556c..7117a45071ef1f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2387,7 +2387,7 @@ static PyMemberDef tuplegetter_members[] = { static PyTypeObject tuplegetter_type = { PyVarObject_HEAD_INIT(NULL, 0) - "_collections.tuplegetter", /* tp_name */ + "_collections._tuplegetter", /* tp_name */ sizeof(_tuplegetterobject), /* tp_basicsize */ 0, /* tp_itemsize */ /* methods */ From 21be735c328b1054795af574da559ee90a080a37 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 13 Nov 2018 10:51:44 +0000 Subject: [PATCH 04/14] Check for tuple in the __get__ of the new descriptor and don't cache the descriptor itself --- Lib/collections/__init__.py | 7 +++---- Modules/_collectionsmodule.c | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 478591a81524ca..62dab6786c7610 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -459,13 +459,12 @@ def __getnewargs__(self): cache = _nt_itemgetters for index, name in enumerate(field_names): try: - tuplegetter_object, doc = cache[index] + doc = cache[index] except KeyError: doc = f'Alias for field number {index}' - tuplegetter_object = _tuplegetter(index, doc=doc) - cache[index] = tuplegetter_object, doc + cache[index] = doc - tuplegetter_object.__doc__ = doc + tuplegetter_object = _tuplegetter(index, doc=doc) class_namespace[name] = tuplegetter_object result = type(typename, (tuple,), class_namespace) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 7117a45071ef1f..0f262faf591b1d 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2367,6 +2367,10 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) Py_INCREF(self); return self; } + if (!PyTuple_Check(obj)){ + PyErr_SetString(PyExc_TypeError, "_tuplegetter must be used with tuples"); + return NULL; + } result = PyTuple_GetItem(obj, ((_tuplegetterobject*)self)->index); Py_XINCREF(result); return result; From 1e145090919c052a0dddd0cfa3e32a6477dd4b9b Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 15 Nov 2018 02:01:12 +0000 Subject: [PATCH 05/14] Don't inherit from property. Implement GC methods to handle __doc__ --- Include/descrobject.h | 8 ----- Modules/_collectionsmodule.c | 69 ++++++++++++++++++++++++++---------- Objects/descrobject.c | 9 +++++ 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/Include/descrobject.h b/Include/descrobject.h index 07844f073c05f8..73bbb3fe54e5d0 100644 --- a/Include/descrobject.h +++ b/Include/descrobject.h @@ -101,14 +101,6 @@ PyAPI_FUNC(PyObject *) PyDescr_NewWrapper(PyTypeObject *, PyAPI_FUNC(PyObject *) PyDictProxy_New(PyObject *); PyAPI_FUNC(PyObject *) PyWrapper_New(PyObject *, PyObject *); -typedef struct { - PyObject_HEAD - PyObject *prop_get; - PyObject *prop_set; - PyObject *prop_del; - PyObject *prop_doc; - int getter_doc; -} propertyobject; PyAPI_DATA(PyTypeObject) PyProperty_Type; #ifdef __cplusplus diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 0f262faf591b1d..cc7c9c5ae8c85d 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2331,32 +2331,45 @@ _count_elements(PyObject *self, PyObject *args) /* Helper functions for namedtuples */ typedef struct { - propertyobject po; + PyObject_HEAD Py_ssize_t index; PyObject* doc; } _tuplegetterobject; static PyObject * tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +{ + _tuplegetterobject* self; + self = (_tuplegetterobject *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } + self->index = 0; + Py_INCREF(Py_None); + self->doc = Py_None; + return (PyObject *)self; +} + + +static int +tuplegetter_init(_tuplegetterobject *self, PyObject *args, PyObject *kwds) { Py_ssize_t index; - PyObject *doc; - _tuplegetterobject *self; + PyObject *doc, *tmp; static char *kwlist[] = {"index", "doc", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwds, "n$O", kwlist, &index, &doc)){ - return NULL; - } - - self = (_tuplegetterobject *)PyProperty_Type.tp_new(type, args, kwds); - if (self == NULL) { - return NULL; + return -1; } self->index = index; - Py_INCREF(doc); - self->doc = doc; - return (PyObject *)self; + if (doc) { + tmp = self->doc; + Py_INCREF(doc); + self->doc = doc; + Py_DECREF(tmp); + } + return 0; } static PyObject * @@ -2376,11 +2389,29 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) return result; } + +static int +tuplegetter_traverse(PyObject *self, visitproc visit, void *arg) +{ + _tuplegetterobject *tuplegetter = (_tuplegetterobject *)self; + Py_VISIT(tuplegetter->doc); + return 0; +} + +static int +tuplegetter_clear(PyObject *self) +{ + _tuplegetterobject *tuplegetter = (_tuplegetterobject *)self; + Py_CLEAR(tuplegetter->doc); + return 0; +} + static void tuplegetter_dealloc(_tuplegetterobject *self) { - Py_XDECREF(self->doc); - PyProperty_Type.tp_dealloc((PyObject*)self); + PyObject_GC_UnTrack(self); + tuplegetter_clear((PyObject*)self); + Py_TYPE(self)->tp_free((PyObject*)self); } @@ -2410,10 +2441,10 @@ static PyTypeObject tuplegetter_type = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ 0, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ + (traverseproc)tuplegetter_traverse, /* tp_traverse */ + (inquiry)tuplegetter_clear, /* tp_clear */ 0, /* tp_richcompare */ 0, /* tp_weaklistoffset */ 0, /* tp_iter */ @@ -2421,12 +2452,12 @@ static PyTypeObject tuplegetter_type = { 0, /* tp_methods */ tuplegetter_members, /* tp_members */ 0, /* tp_getset */ - &PyProperty_Type, /* tp_base */ + 0, /* tp_base */ 0, /* tp_dict */ tuplegetterdescr_get, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - 0, /* tp_init */ + (initproc)tuplegetter_init, /* tp_init */ 0, /* tp_alloc */ tuplegetter_new, /* tp_new */ 0, diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 4f8db2c58b4a94..ca814bf78a69dd 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1256,6 +1256,15 @@ class property(object): */ +typedef struct { + PyObject_HEAD + PyObject *prop_get; + PyObject *prop_set; + PyObject *prop_del; + PyObject *prop_doc; + int getter_doc; +} propertyobject; + static PyObject * property_copy(PyObject *, PyObject *, PyObject *, PyObject *); From f9ca1e47dc21f695ae39cfa33a0764683306b3e4 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 16 Nov 2018 00:15:15 +0000 Subject: [PATCH 06/14] Add a test for the docstring substitution in descriptors --- Lib/test/test_collections.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_collections.py b/Lib/test/test_collections.py index e2ffaaafd45424..c0815947f908b2 100644 --- a/Lib/test/test_collections.py +++ b/Lib/test/test_collections.py @@ -514,6 +514,14 @@ class Point(namedtuple('_Point', ['x', 'y'])): a.w = 5 self.assertEqual(a.__dict__, {'w': 5}) + def test_namedtuple_can_mutate_doc_of_descriptors_independently(self): + A = namedtuple('A', 'x y') + B = namedtuple('B', 'x y') + A.x.__doc__ = 'foo' + B.x.__doc__ = 'bar' + self.assertEqual(A.x.__doc__, 'foo') + self.assertEqual(B.x.__doc__, 'bar') + ################################################################################ ### Abstract Base Classes From a6187b823ddfe6e776d068d6ae8aa3c4b6c3f91b Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 16 Nov 2018 00:15:30 +0000 Subject: [PATCH 07/14] Update NEWS entry to reflect time against 3.7 branch --- .../Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst b/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst index 7a758f800886cc..24682b14f36d7e 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2018-11-13-01-03-10.bpo-32492.voIdcp.rst @@ -1,2 +1,2 @@ -Speed up :class:`namedtuple` attribute access by 2.5x using a C fast-path +Speed up :class:`namedtuple` attribute access by 1.6x using a C fast-path for the name descriptors. Patch by Pablo Galindo. From 7d2dd842c33c4fcf84fe9e11ed9d9a007e047263 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 16 Nov 2018 00:16:23 +0000 Subject: [PATCH 08/14] Simplify implementation with argument clinic, better error messages, only __new__ --- Modules/_collectionsmodule.c | 56 +++++++++++++-------------- Modules/clinic/_collectionsmodule.c.h | 26 +++++++++++++ 2 files changed, 53 insertions(+), 29 deletions(-) create mode 100644 Modules/clinic/_collectionsmodule.c.h diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index cc7c9c5ae8c85d..702517fef5c93e 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1,5 +1,6 @@ #include "Python.h" #include "structmember.h" +#include "clinic/_collectionsmodule.c.h" #ifdef STDC_HEADERS #include @@ -7,6 +8,11 @@ #include /* For size_t */ #endif +/*[clinic input] +class _tuplegetter "_tuplegetterobject *" "&tuplegetter_type" +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=ee5ed5baabe35068]*/ + /* collections module implementation of a deque() datatype Written and maintained by Raymond D. Hettinger */ @@ -2336,40 +2342,28 @@ typedef struct { PyObject* doc; } _tuplegetterobject; +/*[clinic input] +@classmethod +_tuplegetter.__new__ as tuplegetter_new + + index: Py_ssize_t + doc: object + +[clinic start generated code]*/ + static PyObject * -tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +tuplegetter_new_impl(PyTypeObject *type, Py_ssize_t index, PyObject *doc) +/*[clinic end generated code: output=014be444ad80263f input=bed3e2ac189db22a]*/ { _tuplegetterobject* self; self = (_tuplegetterobject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; } - self->index = 0; - Py_INCREF(Py_None); - self->doc = Py_None; - return (PyObject *)self; -} - - -static int -tuplegetter_init(_tuplegetterobject *self, PyObject *args, PyObject *kwds) -{ - Py_ssize_t index; - PyObject *doc, *tmp; - - static char *kwlist[] = {"index", "doc", NULL}; - if (!PyArg_ParseTupleAndKeywords(args, kwds, "n$O", kwlist, - &index, &doc)){ - return -1; - } self->index = index; - if (doc) { - tmp = self->doc; - Py_INCREF(doc); - self->doc = doc; - Py_DECREF(tmp); - } - return 0; + Py_INCREF(doc); + self->doc = doc; + return (PyObject *)self; } static PyObject * @@ -2380,8 +2374,12 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) Py_INCREF(self); return self; } - if (!PyTuple_Check(obj)){ - PyErr_SetString(PyExc_TypeError, "_tuplegetter must be used with tuples"); + if (!PyTuple_Check(obj)) { + PyErr_Format(PyExc_TypeError, + "descriptor for index '%d' for tuple subclasses " + "doesn't apply to '%s' object", + ((_tuplegetterobject*)self)->index, + obj->ob_type->tp_name); return NULL; } result = PyTuple_GetItem(obj, ((_tuplegetterobject*)self)->index); @@ -2457,7 +2455,7 @@ static PyTypeObject tuplegetter_type = { tuplegetterdescr_get, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - (initproc)tuplegetter_init, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ tuplegetter_new, /* tp_new */ 0, diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h new file mode 100644 index 00000000000000..f41c5f170957e2 --- /dev/null +++ b/Modules/clinic/_collectionsmodule.c.h @@ -0,0 +1,26 @@ +/*[clinic input] +preserve +[clinic start generated code]*/ + +static PyObject * +tuplegetter_new_impl(PyTypeObject *type, Py_ssize_t index, PyObject *doc); + +static PyObject * +tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) +{ + PyObject *return_value = NULL; + static const char * const _keywords[] = {"index", "doc", NULL}; + static _PyArg_Parser _parser = {"nO:_tuplegetter", _keywords, 0}; + Py_ssize_t index; + PyObject *doc; + + if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &_parser, + &index, &doc)) { + goto exit; + } + return_value = tuplegetter_new_impl(type, index, doc); + +exit: + return return_value; +} +/*[clinic end generated code: output=add072a2dd2c77dc input=a9049054013a1b77]*/ From e5bca1d348406007bfa219d2033a672a72eb0021 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 27 Nov 2018 00:43:05 +0000 Subject: [PATCH 09/14] Use positional-only parameters for the __new__ --- Lib/collections/__init__.py | 2 +- Modules/_collectionsmodule.c | 9 +++++---- Modules/clinic/_collectionsmodule.c.h | 10 ++++++---- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 62dab6786c7610..0b74c3f8915fd3 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -464,7 +464,7 @@ def __getnewargs__(self): doc = f'Alias for field number {index}' cache[index] = doc - tuplegetter_object = _tuplegetter(index, doc=doc) + tuplegetter_object = _tuplegetter(index, doc) class_namespace[name] = tuplegetter_object result = type(typename, (tuple,), class_namespace) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 702517fef5c93e..119f9c2a40225b 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1,6 +1,5 @@ #include "Python.h" #include "structmember.h" -#include "clinic/_collectionsmodule.c.h" #ifdef STDC_HEADERS #include @@ -13,6 +12,9 @@ class _tuplegetter "_tuplegetterobject *" "&tuplegetter_type" [clinic start generated code]*/ /*[clinic end generated code: output=da39a3ee5e6b4b0d input=ee5ed5baabe35068]*/ +static PyTypeObject tuplegetter_type; +#include "clinic/_collectionsmodule.c.h" + /* collections module implementation of a deque() datatype Written and maintained by Raymond D. Hettinger */ @@ -2348,12 +2350,12 @@ _tuplegetter.__new__ as tuplegetter_new index: Py_ssize_t doc: object - + / [clinic start generated code]*/ static PyObject * tuplegetter_new_impl(PyTypeObject *type, Py_ssize_t index, PyObject *doc) -/*[clinic end generated code: output=014be444ad80263f input=bed3e2ac189db22a]*/ +/*[clinic end generated code: output=014be444ad80263f input=87c576a5bdbc0bbb]*/ { _tuplegetterobject* self; self = (_tuplegetterobject *)type->tp_alloc(type, 0); @@ -2387,7 +2389,6 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) return result; } - static int tuplegetter_traverse(PyObject *self, visitproc visit, void *arg) { diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index f41c5f170957e2..12626c1f6618f6 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -9,12 +9,14 @@ static PyObject * tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { PyObject *return_value = NULL; - static const char * const _keywords[] = {"index", "doc", NULL}; - static _PyArg_Parser _parser = {"nO:_tuplegetter", _keywords, 0}; Py_ssize_t index; PyObject *doc; - if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &_parser, + if ((type == &tuplegetter_type) && + !_PyArg_NoKeywords("_tuplegetter", kwargs)) { + goto exit; + } + if (!PyArg_ParseTuple(args, "nO:_tuplegetter", &index, &doc)) { goto exit; } @@ -23,4 +25,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=add072a2dd2c77dc input=a9049054013a1b77]*/ +/*[clinic end generated code: output=83746071eacc28d3 input=a9049054013a1b77]*/ From 96eae4c22d9d82e2089138aa2c26b69c5501a055 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 28 Dec 2018 04:45:11 +0000 Subject: [PATCH 10/14] Use PyTuple_GET_SIZE and PyTuple_GET_ITEM to tighter the implementation of tuplegetterdescr_get --- Modules/_collectionsmodule.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 119f9c2a40225b..4207b49c3fca56 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2384,7 +2384,15 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) obj->ob_type->tp_name); return NULL; } - result = PyTuple_GetItem(obj, ((_tuplegetterobject*)self)->index); + + Py_ssize_t index = ((_tuplegetterobject*)self)->index; + + if (index < 0 || index >= PyTuple_GET_SIZE(obj)) { + PyErr_SetString(PyExc_IndexError, "tuple index out of range"); + return NULL; + } + + result = PyTuple_GET_ITEM(obj, index); Py_XINCREF(result); return result; } From 9838c39e36a7ba8e5344388e1fbe9947d4764fe6 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Sat, 29 Dec 2018 04:14:38 +0000 Subject: [PATCH 11/14] Implement __set__ to make tuplegetter a data descriptor --- Modules/_collectionsmodule.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 4207b49c3fca56..8ffbbb734d6368 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2397,6 +2397,17 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) return result; } +static int +tuplegetter_set(PyObject *self, PyObject *obj, PyObject *value) +{ + if (value == NULL) { + PyErr_SetString(PyExc_AttributeError, "can't delete attribute"); + } else { + PyErr_SetString(PyExc_AttributeError, "can't set attribute"); + } + return -1; +} + static int tuplegetter_traverse(PyObject *self, visitproc visit, void *arg) { @@ -2462,7 +2473,7 @@ static PyTypeObject tuplegetter_type = { 0, /* tp_base */ 0, /* tp_dict */ tuplegetterdescr_get, /* tp_descr_get */ - 0, /* tp_descr_set */ + tuplegetter_set, /* tp_descr_set */ 0, /* tp_dictoffset */ 0, /* tp_init */ 0, /* tp_alloc */ From 2e350edc96661572ad9e982cc6abac847b0b8ddb Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Sat, 29 Dec 2018 19:30:34 +0000 Subject: [PATCH 12/14] Use Py_INCREF now that we inline PyTuple_GetItem --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 8ffbbb734d6368..6c9f5e1601a916 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2393,7 +2393,7 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) } result = PyTuple_GET_ITEM(obj, index); - Py_XINCREF(result); + Py_INCREF(result); return result; } From c9772e866273da66059068a2d4cb9fe600eeaaf3 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sun, 30 Dec 2018 00:45:13 -0800 Subject: [PATCH 13/14] Apply the valid_index() function, saving one test --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 6c9f5e1601a916..6124781ba32c5f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2387,7 +2387,7 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) Py_ssize_t index = ((_tuplegetterobject*)self)->index; - if (index < 0 || index >= PyTuple_GET_SIZE(obj)) { + if (!valid_index(index, PyTuple_GET_SIZE(obj))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } From 62cd7fd21b746825a5d5dc6605b46964d53584df Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sun, 30 Dec 2018 00:54:05 -0800 Subject: [PATCH 14/14] Move Py_None test out of the critical path. --- Modules/_collectionsmodule.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 6124781ba32c5f..73e587df40b559 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2372,11 +2372,15 @@ static PyObject * tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type) { PyObject *result; - if (obj == NULL || obj == Py_None) { + if (obj == NULL) { Py_INCREF(self); return self; } if (!PyTuple_Check(obj)) { + if (obj == Py_None) { + Py_INCREF(self); + return self; + } PyErr_Format(PyExc_TypeError, "descriptor for index '%d' for tuple subclasses " "doesn't apply to '%s' object",