[pypy-commit] pypy cpyext-obj-stealing: Tweaks and simplifications
arigo
pypy.commits at gmail.com
Sat May 20 10:36:55 EDT 2017
Author: Armin Rigo <arigo at tunes.org>
Branch: cpyext-obj-stealing
Changeset: r91347:fb0a61fd753d
Date: 2017-05-20 16:36 +0200
http://bitbucket.org/pypy/pypy/changeset/fb0a61fd753d/
Log: Tweaks and simplifications
A few "pointless" operations have been added which I removed again;
I'm unsure if the point was only to make the test pass (I needed to
tweak it) or if there was more.
diff --git a/pypy/module/cpyext/include/listobject.h b/pypy/module/cpyext/include/listobject.h
--- a/pypy/module/cpyext/include/listobject.h
+++ b/pypy/module/cpyext/include/listobject.h
@@ -1,1 +1,1 @@
-#define PyList_GET_ITEM(o, i) PyList_GetItem((PyObject*)(o), (i))
+/* empty */
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
@@ -1,10 +1,10 @@
+from rpython.rlib.objectmodel import always_inline
from rpython.rtyper.lltypesystem import rffi, lltype
from pypy.module.cpyext.api import (cpython_api, CANNOT_FAIL, Py_ssize_t,
build_type_checkers)
from pypy.module.cpyext.pyerrors import PyErr_BadInternalCall
-from pypy.module.cpyext.pyobject import (decref, incref, PyObject, make_ref,
- w_obj_has_pyobj)
+from pypy.module.cpyext.pyobject import decref, incref, PyObject, make_ref
from pypy.objspace.std.listobject import W_ListObject
from pypy.interpreter.error import oefmt
@@ -21,11 +21,18 @@
setting all items to a real object with PyList_SetItem().
"""
w_list = space.newlist([None] * len)
- w_list.convert_to_cpy_strategy(space)
+ #w_list.convert_to_cpy_strategy(space)
return w_list
+ at always_inline
+def get_list_storage(space, w_list):
+ from pypy.module.cpyext.sequence import CPyListStrategy
+ assert isinstance(w_list, W_ListObject)
+ w_list.convert_to_cpy_strategy(space)
+ return CPyListStrategy.unerase(w_list.lstorage)
+
@cpython_api([rffi.VOIDP, Py_ssize_t, PyObject], lltype.Void, error=CANNOT_FAIL)
-def PyList_SET_ITEM(space, w_list, index, w_item):
+def PyList_SET_ITEM(space, w_list, index, py_item):
"""Form of PyList_SetItem() without error checking. This is normally
only used to fill in new lists where there is no previous content.
@@ -33,22 +40,15 @@
discard a reference to any item that it being replaced; any reference in
list at position i will be leaked.
"""
- from pypy.module.cpyext.sequence import CPyListStrategy
- cpy_strategy = space.fromcache(CPyListStrategy)
- assert isinstance(w_list, W_ListObject)
- assert w_list.strategy is cpy_strategy, "list strategy not CPyListStrategy"
+ storage = get_list_storage(space, w_list)
assert 0 <= index < w_list.length()
- w_old = w_list.getitem(index)
- incref(space, w_old) # since setitem calls decref, maintain cpython semantics here
- w_list.setitem(index, w_item)
- decref(space, w_item)
-
+ storage._elems[index] = py_item
@cpython_api([PyObject, Py_ssize_t, PyObject], rffi.INT_real, error=-1)
-def PyList_SetItem(space, w_list, index, w_item):
+def PyList_SetItem(space, w_list, index, py_item):
"""Set the item at index index in list to item. Return 0 on success
or -1 on failure.
-
+
This function "steals" a reference to item and discards a reference to
an item already in the list at the affected position.
"""
@@ -58,12 +58,19 @@
if index < 0 or index >= w_list.length():
decref(space, w_item)
raise oefmt(space.w_IndexError, "list assignment index out of range")
- w_list.convert_to_cpy_strategy(space)
- w_list.setitem(index, w_item)
- decref(space, w_item)
+ storage = get_list_storage(space, w_list)
+ py_old = storage._elems[index]
+ storage._elems[index] = py_item
+ decref(w_list.space, py_old)
return 0
- at cpython_api([PyObject, Py_ssize_t], PyObject, result_borrowed=True)
+ at cpython_api([rffi.VOIDP, Py_ssize_t], PyObject, result_is_ll=True)
+def PyList_GET_ITEM(space, w_list, index):
+ storage = get_list_storage(space, w_list)
+ assert 0 <= index < w_list.length()
+ return storage._elems[index] # borrowed ref
+
+ at cpython_api([PyObject, Py_ssize_t], PyObject, result_is_ll=True)
def PyList_GetItem(space, w_list, index):
"""Return the object at position pos in the list pointed to by p. The
position must be positive, indexing from the end of the list is not
@@ -73,18 +80,15 @@
PyErr_BadInternalCall(space)
if index < 0 or index >= w_list.length():
raise oefmt(space.w_IndexError, "list index out of range")
- w_list.convert_to_cpy_strategy(space)
- w_res = w_list.getitem(index)
- return w_res # borrowed ref
+ storage = get_list_storage(space, w_list)
+ return storage._elems[index] # borrowed ref
@cpython_api([PyObject, PyObject], rffi.INT_real, error=-1)
def PyList_Append(space, w_list, w_item):
if not isinstance(w_list, W_ListObject):
PyErr_BadInternalCall(space)
- #pyobj = make_ref(space, w_item)
w_list.append(w_item)
- w_list.convert_to_cpy_strategy(space) # calls incref when switching strategy
return 0
@cpython_api([PyObject, Py_ssize_t, PyObject], rffi.INT_real, error=-1)
@@ -92,7 +96,6 @@
"""Insert the item item into list list in front of index index. Return
0 if successful; return -1 and set an exception if unsuccessful.
Analogous to list.insert(index, item)."""
- make_ref(space, w_item)
space.call_method(space.w_list, "insert", w_list, space.newint(index), w_item)
return 0
diff --git a/pypy/module/cpyext/pyobject.py b/pypy/module/cpyext/pyobject.py
--- a/pypy/module/cpyext/pyobject.py
+++ b/pypy/module/cpyext/pyobject.py
@@ -276,14 +276,14 @@
"""
if is_pyobj(obj):
pyobj = rffi.cast(PyObject, obj)
- assert pyobj.c_ob_refcnt > 0
+ at_least = 1
else:
pyobj = as_pyobj(space, obj, w_userdata)
- if not pyobj:
- keepalive_until_here(obj)
- return pyobj
- assert pyobj.c_ob_refcnt >= rawrefcount.REFCNT_FROM_PYPY
- pyobj.c_ob_refcnt += 1
+ at_least = rawrefcount.REFCNT_FROM_PYPY
+ if pyobj:
+ assert pyobj.c_ob_refcnt >= at_least
+ pyobj.c_ob_refcnt += 1
+ keepalive_until_here(obj)
return pyobj
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
@@ -256,8 +256,9 @@
def setitem(self, w_list, index, w_obj):
storage = self.unerase(w_list.lstorage)
index = self._check_index(index, storage._length)
- decref(w_list.space, storage._elems[index])
+ py_old = storage._elems[index]
storage._elems[index] = make_ref(w_list.space, w_obj)
+ decref(w_list.space, py_old)
def length(self, w_list):
storage = self.unerase(w_list.lstorage)
diff --git a/pypy/module/cpyext/test/test_listobject.py b/pypy/module/cpyext/test/test_listobject.py
--- a/pypy/module/cpyext/test/test_listobject.py
+++ b/pypy/module/cpyext/test/test_listobject.py
@@ -38,11 +38,13 @@
assert api.PyList_Insert(w_l, 0, space.wrap(1)) == 0
assert api.PyList_Size(w_l) == 3
assert api.PyList_Insert(w_l, 99, space.wrap(2)) == 0
- assert space.unwrap(api.PyList_GetItem(w_l, 3)) == 2
+ assert api.PyObject_Compare(api.PyList_GetItem(w_l, 3),
+ space.wrap(2)) == 0
# insert at index -1: next-to-last
assert api.PyList_Insert(w_l, -1, space.wrap(3)) == 0
- assert space.unwrap(api.PyList_GetItem(w_l, 3)) == 3
-
+ assert api.PyObject_Compare(api.PyList_GET_ITEM(w_l, 3),
+ space.wrap(3)) == 0
+
def test_sort(self, space, api):
l = space.newlist([space.wrap(1), space.wrap(0), space.wrap(7000)])
assert api.PyList_Sort(l) == 0
@@ -204,7 +206,7 @@
def test_item_refcounts(self):
"""PyList_SET_ITEM leaks a reference to the target."""
module = self.import_extension('foo', [
- ("test_refcount_diff", "METH_NOARGS",
+ ("test_refcount_diff", "METH_VARARGS",
"""
/* test that the refcount differences for functions
* are correct. diff1 - expected refcount diff for i1,
@@ -230,33 +232,38 @@
PyObject* tmp, *o = PyList_New(0);
char errbuffer[1024];
- PyObject* i1 = PyBytes_FromString("random string 1");
- PyObject* i2 = PyBytes_FromString("random string 2");
+ PyObject* i1 = PyTuple_GetItem(args, 0);
+ PyObject* i2 = PyTuple_GetItem(args, 1);
Py_ssize_t old_count1, new_count1;
Py_ssize_t old_count2, new_count2;
Py_ssize_t diff;
int ret;
- Py_INCREF(i2); // since it is used in macros
-
- old_count1 = Py_REFCNT(i1); // 1
- old_count2 = Py_REFCNT(i2); // 1
+ old_count1 = Py_REFCNT(i1);
+ old_count2 = Py_REFCNT(i2);
ret = PyList_Append(o, i1);
if (ret != 0)
return NULL;
+ /* check the result of Append(), and also force the list
+ to use the CPyListStrategy now */
+ if (PyList_GET_ITEM(o, 0) != i1)
+ {
+ PyErr_SetString(PyExc_AssertionError, "Append() error?");
+ return NULL;
+ }
CHECKCOUNT(1, 0, "PyList_Append");
+ Py_INCREF(i2); /* for PyList_SET_ITEM */
+ CHECKCOUNT(0, 1, "Py_INCREF");
+
PyList_SET_ITEM(o, 0, i2);
CHECKCOUNT(0, 0, "PyList_SET_ITEM");
tmp = PyList_GET_ITEM(o, 0);
- // XXX should tmp == i2?
- if ((Py_REFCNT(tmp) != Py_REFCNT(i2)))
+ if (tmp != i2)
{
- sprintf(errbuffer, "GETITEM return (%ld) and i2 (%ld)refcounts"
- " unequal", (long)Py_REFCNT(tmp), (long)Py_REFCNT(i2));
- PyErr_SetString(PyExc_AssertionError, errbuffer);
+ PyErr_SetString(PyExc_AssertionError, "SetItem() error?");
return NULL;
}
CHECKCOUNT(0, 0, "PyList_GET_ITEM");
@@ -274,8 +281,6 @@
CHECKCOUNT(-1, 0, "Py_DECREF(o)");
}
#endif
- Py_DECREF(i1); // append incref'd.
- Py_DECREF(i2);
return PyLong_FromSsize_t(0);
""")])
- assert module.test_refcount_diff() == 0
+ assert module.test_refcount_diff(["first"], ["second"]) == 0
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
@@ -237,10 +237,6 @@
self.strategy = object_strategy
object_strategy.init_from_list_w(self, list_w)
- def ensure_object_strategy(self): # for cpyext
- if self.strategy is not self.space.fromcache(ObjectListStrategy):
- self.switch_to_object_strategy()
-
def _temporarily_as_objects(self):
if self.strategy is self.space.fromcache(ObjectListStrategy):
return self
More information about the pypy-commit
mailing list