[pypy-commit] pypy typed-cells: slightly different approach for integrating mutable cells and immutability:

cfbolz pypy.commits at gmail.com
Fri Jan 15 13:40:03 EST 2016


Author: Carl Friedrich Bolz <cfbolz at gmx.de>
Branch: typed-cells
Changeset: r81814:d4fbc7fa07b0
Date: 2016-01-15 19:39 +0100
http://bitbucket.org/pypy/pypy/changeset/d4fbc7fa07b0/

Log:	slightly different approach for integrating mutable cells and
	immutability:

	before, an integer field went from storing a W_IntObject immutably,
	then switching to a IntMutableCell on the first mutation. This makes
	everything less type stable, leading to more bridges and making this
	branch fundamentally incompatible with heap profiling. Now the
	IntMutableCell is *always* stored, but if it is ever mutated
	map.ever_mutated is set to True. That means that as long as
	ever_mutated is False, it's safe to fold the read from the
	IntMutableCell away as well, in an @elidable method.

diff --git a/pypy/objspace/std/mapdict.py b/pypy/objspace/std/mapdict.py
--- a/pypy/objspace/std/mapdict.py
+++ b/pypy/objspace/std/mapdict.py
@@ -36,19 +36,15 @@
         if attr is None:
             return self.terminator._read_terminator(obj, selector)
         if (
-            jit.isconstant(attr.storageindex) and
+            jit.isconstant(attr) and
             jit.isconstant(obj) and
             not attr.ever_mutated
         ):
-            result = self._pure_mapdict_read_storage(obj, attr.storageindex)
+            result = attr._pure_read(obj)
         else:
             result = obj._mapdict_read_storage(attr.storageindex)
         return attr._read_cell(result)
 
-    @jit.elidable
-    def _pure_mapdict_read_storage(self, obj, storageindex):
-        return obj._mapdict_read_storage(storageindex)
-
     def write(self, obj, selector, w_value):
         attr = self.find_map_attr(selector)
         if attr is None:
@@ -176,6 +172,8 @@
         # the order is important here: first change the map, then the storage,
         # for the benefit of the special subclasses
         obj._set_mapdict_map(attr)
+        w_value = attr._write_cell(None, w_value)
+        assert w_value is not None
         obj._mapdict_write_storage(attr.storageindex, w_value)
 
     def materialize_r_dict(self, space, obj, dict_w):
@@ -298,6 +296,13 @@
         # if the flag is False, we don't need to unbox the attribute.
         self.can_contain_mutable_cell = False
 
+    @jit.elidable
+    def _pure_read(self, obj):
+        # this is safe even if the mapdict stores a mutable cell. the cell can
+        # only be changed is ever_mutated is set to True
+        result = obj._mapdict_read_storage(self.storageindex)
+        return self._read_cell(result)
+
     def _read_cell(self, w_cell):
         if not self.can_contain_mutable_cell:
             return w_cell
@@ -313,16 +318,14 @@
                 return None
             check = self._ensure_can_contain_mutable_cell()
             assert check
-            if self.ever_mutated:
-                return IntMutableCell(w_value.intval)
+            return IntMutableCell(w_value.intval)
         if type(w_value) is W_FloatObject:
             if isinstance(w_cell, FloatMutableCell):
                 w_cell.floatvalue = w_value.floatval
                 return None
             check = self._ensure_can_contain_mutable_cell()
             assert check
-            if self.ever_mutated:
-                return FloatMutableCell(w_value.floatval)
+            return FloatMutableCell(w_value.floatval)
         return w_value
 
     @jit.elidable
diff --git a/pypy/objspace/std/test/test_mapdict.py b/pypy/objspace/std/test/test_mapdict.py
--- a/pypy/objspace/std/test/test_mapdict.py
+++ b/pypy/objspace/std/test/test_mapdict.py
@@ -108,23 +108,27 @@
     assert obj2.map is obj.map
 
 def test_attr_immutability(monkeypatch):
+    from pypy.objspace.std.intobject import W_IntObject
     cls = Class()
     obj = cls.instantiate()
