[pypy-commit] pypy list-strategies: implement a fast path for list.pop() (without arguments)

[cfbolz]

Author: Carl Friedrich Bolz <cfbolz at gmx.de>
Branch: list-strategies
Changeset: r48667:caa63b86e8cf
Date: 2011-11-02 18:19 +0100
http://bitbucket.org/pypy/pypy/changeset/caa63b86e8cf/

Log:	implement a fast path for list.pop() (without arguments)

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
@@ -189,6 +189,10 @@
         May raise IndexError."""
         return self.strategy.pop(self, index)
 
+    def pop_end(self):
+        """ Pop the last element from the list."""
+        return self.strategy.pop_end(self)
+
     def setitem(self, index, w_item):
         """Inserts a wrapped item at the given (unwrapped) index.
         May raise IndexError."""
@@ -282,6 +286,9 @@
     def pop(self, w_list, index):
         raise NotImplementedError
 
+    def pop_end(self, w_list):
+        return self.pop(w_list, self.length(w_list) - 1)
+
     def setitem(self, w_list, index, w_item):
         raise NotImplementedError
 
@@ -372,7 +379,7 @@
         pass
 
     def pop(self, w_list, index):
-        # will not be called becuase IndexError was already raised in
+        # will not be called because IndexError was already raised in
         # list_pop__List_ANY
         raise IndexError
 
@@ -527,21 +534,25 @@
         self.switch_to_integer_strategy(w_list)
         w_list.deleteslice(start, step, slicelength)
 
+    def pop_end(self, w_list):
+        start, step, length = self.unerase(w_list.lstorage)
+        w_result = self.wrap(start + (length - 1) * step)
+        new = self.erase((start, step, length - 1))
+        w_list.lstorage = new
+        return w_result
+
     def pop(self, w_list, index):
         l = self.unerase(w_list.lstorage)
         start = l[0]
         step = l[1]
         length = l[2]
         if index == 0:
-            r = self.getitem(w_list, index)
+            w_result = self.wrap(start)
             new = self.erase((start + step, step, length - 1))
             w_list.lstorage = new
-            return r
+            return w_result
         elif index == length - 1:
-            r = self.getitem(w_list, index)
-            new = self.erase((start, step, length - 1))
-            w_list.lstorage = new
-            return r
+            return self.pop_end(w_list)
         else:
             self.switch_to_integer_strategy(w_list)
             return w_list.pop(index)
@@ -812,6 +823,10 @@
             assert start >= 0 # annotator hint
             del items[start:]
 
+    def pop_end(self, w_list):
+        l = self.unerase(w_list.lstorage)
+        return self.wrap(l.pop())
+
     def pop(self, w_list, index):
         l = self.unerase(w_list.lstorage)
         # not sure if RPython raises IndexError on pop
@@ -1196,12 +1211,15 @@
     w_list.extend(w_other)
     return space.w_None
 
-# note that the default value will come back wrapped!!!
-def list_pop__List_ANY(space, w_list, w_idx=-1):
+# default of w_idx is space.w_None (see listtype.py)
+def list_pop__List_ANY(space, w_list, w_idx):
     length = w_list.length()
     if length == 0:
         raise OperationError(space.w_IndexError,
                              space.wrap("pop from empty list"))
+    # clearly differentiate between list.pop() and list.pop(index)
+    if space.is_w(w_idx, space.w_None):
+        return w_list.pop_end() # cannot raise because list is not empty
     if space.isinstance_w(w_idx, space.w_float):
         raise OperationError(space.w_TypeError,
             space.wrap("integer argument expected, got float")
diff --git a/pypy/objspace/std/listtype.py b/pypy/objspace/std/listtype.py
--- a/pypy/objspace/std/listtype.py
+++ b/pypy/objspace/std/listtype.py
@@ -11,7 +11,7 @@
 list_extend   = SMM('extend', 2,
                     doc='L.extend(iterable) -- extend list by appending'
                         ' elements from the iterable')
-list_pop      = SMM('pop',    2, defaults=(-1,),
+list_pop      = SMM('pop',    2, defaults=(None,),
                     doc='L.pop([index]) -> item -- remove and return item at'
                         ' index (default last)')
 list_remove   = SMM('remove', 2,
diff --git a/pypy/objspace/std/test/test_liststrategies.py b/pypy/objspace/std/test/test_liststrategies.py
--- a/pypy/objspace/std/test/test_liststrategies.py
+++ b/pypy/objspace/std/test/test_liststrategies.py
@@ -1,4 +1,5 @@
 from pypy.objspace.std.listobject import W_ListObject, EmptyListStrategy, ObjectListStrategy, IntegerListStrategy, StringListStrategy, RangeListStrategy, make_range_list
+from pypy.objspace.std import listobject
 from pypy.objspace.std.test.test_listobject import TestW_ListObject
 
 from pypy.conftest import gettestobjspace
@@ -237,6 +238,18 @@
 
         l = make_range_list(self.space, 1,3,7)
         assert isinstance(l.strategy, RangeListStrategy)
+        v = l.pop(0)
+        assert self.space.eq_w(v, self.space.wrap(1))
+        assert isinstance(l.strategy, RangeListStrategy)
+        v = l.pop(l.length() - 1)
+        assert self.space.eq_w(v, self.space.wrap(19))
+        assert isinstance(l.strategy, RangeListStrategy)
+        v = l.pop_end()
+        assert self.space.eq_w(v, self.space.wrap(16))
+        assert isinstance(l.strategy, RangeListStrategy)
+
+        l = make_range_list(self.space, 1,3,7)
+        assert isinstance(l.strategy, RangeListStrategy)
         l.append(self.space.wrap("string"))
         assert isinstance(l.strategy, ObjectListStrategy)
 
@@ -379,6 +392,13 @@
         assert space.listview_str(w_l) == ["a", "b", "c"]
         assert space.listview_str(w_l2) == ["a", "b", "c"]
 
+    def test_pop_without_argument_is_fast(self):
+        space = self.space
+        w_l = W_ListObject(space, [space.wrap(1), space.wrap(2), space.wrap(3)])
+        w_l.pop = None
+        w_res = listobject.list_pop__List_ANY(space, w_l, space.w_None) # does not crash
+        assert space.unwrap(w_res) == 3
+
 
 class TestW_ListStrategiesDisabled:
     def setup_class(cls):