[pypy-commit] pypy guard-compatible: re-enable support for immutability tracking of attributes of user-defined

cfbolz pypy.commits at gmail.com
Wed Mar 21 08:02:50 EDT 2018


Author: Carl Friedrich Bolz-Tereick <cfbolz at gmx.de>
Branch: guard-compatible
Changeset: r94051:7825277c4910
Date: 2018-03-20 10:51 +0100
http://bitbucket.org/pypy/pypy/changeset/7825277c4910/

Log:	re-enable support for immutability tracking of attributes of user-
	defined instances. This works by piggybacking the elidability on the
	map's version.

	When an attribute goes from immutable to mutable (ie a lot during
	startup) this approach changes versions a bit too much: for all
	children of the terminator of the involved attribute, a new version
	is generated. Could be improved a little bit.

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
@@ -76,23 +76,22 @@
             # XXX can improve the devolved case
             terminator = self._get_terminator()
             return terminator._read_terminator(obj, name, index)
-        #if ( # XXX in the guard_compatible world the following isconstant may never be true?
-        #    jit.isconstant(attr.storageindex) and
-        #    jit.isconstant(obj) and
-        #    not attr.ever_mutated
-        #):
-        #    return self._pure_mapdict_read_storage(obj, attr.storageindex)
-        #else:
-        return obj._mapdict_read_storage(storageindex)
-
-    @jit.elidable
-    def _pure_mapdict_read_storage(self, obj, storageindex):
+        if (
+                jit.isconstant(name) and
+                jit.isconstant(index) and
+                jit.isconstant(obj)
+            ):
+            can_constfold = self.can_constfold_attr_read(name, index)
+            if can_constfold:
+                return _pure_mapdict_read_storage(obj, storageindex)
         return obj._mapdict_read_storage(storageindex)
 
     def write(self, obj, name, index, w_value):
         storageindex = self.find_map_storageindex(name, index)
         if storageindex < 0:
             return self._get_terminator()._write_terminator(obj, name, index, w_value)
+        if self.can_constfold_attr_read(name, index):
+            self.invalidate_ever_mutated(name, index)
         obj._mapdict_write_storage(storageindex, w_value)
         return True
 
@@ -112,6 +111,24 @@
             return NOATTR
         return attr.storageindex
 
+    @jit.elidable_compatible(quasi_immut_field_name_for_second_arg="version")
+    def can_constfold_attr_read(self, version, name, index):
+        if version is None:
+            return False
+        attr = self.find_map_attr(name, index)
+        if attr is None:
+            return False
+        return not attr.ever_mutated
+
+    def invalidate_ever_mutated(self, name, index):
+        attr = self.find_map_attr(name, index)
+        assert not attr.ever_mutated
+        attr.ever_mutated = True
+        # XXX is the next line too expensive?
+        # it's necessary for the version to change, to make
+        # can_constfold_attr_read actually elidable
+        self.terminator.mutated(self.version is not None)
+
     @jit.elidable_compatible()
     def find_map_attr(self, name, index):
         # attr cache
@@ -384,14 +401,14 @@
         self.w_cls = w_cls
         self.all_children = None
 
-    def mutated_w_cls_version(self, version):
-        if version is None:
+    def mutated(self, have_version):
+        if not have_version:
             self.version = None
         else:
             self.version = Version()
         if self.all_children is not None:
             for map in self.all_children:
-                if version is None:
+                if not have_version:
                     map.version = None
                 else:
                     map.version = Version()
@@ -437,9 +454,9 @@
         Terminator.__init__(self, space, w_cls)
         self.devolved_dict_terminator = DevolvedDictTerminator(space, w_cls)
 
-    def mutated_w_cls_version(self, version):
-        self.devolved_dict_terminator.mutated_w_cls_version(version)
-        Terminator.mutated_w_cls_version(self, version)
+    def mutated(self, have_version):
+        self.devolved_dict_terminator.mutated(have_version)
+        Terminator.mutated(self, have_version)
 
     def materialize_r_dict(self, space, obj, dict_w):
         return self._make_devolved(space)
@@ -500,7 +517,7 @@
         return Terminator.set_terminator(self, obj, terminator)
 
 class PlainAttribute(AbstractAttribute):
-    _immutable_fields_ = ['name', 'index', 'storageindex', 'back', 'ever_mutated?', 'order']
+    _immutable_fields_ = ['name', 'index', 'storageindex', 'back', 'ever_mutated', 'order']
 
     def __init__(self, name, index, back):
         AbstractAttribute.__init__(self, back.space, back.terminator)