-    obj.setdictvalue(space, "a", 10)
-    obj.setdictvalue(space, "b", 20)
-    obj.setdictvalue(space, "b", 30)
-    assert obj.storage == [10, 30]
+    obj.setdictvalue(space, "a", W_IntObject(10))
+    obj.setdictvalue(space, "b", W_IntObject(20))
+    obj.setdictvalue(space, "b", W_IntObject(30))
+    mutcella, mutcellb = obj.storage
+    assert mutcella.intvalue == 10
+    assert mutcellb.intvalue == 30
     assert obj.map.ever_mutated == True
     assert obj.map.back.ever_mutated == False
 
     indices = []
+    orig_pure_read = PlainAttribute._pure_read
 
-    def _pure_mapdict_read_storage(obj, storageindex):
-        assert storageindex == 0
-        indices.append(storageindex)
-        return obj._mapdict_read_storage(storageindex)
+    def _pure_read(self, obj):
+        assert self.storageindex == 0
+        indices.append(self.storageindex)
+        return orig_pure_read(self, obj)
 
-    obj.map._pure_mapdict_read_storage = _pure_mapdict_read_storage
+    monkeypatch.setattr(PlainAttribute, "_pure_read", _pure_read)
     monkeypatch.setattr(jit, "isconstant", lambda c: True)
 
     assert obj.getdictvalue(space, "a") == 10
@@ -133,16 +137,20 @@
     assert indices == [0, 0]
 
     obj2 = cls.instantiate()
-    obj2.setdictvalue(space, "a", 15)
-    obj2.setdictvalue(space, "b", 25)
+    obj2.setdictvalue(space, "a", W_IntObject(15))
+    obj2.setdictvalue(space, "b", W_IntObject(25))
+    mutcella, mutcellb = obj2.storage
     assert obj2.map is obj.map
     assert obj2.map.ever_mutated == True
     assert obj2.map.back.ever_mutated == False
 
     # mutating obj2 changes the map
-    obj2.setdictvalue(space, "a", 50)
+    obj2.setdictvalue(space, "a", W_IntObject(50))
     assert obj2.map.back.ever_mutated == True
     assert obj2.map is obj.map
+    assert obj2.storage[0] is mutcella
+    assert obj2.storage[1] is mutcellb
+
 
 def test_attr_immutability_delete():
     cls = Class()
@@ -154,6 +162,21 @@
     assert obj.map.ever_mutated == True
     assert obj.map is map1
 
+def test_immutable_with_mutcell():
+    # even an immutable attribute will be stored as a mutcell. The reason is
+    # that then the type of the attribute is more predictable (eg always
+    # IntMutableCell and sometimes IntMutableCell and sometimes W_IntObject)
+    from pypy.objspace.std.intobject import W_IntObject
+    cls = Class()
+    obj = cls.instantiate()
+    # make sure the attribute counts as mutable
+    obj.setdictvalue(space, "a", W_IntObject(4))
+    # not wrapped because of the FakeSpace :-(
+    assert obj.getdictvalue(space, "a") == 4
+    mutcell = obj._mapdict_read_storage(0)
+    assert mutcell.intvalue == 4
+
+
 def test_mutcell_not_immutable():
     from pypy.objspace.std.intobject import W_IntObject
     cls = Class()
@@ -211,19 +234,6 @@
     assert mutcell2 is mutcell1
 
 
-def test_no_mutcell_if_immutable():
-    # don't introduce an immutable cell if the attribute seems immutable
-    from pypy.objspace.std.intobject import W_IntObject
-    cls = Class()
-    obj = cls.instantiate()
-    obj.setdictvalue(space, "a", W_IntObject(5))
-    assert not obj.map.ever_mutated
-
-    assert obj.getdictvalue(space, "a").intval == 5
-    mutcell = obj._mapdict_read_storage(0)
-    assert mutcell.intval == 5
-
-
 def test_mutcell_unwrap_only_if_needed():
     from pypy.objspace.std.intobject import W_IntObject
     cls = Class()


More information about the pypy-commit mailing list