From 9b742052809ca844ffd868cc67a62adc38b9a784 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 16 Dec 2013 20:16:46 +0100 Subject: [PATCH] optimise list.pop() a little more by splitting it off the generic anyobj.pop() implementation --- Cython/Compiler/Optimize.py | 52 +++++++++++++++++++------------- Cython/Utility/Optimize.c | 73 ++++++++++++++++++++++++++------------------- tests/run/list_pop.pyx | 46 ++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 50 deletions(-) diff --git a/Cython/Compiler/Optimize.py b/Cython/Compiler/Optimize.py index 675cd34..4320e92 100644 --- a/Cython/Compiler/Optimize.py +++ b/Cython/Compiler/Optimize.py @@ -2388,42 +2388,54 @@ class OptimizeBuiltinCalls(Visitor.MethodDispatcherTransform): PyrexTypes.CFuncTypeArg("index", PyrexTypes.c_long_type, None), ]) - def _handle_simple_method_object_pop(self, node, function, args, is_unbound_method): + def _handle_simple_method_list_pop(self, node, function, args, is_unbound_method): + return self._handle_simple_method_object_pop( + node, function, args, is_unbound_method, is_list=True) + + def _handle_simple_method_object_pop(self, node, function, args, is_unbound_method, is_list=False): """Optimistic optimisation as X.pop([n]) is almost always referring to a list. """ + if not args: + return node + args = args[:] + if is_list: + type_name = 'List' + args[0] = args[0].as_none_safe_node( + "'NoneType' object has no attribute '%s'", + error="PyExc_AttributeError", + format_args=['pop']) + else: + type_name = 'Object' if len(args) == 1: return ExprNodes.PythonCapiCallNode( - node.pos, "__Pyx_PyObject_Pop", self.PyObject_Pop_func_type, - args = args, - may_return_none = True, - is_temp = node.is_temp, - utility_code = load_c_utility('pop') - ) + node.pos, "__Pyx_Py%s_Pop" % type_name, + self.PyObject_Pop_func_type, + args=args, + may_return_none=True, + is_temp=node.is_temp, + utility_code=load_c_utility('pop'), + ) elif len(args) == 2: - index = args[1] - if isinstance(index, ExprNodes.CoerceToPyTypeNode): - index = index.arg - if isinstance(index, ExprNodes.IntNode): - index = index.coerce_to(PyrexTypes.c_py_ssize_t_type, None) + index = unwrap_coerced_node(args[1]) + if is_list or isinstance(index, ExprNodes.IntNode): + index = index.coerce_to(PyrexTypes.c_py_ssize_t_type, self.current_env()) if index.type.is_int: widest = PyrexTypes.widest_numeric_type( index.type, PyrexTypes.c_py_ssize_t_type) if widest == PyrexTypes.c_py_ssize_t_type: args[1] = index return ExprNodes.PythonCapiCallNode( - node.pos, "__Pyx_PyObject_PopIndex", + node.pos, "__Pyx_Py%s_PopIndex" % type_name, self.PyObject_PopIndex_func_type, - args = args, - may_return_none = True, - is_temp = node.is_temp, - utility_code = load_c_utility("pop_index") - ) + args=args, + may_return_none=True, + is_temp=node.is_temp, + utility_code=load_c_utility("pop_index"), + ) return node - _handle_simple_method_list_pop = _handle_simple_method_object_pop - single_param_func_type = PyrexTypes.CFuncType( PyrexTypes.c_returncode_type, [ PyrexTypes.CFuncTypeArg("obj", PyrexTypes.py_object_type, None), diff --git a/Cython/Utility/Optimize.c b/Cython/Utility/Optimize.c index 65d698b..a5e1d57 100644 --- a/Cython/Utility/Optimize.c +++ b/Cython/Utility/Optimize.c @@ -76,24 +76,31 @@ static CYTHON_INLINE int __Pyx_PyList_Extend(PyObject* L, PyObject* v) { /////////////// pop.proto /////////////// -static CYTHON_INLINE PyObject* __Pyx_PyObject_Pop(PyObject* L); /*proto*/ +#define __Pyx_PyObject_Pop(L) (PyList_CheckExact(L) ? \ + __Pyx_PyList_Pop(L) : __Pyx__PyObject_Pop(L)) + +static CYTHON_INLINE PyObject* __Pyx_PyList_Pop(PyObject* L); /*proto*/ +static CYTHON_INLINE PyObject* __Pyx__PyObject_Pop(PyObject* L); /*proto*/ /////////////// pop /////////////// //@requires: ObjectHandling.c::PyObjectCallMethod -static CYTHON_INLINE PyObject* __Pyx_PyObject_Pop(PyObject* L) { +static CYTHON_INLINE PyObject* __Pyx__PyObject_Pop(PyObject* L) { +#if CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX >= 0x02050000 + if (Py_TYPE(L) == &PySet_Type) { + return PySet_Pop(L); + } +#endif + return __Pyx_PyObject_CallMethod0(L, PYIDENT("pop")); +} + +static CYTHON_INLINE PyObject* __Pyx_PyList_Pop(PyObject* L) { #if CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX >= 0x02040000 - if (likely(PyList_CheckExact(L)) - /* Check that both the size is positive and no reallocation shrinking needs to be done. */ - && likely(PyList_GET_SIZE(L) > (((PyListObject*)L)->allocated >> 1))) { + /* Check that both the size is positive and no reallocation shrinking needs to be done. */ + if (likely(PyList_GET_SIZE(L) > (((PyListObject*)L)->allocated >> 1))) { Py_SIZE(L) -= 1; return PyList_GET_ITEM(L, PyList_GET_SIZE(L)); } -#if PY_VERSION_HEX >= 0x02050000 - else if (Py_TYPE(L) == (&PySet_Type)) { - return PySet_Pop(L); - } -#endif #endif return __Pyx_PyObject_CallMethod0(L, PYIDENT("pop")); } @@ -101,31 +108,17 @@ static CYTHON_INLINE PyObject* __Pyx_PyObject_Pop(PyObject* L) { /////////////// pop_index.proto /////////////// -static PyObject* __Pyx_PyObject_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/ +#define __Pyx_PyObject_PopIndex(L, ix) (PyList_CheckExact(L) ? \ + __Pyx_PyList_PopIndex(L, ix) : __Pyx__PyObject_PopIndex(L, ix)) + +static PyObject* __Pyx_PyList_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/ +static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/ /////////////// pop_index /////////////// //@requires: ObjectHandling.c::PyObjectCallMethod -static PyObject* __Pyx_PyObject_PopIndex(PyObject* L, Py_ssize_t ix) { +static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, Py_ssize_t ix) { PyObject *r, *py_ix; -#if CYTHON_COMPILING_IN_CPYTHON - if (likely(PyList_CheckExact(L))) { - Py_ssize_t size = PyList_GET_SIZE(L); - if (likely(size > (((PyListObject*)L)->allocated >> 1))) { - Py_ssize_t cix = ix; - if (cix < 0) { - cix += size; - } - if (likely(0 <= cix && cix < size)) { - PyObject* v = PyList_GET_ITEM(L, cix); - Py_SIZE(L) -= 1; - size -= 1; - memmove(&PyList_GET_ITEM(L, cix), &PyList_GET_ITEM(L, cix+1), (size-cix)*sizeof(PyObject*)); - return v; - } - } - } -#endif py_ix = PyInt_FromSsize_t(ix); if (!py_ix) return NULL; r = __Pyx_PyObject_CallMethod1(L, PYIDENT("pop"), py_ix); @@ -133,6 +126,26 @@ static PyObject* __Pyx_PyObject_PopIndex(PyObject* L, Py_ssize_t ix) { return r; } +static PyObject* __Pyx_PyList_PopIndex(PyObject* L, Py_ssize_t ix) { +#if CYTHON_COMPILING_IN_CPYTHON + Py_ssize_t size = PyList_GET_SIZE(L); + if (likely(size > (((PyListObject*)L)->allocated >> 1))) { + Py_ssize_t cix = ix; + if (cix < 0) { + cix += size; + } + if (likely(0 <= cix && cix < size)) { + PyObject* v = PyList_GET_ITEM(L, cix); + Py_SIZE(L) -= 1; + size -= 1; + memmove(&PyList_GET_ITEM(L, cix), &PyList_GET_ITEM(L, cix+1), (size-cix)*sizeof(PyObject*)); + return v; + } + } +#endif + return __Pyx__PyObject_PopIndex(L, ix); +} + /////////////// dict_getitem_default.proto /////////////// diff --git a/tests/run/list_pop.pyx b/tests/run/list_pop.pyx index 2e365da..a0cfc06 100644 --- a/tests/run/list_pop.pyx +++ b/tests/run/list_pop.pyx @@ -108,6 +108,10 @@ def index_pop_typed(list L, int i): Traceback (most recent call last): IndexError: pop index out of range + >>> index_pop_typed(None, 0) + Traceback (most recent call last): + AttributeError: 'NoneType' object has no attribute 'pop' + >>> while L: ... _ = index_pop_typed(L, 0) @@ -120,6 +124,48 @@ def index_pop_typed(list L, int i): """ return L.pop(i) + +@cython.test_assert_path_exists('//PythonCapiCallNode') +@cython.test_fail_if_path_exists('//SimpleCallNode/AttributeNode') +def index_pop_list_object_index(list L, i): + """ + >>> L = list(range(10)) + >>> index_pop_list_object_index(L, 2) + 2 + >>> index_pop_list_object_index(L, -2) + 8 + >>> L + [0, 1, 3, 4, 5, 6, 7, 9] + >>> index_pop_list_object_index(L, 100) + Traceback (most recent call last): + IndexError: pop index out of range + >>> index_pop_list_object_index(L, -100) + Traceback (most recent call last): + IndexError: pop index out of range + + >>> index_pop_list_object_index(None, 0) + Traceback (most recent call last): + AttributeError: 'NoneType' object has no attribute 'pop' + >>> index_pop_list_object_index([1], None) # doctest: +ELLIPSIS + Traceback (most recent call last): + TypeError: ... + >>> index_pop_list_object_index([1], 'abc') # doctest: +ELLIPSIS + Traceback (most recent call last): + TypeError: ... + + >>> while L: + ... _ = index_pop_list_object_index(L, 0) + + >>> L + [] + + >>> index_pop_list_object_index(L, 0) + Traceback (most recent call last): + IndexError: pop from empty list + """ + return L.pop(i) + + @cython.test_assert_path_exists('//PythonCapiCallNode') @cython.test_fail_if_path_exists('//SimpleCallNode/AttributeNode') def index_pop_literal(list L): -- 2.7.4