[pypy-commit] pypy value-profiling: proper integration between a field being known an instance of W_IntObject or

[cfbolz]

Author: Carl Friedrich Bolz <cfbolz at gmx.de>
Branch: value-profiling
Changeset: r81863:703d8c63a5f6
Date: 2016-01-16 07:20 +0100
http://bitbucket.org/pypy/pypy/changeset/703d8c63a5f6/

Log:	proper integration between a field being known an instance of
	W_IntObject or W_FloatObject

diff --git a/pypy/interpreter/valueprof.py b/pypy/interpreter/valueprof.py
--- a/pypy/interpreter/valueprof.py
+++ b/pypy/interpreter/valueprof.py
@@ -28,6 +28,22 @@
     def get_int_val(self, w_obj):
         raise NotImplementedError("abstract base")
 
+    def write_necessary(self, w_value):
+        status = self._vprof_status
+        if status == SEEN_TOO_MUCH:
+            return True
+        # we must have seen something already, because it only makes sense to
+        # call write_necessary if there is already a value there
+        assert not status == SEEN_NOTHING
+        if status == SEEN_CONSTANT_INT:
+            return (self.is_int(w_value) and
+                self.read_constant_int() != self.get_int_val(w_value))
+        elif status == SEEN_CONSTANT_OBJ:
+            prev_obj = self.try_read_constant_obj()
+            return prev_obj is not w_value
+        return True
+
+
     def see_write(self, w_value):
         """ inform the value profiler of a write. returns False, unless the
         value is known to be a constant, and w_value that constant (in that
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
@@ -34,6 +34,7 @@
 
     def read(self, obj, selector):
         from pypy.objspace.std.intobject import W_IntObject
+        from pypy.objspace.std.floatobject import W_FloatObject
         attr = self.find_map_attr(selector)
         if attr is None:
             return self.terminator._read_terminator(obj, selector)
@@ -54,21 +55,30 @@
         else:
             result = obj._mapdict_read_storage(attr.storageindex)
             if jit.we_are_jitted() and attr.class_is_known():
-                jit.record_exact_class(result, attr.read_constant_cls())
+                cls = attr.read_constant_cls()
+                if cls is W_IntObject:
+                    # this means that the class stored in the storage is an
+                    # IntMutableCell
+                    return W_IntObject(result.intvalue)
+                if cls is W_FloatObject:
+                    # ditto
+                    return W_FloatObject(result.floatvalue)
+                jit.record_exact_class(result, cls)
             return attr._read_cell(result)
 
     def write(self, obj, selector, w_value):
         attr = self.find_map_attr(selector)
         if attr is None:
             return self.terminator._write_terminator(obj, selector, w_value)
-        write_unnecessary = attr.see_write(w_value)
+        # if the write is not necessary, the storage is already filled from the
+        # time we did the map transition. Therefore, if the value profiler says
+        # so, we can not do the write
+        write_necessary = attr.write_necessary(w_value)
+        if not write_necessary:
+            return True
         if not attr.ever_mutated:
             attr.ever_mutated = True
-        # if write_unnecessary, the storage is already filled from the time we
-        # did the map transition. Therefore, if the value profiler says so, we
-        # can not do the write
-        if write_unnecessary:
-            return True
+        self.see_write(w_value)
         cell = obj._mapdict_read_storage(attr.storageindex)
         w_value = attr._write_cell(cell, w_value)
         if w_value is not None:
@@ -173,7 +183,6 @@
     def add_attr(self, obj, selector, w_value):
         # grumble, jit needs this
         attr = self._get_new_attr(selector[0], selector[1])
-        w_value = attr._write_cell(None, w_value)
         oldattr = obj._get_mapdict_map()
         if not jit.we_are_jitted():
             size_est = (oldattr._size_estimate + attr.size_estimate()
@@ -190,10 +199,11 @@
         # the order is important here: first change the map, then the storage,
         # for the benefit of the special subclasses
         obj._set_mapdict_map(attr)
+        # important to see the write of the original value, not the cell
+        attr.see_write(w_value)
         w_value = attr._write_cell(None, w_value)
         assert w_value is not None
         obj._mapdict_write_storage(attr.storageindex, w_value)
-        attr.see_write(w_value)
 
     def materialize_r_dict(self, space, obj, dict_w):
         raise NotImplementedError("abstract base class")
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
@@ -512,6 +512,29 @@
     assert obj.getdictvalue(space, "a") == a
     assert seen == [(a, Value), (a, Value)]
 
+def test_value_profiling_known_cls_mutcells(monkeypatch):
+    from pypy.objspace.std.intobject import W_IntObject
+    from pypy.objspace.std.floatobject import W_FloatObject
+    for cls, mutcls, val, attrname in [
+            (W_IntObject, IntMutableCell, 0, "intval"),
+            (W_FloatObject, FloatMutableCell, 0.0, "floatval")]:
+        a = cls(val)
+        a1 = cls(val + 1)
+        cls = Class()
+        obj = cls.instantiate()
+        obj.setdictvalue(space, "a", a1)
+        obj = cls.instantiate()
+        obj.setdictvalue(space, "a", a1)
+        obj.setdictvalue(space, "a", a)
+
+        def f(obj, cls):
+            assert False, "unreachable"
+        monkeypatch.setattr(jit, "we_are_jitted", lambda : True)
+        monkeypatch.setattr(jit, "record_exact_class", f)
+
+        assert getattr(obj.getdictvalue(space, "a"), attrname) == val
+        assert getattr(obj.getdictvalue(space, "a"), attrname) == val
+
 
 def test_value_profiling_elide_write(monkeypatch):
     monkeypatch.setattr(jit, "we_are_jitted", lambda : True)