Skip to content

Commit

Permalink
Issue python#18594: Fix the fast path for collections.Counter().
Browse files Browse the repository at this point in the history
The path wasn't being taken due to an over-restrictive type check.
  • Loading branch information
rhettinger committed Oct 1, 2013
1 parent 21b2933 commit 2ff2190
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
1 change: 1 addition & 0 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ PyAPI_FUNC(PyObject *) PyType_GenericNew(PyTypeObject *,
PyObject *, PyObject *);
#ifndef Py_LIMITED_API
PyAPI_FUNC(PyObject *) _PyType_Lookup(PyTypeObject *, PyObject *);
PyAPI_FUNC(PyObject *) _PyType_LookupId(PyTypeObject *, _Py_Identifier *);
PyAPI_FUNC(PyObject *) _PyObject_LookupSpecial(PyObject *, _Py_Identifier *);
PyAPI_FUNC(PyTypeObject *) _PyType_CalculateMetaclass(PyTypeObject *, PyObject *);
#endif
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ Library
- Issue #12641: Avoid passing "-mno-cygwin" to the mingw32 compiler, except
when necessary. Patch by Oscar Benjamin.

- Issue #18594: The fast path for collections.Counter() was never taken
due to an over-restrictive type check.

- Properly initialize all fields of a SSL object after allocation.

- Issue #4366: Fix building extensions on all platforms when --enable-shared
Expand Down
16 changes: 15 additions & 1 deletion Modules/_collectionsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1689,10 +1689,16 @@ Count elements in the iterable, updating the mappping");
static PyObject *
_count_elements(PyObject *self, PyObject *args)
{
_Py_IDENTIFIER(__getitem__);
_Py_IDENTIFIER(__setitem__);
PyObject *it, *iterable, *mapping, *oldval;
PyObject *newval = NULL;
PyObject *key = NULL;
PyObject *one = NULL;
PyObject *mapping_getitem;
PyObject *mapping_setitem;
PyObject *dict_getitem;
PyObject *dict_setitem;

if (!PyArg_UnpackTuple(args, "_count_elements", 2, 2, &mapping, &iterable))
return NULL;
Expand All @@ -1707,7 +1713,15 @@ _count_elements(PyObject *self, PyObject *args)
return NULL;
}

if (PyDict_CheckExact(mapping)) {
mapping_getitem = _PyType_LookupId(Py_TYPE(mapping), &PyId___getitem__);
dict_getitem = _PyType_LookupId(&PyDict_Type, &PyId___getitem__);
mapping_setitem = _PyType_LookupId(Py_TYPE(mapping), &PyId___setitem__);
dict_setitem = _PyType_LookupId(&PyDict_Type, &PyId___setitem__);

if (mapping_getitem != NULL &&
mapping_getitem == dict_getitem &&
mapping_setitem != NULL &&
mapping_setitem == dict_setitem) {
while (1) {
key = PyIter_Next(it);
if (key == NULL)
Expand Down
5 changes: 1 addition & 4 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ _Py_IDENTIFIER(__module__);
_Py_IDENTIFIER(__name__);
_Py_IDENTIFIER(__new__);

static PyObject *
_PyType_LookupId(PyTypeObject *type, struct _Py_Identifier *name);

static PyObject *
slot_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds);

Expand Down Expand Up @@ -2589,7 +2586,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name)
return res;
}

static PyObject *
PyObject *
_PyType_LookupId(PyTypeObject *type, struct _Py_Identifier *name)
{
PyObject *oname;
Expand Down

0 comments on commit 2ff2190

Please sign in to comment.