[pypy-commit] pypy default: Refactor PySequence_FAST_XXX to allow notably for specialized tuples (plus it's

arigo pypy.commits at gmail.com
Sat Feb 16 09:18:28 EST 2019


Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r96026:1d688af446c2
Date: 2019-02-16 13:53 +0100
http://bitbucket.org/pypy/pypy/changeset/1d688af446c2/

Log:	Refactor PySequence_FAST_XXX to allow notably for specialized tuples
	(plus it's a clean-up, I think)

diff --git a/pypy/module/cpyext/listobject.py b/pypy/module/cpyext/listobject.py
--- a/pypy/module/cpyext/listobject.py
+++ b/pypy/module/cpyext/listobject.py
@@ -104,7 +104,8 @@
 def PyList_GET_SIZE(space, w_obj):
     """Macro form of PyList_Size() without error checking.
     """
-    return space.len_w(w_obj)
+    assert isinstance(w_obj, W_ListObject)
+    return w_obj.length()
 
 
 @cpython_api([PyObject], Py_ssize_t, error=-1)
diff --git a/pypy/module/cpyext/sequence.py b/pypy/module/cpyext/sequence.py
--- a/pypy/module/cpyext/sequence.py
+++ b/pypy/module/cpyext/sequence.py
@@ -50,50 +50,56 @@
     converted to a sequence, and raises a TypeError, raise a new TypeError with
     m as the message text. If the conversion otherwise, fails, reraise the
     original exception"""
-    if isinstance(w_obj, tupleobject.W_TupleObject):
+    if isinstance(w_obj, tupleobject.W_AbstractTupleObject):
         return w_obj   # CCC avoid the double conversion that occurs here
     if isinstance(w_obj, W_ListObject):
-        # make sure we can return a borrowed obj from PySequence_Fast_GET_ITEM
-        w_obj.convert_to_cpy_strategy(space)
+        # note: we used to call w_obj.convert_to_cpy_strategy() here,
+        # but we really have to call it from PySequence_Fast_GET_ITEM()
+        # because some people never call PySequence_Fast() if they know
+        # the object is a list.
         return w_obj
     try:
-        return W_ListObject.newlist_cpyext(space, space.listview(w_obj))
+        return tupleobject.W_TupleObject(space.fixedview(w_obj))
     except OperationError as e:
         if e.match(space, space.w_TypeError):
             raise OperationError(space.w_TypeError, space.newtext(rffi.charp2str(m)))
         raise e
 
-# CCC this should be written as a C macro
- at cpython_api([rffi.VOIDP, Py_ssize_t], PyObject, result_borrowed=True)
-def PySequence_Fast_GET_ITEM(space, w_obj, index):
+# CCC this should be written as a C macro, at least for the tuple case
+ at cpython_api([rffi.VOIDP, Py_ssize_t], PyObject, result_is_ll=True)
+def PySequence_Fast_GET_ITEM(space, py_obj, index):
     """Return the ith element of o, assuming that o was returned by
     PySequence_Fast(), o is not NULL, and that i is within bounds.
     """
-    if isinstance(w_obj, W_ListObject):
-        return w_obj.getitem(index)
-    elif isinstance(w_obj, tupleobject.W_TupleObject):
-        return w_obj.wrappeditems[index]
-    raise oefmt(space.w_TypeError,
-                "PySequence_Fast_GET_ITEM called but object is not a list or "
-                "sequence")
+    py_obj = rffi.cast(PyObject, py_obj)
+    if PyTuple_Check(space, py_obj):
+        from pypy.module.cpyext.tupleobject import PyTupleObject
+        py_tuple = rffi.cast(PyTupleObject, py_obj)
+        return py_tuple.c_ob_item[index]
+    else:
+        from pypy.module.cpyext.listobject import PyList_GET_ITEM
+        w_obj = from_ref(space, py_obj)
+        return PyList_GET_ITEM(space, w_obj, index)
 
 @cpython_api([rffi.VOIDP], Py_ssize_t, error=CANNOT_FAIL)
-def PySequence_Fast_GET_SIZE(space, w_obj):
+def PySequence_Fast_GET_SIZE(space, py_obj):
     """Returns the length of o, assuming that o was returned by
     PySequence_Fast() and that o is not NULL.  The size can also be
     gotten by calling PySequence_Size() on o, but
     PySequence_Fast_GET_SIZE() is faster because it can assume o is a list
     or tuple."""
-    if isinstance(w_obj, W_ListObject):
-        return w_obj.length()
-    elif isinstance(w_obj, tupleobject.W_TupleObject):
-        return len(w_obj.wrappeditems)
-    raise oefmt(space.w_TypeError,
-                "PySequence_Fast_GET_SIZE called but object is not a list or "
-                "sequence")
+    py_obj = rffi.cast(PyObject, py_obj)
+    if PyTuple_Check(space, py_obj):
+        from pypy.module.cpyext.tupleobject import PyTupleObject
+        py_tuple = rffi.cast(PyTupleObject, py_obj)
+        return py_tuple.c_ob_size
+    else:
+        from pypy.module.cpyext.listobject import PyList_GET_SIZE
+        w_obj = from_ref(space, py_obj)
+        return PyList_GET_SIZE(space, w_obj)
 
 @cpython_api([rffi.VOIDP], PyObjectP)
-def PySequence_Fast_ITEMS(space, w_obj):
+def PySequence_Fast_ITEMS(space, py_obj):
     """Return the underlying array of PyObject pointers.  Assumes that o was returned
     by PySequence_Fast() and o is not NULL.
 
@@ -101,18 +107,17 @@
     So, only use the underlying array pointer in contexts where the sequence
     cannot change.
     """
-    if isinstance(w_obj, W_ListObject):
-        cpy_strategy = space.fromcache(CPyListStrategy)
-        if w_obj.strategy is cpy_strategy:
-            return w_obj.get_raw_items() # asserts it's a cpyext strategy
-    elif isinstance(w_obj, tupleobject.W_TupleObject):
+    py_obj = rffi.cast(PyObject, py_obj)
+    if PyTuple_Check(space, py_obj):
         from pypy.module.cpyext.tupleobject import PyTupleObject
-        py_obj = as_pyobj(space, w_obj)
         py_tuple = rffi.cast(PyTupleObject, py_obj)
         return rffi.cast(PyObjectP, py_tuple.c_ob_item)
-    raise oefmt(space.w_TypeError,
-                "PySequence_Fast_ITEMS called but object is not the result of "
-                "PySequence_Fast")
+    else:
+        from pypy.module.cpyext.listobject import get_list_storage
+        w_obj = from_ref(space, py_obj)
+        assert isinstance(w_obj, W_ListObject)
+        storage = get_list_storage(space, w_obj)
+        return rffi.cast(PyObjectP, storage._elems)
 
 @cpython_api([PyObject, Py_ssize_t, Py_ssize_t], PyObject)
 def PySequence_GetSlice(space, w_obj, start, end):
@@ -300,10 +305,6 @@
         storage = self.unerase(w_list.lstorage)
         return storage._length
 
-    def get_raw_items(self, w_list):
-        storage = self.unerase(w_list.lstorage)
-        return storage._elems
-
     def getslice(self, w_list, start, stop, step, length):
         w_list.switch_to_object_strategy()
         return w_list.strategy.getslice(w_list, start, stop, step, length)
diff --git a/pypy/module/cpyext/test/test_cpyext.py b/pypy/module/cpyext/test/test_cpyext.py
--- a/pypy/module/cpyext/test/test_cpyext.py
+++ b/pypy/module/cpyext/test/test_cpyext.py
@@ -138,6 +138,7 @@
                                    'itertools', 'time', 'binascii',
                                    'mmap'
                                    ])
+    spaceconfig["objspace.std.withspecialisedtuple"] = True
 
     @classmethod
     def preload_builtins(cls, space):
diff --git a/pypy/module/cpyext/test/test_sequence.py b/pypy/module/cpyext/test/test_sequence.py
--- a/pypy/module/cpyext/test/test_sequence.py
+++ b/pypy/module/cpyext/test/test_sequence.py
@@ -21,12 +21,14 @@
         w_l = space.wrap([1, 2, 3, 4])
         assert api.PySequence_Fast(w_l, "message") is w_l
 
-        assert space.int_w(api.PySequence_Fast_GET_ITEM(w_l, 1)) == 2
+        py_result = api.PySequence_Fast_GET_ITEM(w_l, 1)
+        w_result = get_w_obj_and_decref(space, py_result)
+        assert space.int_w(w_result) == 2
         assert api.PySequence_Fast_GET_SIZE(w_l) == 4
 
         w_set = space.wrap(set((1, 2, 3, 4)))
         w_seq = api.PySequence_Fast(w_set, "message")
-        assert space.type(w_seq) is space.w_list
+        assert space.type(w_seq) is space.w_tuple
         assert space.len_w(w_seq) == 4
 
         w_seq = api.PySequence_Tuple(w_set)
@@ -83,7 +85,7 @@
 
     def test_get_slice_fast(self, space, api):
         w_t = space.wrap([1, 2, 3, 4, 5])
-        api.PySequence_Fast(w_t, "foo")  # converts
+        api.PyList_GetItem(w_t, 0)  # converts to cpy strategy
         assert space.unwrap(api.PySequence_GetSlice(w_t, 2, 4)) == [3, 4]
         assert space.unwrap(api.PySequence_GetSlice(w_t, 1, -1)) == [2, 3, 4]
 
@@ -215,7 +217,7 @@
 class TestCPyListStrategy(BaseApiTest):
     def test_getitem_setitem(self, space, api):
         w_l = space.wrap([1, 2, 3, 4])
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         assert space.int_w(space.len(w_l)) == 4
         assert space.int_w(space.getitem(w_l, space.wrap(1))) == 2
         assert space.int_w(space.getitem(w_l, space.wrap(0))) == 1
@@ -229,17 +231,17 @@
         w = space.wrap
         w_l = w([1, 2, 3, 4])
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         space.call_method(w_l, 'insert', w(0), w(0))
         assert space.int_w(space.len(w_l)) == 5
         assert space.int_w(space.getitem(w_l, w(3))) == 3
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         space.call_method(w_l, 'sort')
         assert space.int_w(space.len(w_l)) == 5
         assert space.int_w(space.getitem(w_l, w(0))) == 0
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         w_t = space.wrap(space.fixedview(w_l))
         assert space.int_w(space.len(w_t)) == 5
         assert space.int_w(space.getitem(w_t, w(0))) == 0
@@ -247,22 +249,22 @@
         assert space.int_w(space.len(w_l2)) == 5
         assert space.int_w(space.getitem(w_l2, w(0))) == 0
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         w_sum = space.add(w_l, w_l)
         assert space.int_w(space.len(w_sum)) == 10
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         w_prod = space.mul(w_l, space.wrap(2))
         assert space.int_w(space.len(w_prod)) == 10
 
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
         w_l.inplace_mul(2)
         assert space.int_w(space.len(w_l)) == 10
 
     def test_getstorage_copy(self, space, api):
         w = space.wrap
         w_l = w([1, 2, 3, 4])
-        api.PySequence_Fast(w_l, "foo") # converts
+        api.PyList_GetItem(w_l, 0)   # converts to cpy strategy
 
         w_l1 = w([])
         space.setitem(w_l1, space.newslice(w(0), w(0), w(1)), w_l)
@@ -285,6 +287,10 @@
                 if (objects == NULL)
                     return NULL;
                 size = PySequence_Fast_GET_SIZE(foo);
+                for (i = 0; i < size; ++i) {
+                    if (objects[i] != PySequence_Fast_GET_ITEM(foo, i))
+                        return PyBool_FromLong(0);
+                }
                 common_type = size > 0 ? Py_TYPE(objects[0]) : NULL;
                 for (i = 1; i < size; ++i) {
                     if (Py_TYPE(objects[i]) != common_type) {
@@ -304,6 +310,9 @@
         s = (1, 2, 3, 4)
         assert module.test_fast_sequence(s[0:-1])
         assert module.test_fast_sequence(s[::-1])
+        s = (1, 2)    # specialized tuple
+        assert module.test_fast_sequence(s[0:-1])
+        assert module.test_fast_sequence(s[::-1])
         s = "1234"
         assert module.test_fast_sequence(s[0:-1])
         assert module.test_fast_sequence(s[::-1])
diff --git a/pypy/objspace/std/listobject.py b/pypy/objspace/std/listobject.py
--- a/pypy/objspace/std/listobject.py
+++ b/pypy/objspace/std/listobject.py
@@ -215,13 +215,6 @@
         storage = strategy.erase(list_f)
         return W_ListObject.from_storage_and_strategy(space, storage, strategy)
 
-    @staticmethod
-    def newlist_cpyext(space, list):
-        from pypy.module.cpyext.sequence import CPyListStrategy, CPyListStorage
-        strategy = space.fromcache(CPyListStrategy)
-        storage = strategy.erase(CPyListStorage(space, list))
-        return W_ListObject.from_storage_and_strategy(space, storage, strategy)
-
     def __repr__(self):
         """ representation for debugging purposes """
         return "%s(%s, %s)" % (self.__class__.__name__, self.strategy,
@@ -260,13 +253,6 @@
         self.strategy = cpy_strategy
         self.lstorage = cpy_strategy.erase(CPyListStorage(space, lst))
 
-    def get_raw_items(self):
-        from pypy.module.cpyext.sequence import CPyListStrategy
-
-        cpy_strategy = self.space.fromcache(CPyListStrategy)
-        assert self.strategy is cpy_strategy # should we return an error?
-        return cpy_strategy.get_raw_items(self)
-
     # ___________________________________________________
 
     def init_from_list_w(self, list_w):


More information about the pypy-commit mailing list