@@ -509,7 +526,7 @@
         self.storageindex = back.length()
         self.back = back
         self._size_estimate = self.length() * NUM_DIGITS_POW2
-        #self.ever_mutated = False # XXX XXX XXX immutability is disabled for now
+        self.ever_mutated = False
         self.order = len(back.cache_attrs) if back.cache_attrs else 0
 
     def _copy_attr(self, obj, new_obj):
@@ -518,9 +535,9 @@
 
     def delete(self, obj, name, index):
         if index == self.index and name == self.name:
+            if not self.ever_mutated:
+                self.invalidate_ever_mutated(name, index)
             # ok, attribute is deleted
-            #if not self.ever_mutated:
-            #    self.ever_mutated = True
             return self.back.copy(obj)
         new_obj = self.back.delete(obj, name, index)
         if new_obj is not None:
@@ -602,6 +619,13 @@
 INVALID = 2
 SLOTS_STARTING_FROM = 3
 
+
+
+ at jit.elidable
+def _pure_mapdict_read_storage(obj, storageindex):
+    return obj._mapdict_read_storage(storageindex)
+
+
 # a little bit of a mess of mixin classes that implement various pieces of
 # objspace user object functionality in terms of mapdict
 
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
@@ -17,7 +17,7 @@
 class Class(object):
     def __init__(self, hasdict=True):
         self.hasdict = hasdict
-        self._version_tag = None
+        self._version_tag = object()
         if hasdict:
             self.terminator = DictTerminator(space, self)
         else:
@@ -33,6 +33,9 @@
         result.user_setup(sp, self)
         return result
 
+    def mutated(self, _):
+        self._version_tag = object()
+
 class ObjectWithoutDict(ObjectWithoutDict):
     class typedef:
         hasdict = False
@@ -291,7 +294,7 @@
 
 
 def test_attr_immutability(monkeypatch):
-    pytest.skip("disabled for now")
+    from pypy.objspace.std import mapdict
     cls = Class()
     obj = cls.instantiate()
     obj.setdictvalue(space, "a", 10)
@@ -308,7 +311,7 @@
         indices.append(storageindex)
         return obj._mapdict_read_storage(storageindex)
 
-    obj.map._pure_mapdict_read_storage = _pure_mapdict_read_storage
+    monkeypatch.setattr(mapdict, "_pure_mapdict_read_storage", _pure_mapdict_read_storage)
     monkeypatch.setattr(jit, "isconstant", lambda c: True)
 
     assert obj.getdictvalue(space, "a") == 10
@@ -329,7 +332,6 @@
     assert obj2.map is obj.map
 
 def test_attr_immutability_delete():
-    pytest.skip("disabled for now")
     cls = Class()
     obj = cls.instantiate()
     obj.setdictvalue(space, "a", 10)
@@ -1031,14 +1033,14 @@
         a.x = 42
         def f():
             return a.x
-        #
+
         res = self.check(f, 'x')
         assert res == (1, 0, 0)
         res = self.check(f, 'x')
         assert res == (0, 1, 0)
         res = self.check(f, 'x')
         assert res == (0, 1, 0)
-        #
+
         a.y = "foo"      # changes the map
         res = self.check(f, 'x')
         assert res == (1, 0, 0)
@@ -1046,8 +1048,18 @@
         assert res == (0, 1, 0)
         res = self.check(f, 'x')
         assert res == (0, 1, 0)
-        #
-        a.y = "bar"      # does not change the map any more
+
+        # the following does not change the map, but changes the version since
+        # y goes from immutable to mutable
+        a.y = "bar"
+        res = self.check(f, 'x')
+        assert res == (1, 0, 0)
+        res = self.check(f, 'x')
+        assert res == (0, 1, 0)
+        res = self.check(f, 'x')
+        assert res == (0, 1, 0)
+
+        a.y = "baz"      # now everything works
         res = self.check(f, 'x')
         assert res == (0, 1, 0)
         res = self.check(f, 'x')
diff --git a/pypy/objspace/std/typeobject.py b/pypy/objspace/std/typeobject.py
--- a/pypy/objspace/std/typeobject.py
+++ b/pypy/objspace/std/typeobject.py
@@ -255,7 +255,7 @@
 
     def _set_version_tag(self, version_tag):
         self._version_tag = version_tag
-        self.terminator.mutated_w_cls_version(version_tag)
+        self.terminator.mutated(version_tag is not None)
 
     def getattribute_if_not_from_object(self):
         """ this method returns the applevel __getattribute__ if that is not


More information about the pypy-commit mailing list