[pypy-svn] pypy default: Refactor heap.py, encapsulating in a new class CachedField the

arigo commits-noreply at bitbucket.org
Thu Mar 24 17:56:05 CET 2011


Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r42904:4116744473d4
Date: 2011-03-24 17:55 +0100
http://bitbucket.org/pypy/pypy/changeset/4116744473d4/

Log:	Refactor heap.py, encapsulating in a new class CachedField the exact
	states it can be in, and writing a long comment about it.

	This refactoring is actually giving benefits, by removing some extra
	setfields. To avoid breaking the original purpose of some of the
	tests which fail because they don't see the setfield any more, "fix"
	them by adding an escape() operation.

diff --git a/pypy/jit/metainterp/test/test_optimizeopt.py b/pypy/jit/metainterp/test/test_optimizeopt.py
--- a/pypy/jit/metainterp/test/test_optimizeopt.py
+++ b/pypy/jit/metainterp/test/test_optimizeopt.py
@@ -1791,7 +1791,7 @@
         """
         self.optimize_loop(ops, ops)
 
-    def test_duplicate_setfield_1(self):
+    def test_duplicate_setfield_0(self):
         ops = """
         [p1, i1, i2]
         setfield_gc(p1, i1, descr=valuedescr)
@@ -1800,8 +1800,27 @@
         """
         expected = """
         [p1, i1, i2]
+        jump(p1, i1, i2)
+        """
+        # in this case, all setfields are removed, because we can prove
+        # that in the loop it will always have the same value
+        self.optimize_loop(ops, expected)
+
+    def test_duplicate_setfield_1(self):
+        ops = """
+        [p1]
+        i1 = escape()
+        i2 = escape()
+        setfield_gc(p1, i1, descr=valuedescr)
         setfield_gc(p1, i2, descr=valuedescr)
-        jump(p1, i1, i2)
+        jump(p1)
+        """
+        expected = """
+        [p1]
+        i1 = escape()
+        i2 = escape()
+        setfield_gc(p1, i2, descr=valuedescr)
+        jump(p1)
         """
         self.optimize_loop(ops, expected)
 
@@ -1848,6 +1867,7 @@
         setfield_gc(p1, i4, descr=nextdescr)
         #
         setfield_gc(p1, i2, descr=valuedescr)
+        escape()
         jump(p1, i1, i2, p3)
         """
         preamble = """
@@ -1860,6 +1880,7 @@
         #
         setfield_gc(p1, i2, descr=valuedescr)
         setfield_gc(p1, i4, descr=nextdescr)
+        escape()
         jump(p1, i1, i2, p3, i3)
         """
         expected = """
@@ -1871,6 +1892,7 @@
         #
         setfield_gc(p1, i2, descr=valuedescr)
         setfield_gc(p1, i4, descr=nextdescr)
+        escape()
         jump(p1, i1, i2, p3, i3)
         """
         self.optimize_loop(ops, expected, preamble)
@@ -1943,6 +1965,7 @@
         guard_true(i3) []
         i4 = int_neg(i2)
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, i4)
         """
         preamble = """
@@ -1950,12 +1973,14 @@
         guard_true(i3) [p1]
         i4 = int_neg(i2)
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, i4)
         """
         expected = """
         [p1, i2, i4]
         guard_true(i4) [p1]
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, 1)
         """
         self.optimize_loop(ops, expected, preamble)
@@ -1969,6 +1994,7 @@
         guard_true(i3) []
         i4 = int_neg(i2)
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, i4)
         """
         preamble = """
@@ -1976,12 +2002,14 @@
         guard_true(i3) [i2, p1]
         i4 = int_neg(i2)
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, i4)
         """
         expected = """
         [p1, i2, i4]
         guard_true(i4) [i2, p1]
         setfield_gc(p1, NULL, descr=nextdescr)
+        escape()
         jump(p1, i2, 1)
         """
         self.optimize_loop(ops, expected)
@@ -2027,11 +2055,13 @@
         guard_value(p1, ConstPtr(myptr)) []
         setfield_gc(p1, i1, descr=valuedescr)
         setfield_gc(ConstPtr(myptr), i2, descr=valuedescr)
+        escape()
         jump(p1, i1, i2)
         """
         expected = """
         [i1, i2]
         setfield_gc(ConstPtr(myptr), i2, descr=valuedescr)
+        escape()
         jump(i1, i2)
         """
         self.optimize_loop(ops, expected)
@@ -3130,6 +3160,7 @@
         guard_no_exception(descr=fdescr) [p2, p1]
         virtual_ref_finish(p2, p1)
         setfield_gc(p0, NULL, descr=refdescr)
+        escape()
         jump(p0, i1)
         """
         preamble = """
@@ -3138,6 +3169,7 @@
         call(i1, descr=nonwritedescr)
         guard_no_exception(descr=fdescr) [i3, i1, p0]
         setfield_gc(p0, NULL, descr=refdescr)
+        escape()
         jump(p0, i1)
         """
         expected = """
@@ -3146,6 +3178,7 @@
         call(i1, descr=nonwritedescr)
         guard_no_exception(descr=fdescr2) [i3, i1, p0]
         setfield_gc(p0, NULL, descr=refdescr)
+        escape()
         jump(p0, i1)
         """
         self.optimize_loop(ops, expected, preamble)

diff --git a/pypy/jit/metainterp/optimizeopt/heap.py b/pypy/jit/metainterp/optimizeopt/heap.py
--- a/pypy/jit/metainterp/optimizeopt/heap.py
+++ b/pypy/jit/metainterp/optimizeopt/heap.py
@@ -3,8 +3,102 @@
 from pypy.jit.metainterp.resoperation import rop, ResOperation
 from pypy.rlib.objectmodel import we_are_translated
 from pypy.jit.metainterp.jitexc import JitException
+from pypy.jit.metainterp.optimizeopt.optimizer import Optimization
 
-from pypy.jit.metainterp.optimizeopt.optimizer import Optimization
+
+class CachedField(object):
+    def __init__(self):
+        # Cache information for a field descr.  It can be in one
+        # of two states:
+        #
+        #   1. 'cached_fields' is a dict mapping OptValues of structs
+        #      to OptValues of fields.  All fields on-heap are
+        #      synchronized with the values stored in the cache.
+        #
+        #   2. we just did one setfield, which is delayed (and thus
+        #      not synchronized).  'lazy_setfield' is the delayed
+        #      ResOperation.  In this state, 'cached_fields' contains
+        #      out-of-date information.  More precisely, the field
+        #      value pending in the ResOperation is *not* visible in
+        #      'cached_fields'.
+        #
+        self._cached_fields = {}
+        self._lazy_setfield = None
+        self._lazy_setfield_registered = False
+
+    def do_setfield(self, optheap, op):
+        # Update the state with the SETFIELD_GC operation 'op'.
+        structvalue = optheap.getvalue(op.getarg(0))
+        fieldvalue  = optheap.getvalue(op.getarg(1))
+        if self.possible_aliasing(optheap, structvalue):
+            self.force_lazy_setfield(optheap)
+            assert not self.possible_aliasing(optheap, structvalue)
+        cached_fieldvalue = self._cached_fields.get(structvalue, None)
+        if cached_fieldvalue is not fieldvalue:
+            # common case: store the 'op' as lazy_setfield, and register
+            # myself in the optheap's _lazy_setfields list
+            self._lazy_setfield = op
+            if not self._lazy_setfield_registered:
+                optheap._lazy_setfields.append(self)
+                self._lazy_setfield_registered = True
+        else:
+            # this is the case where the pending setfield ends up
+            # storing precisely the value that is already there,
+            # as proved by 'cached_fields'.  In this case, we don't
+            # need any _lazy_setfield: the heap value is already right.
+            # Note that this may reset to None a non-None lazy_setfield,
+            # cancelling its previous effects with no side effect.
+            self._lazy_setfield = None
+
+    def possible_aliasing(self, optheap, structvalue):
+        # If lazy_setfield is set and contains a setfield on a different
+        # structvalue, then we are annoyed, because it may point to either
+        # the same or a different structure at runtime.
+        return (self._lazy_setfield is not None
+                and (optheap.getvalue(self._lazy_setfield.getarg(0))
+                     is not structvalue))
+
+    def getfield_from_cache(self, optheap, structvalue):
+        # Returns the up-to-date field's value, or None if not cached.
+        if self.possible_aliasing(optheap, structvalue):
+            self.force_lazy_setfield(optheap)
+        if self._lazy_setfield is not None:
+            op = self._lazy_setfield
+            assert optheap.getvalue(op.getarg(0)) is structvalue
+            return optheap.getvalue(op.getarg(1))
+        else:
+            return self._cached_fields.get(structvalue, None)
+
+    def remember_field_value(self, structvalue, fieldvalue):
+        assert self._lazy_setfield is None
+        self._cached_fields[structvalue] = fieldvalue
+
+    def force_lazy_setfield(self, optheap):
+        op = self._lazy_setfield
+        if op is not None:
+            # This is the way _lazy_setfield is usually reset to None.
+            # Now we clear _cached_fields, because actually doing the
+            # setfield might impact any of the stored result (because of
+            # possible aliasing).
+            self._cached_fields.clear()
+            self._lazy_setfield = None
+            optheap.next_optimization.propagate_forward(op)
+            # Once it is done, we can put at least one piece of information
+            # back in the cache: the value of this particular structure's
+            # field.
+            structvalue = optheap.getvalue(op.getarg(0))
+            fieldvalue  = optheap.getvalue(op.getarg(1))
+            self.remember_field_value(structvalue, fieldvalue)
+
+    def get_reconstructed(self, optimizer, valuemap):
+        assert self._lazy_setfield is None
+        cf = CachedField()
+        for structvalue, fieldvalue in self._cached_fields.iteritems():
+            structvalue2 = structvalue.get_reconstructed(optimizer, valuemap)
+            fieldvalue2  = fieldvalue .get_reconstructed(optimizer, valuemap)
+            cf._cached_fields[structvalue2] = fieldvalue2
+        return cf
+
 
 class CachedArrayItems(object):
     def __init__(self):
@@ -20,40 +114,23 @@
     """Cache repeated heap accesses"""
     
     def __init__(self):
-        # cached fields:  {descr: {OptValue_instance: OptValue_fieldvalue}}
+        # cached fields:  {descr: CachedField}
         self.cached_fields = {}
-        self.known_heap_fields = {}
+        self._lazy_setfields = []
         # cached array items:  {descr: CachedArrayItems}
         self.cached_arrayitems = {}
-        # lazily written setfields (at most one per descr):  {descr: op}
-        self.lazy_setfields = {}
-        self.lazy_setfields_descrs = []     # keys (at least) of previous dict
 
     def reconstruct_for_next_iteration(self, optimizer, valuemap):
         new = OptHeap()
 
         if True:
             self.force_all_lazy_setfields()
-            assert not self.lazy_setfields_descrs
-            assert not self.lazy_setfields
         else:
-            new.lazy_setfields_descrs = self.lazy_setfields_descrs
-            new.lazy_setfields = self.lazy_setfields
+            assert 0   # was: new.lazy_setfields = self.lazy_setfields
         
         for descr, d in self.cached_fields.items():
-            newd = {}
-            new.cached_fields[descr] = newd
-            for value, fieldvalue in d.items():
-                newd[value.get_reconstructed(optimizer, valuemap)] = \
-                                       fieldvalue.get_reconstructed(optimizer, valuemap)
-            
-        for descr, d in self.known_heap_fields.items():
-            newd = {}
-            new.known_heap_fields[descr] = newd
-            for value, fieldvalue in d.items():
-                newd[value.get_reconstructed(optimizer, valuemap)] = \
-                                       fieldvalue.get_reconstructed(optimizer, valuemap)
-            
+            new.cached_fields[descr] = d.get_reconstructed(optimizer, valuemap)
+
         new.cached_arrayitems = {}
         for descr, d in self.cached_arrayitems.items():
             newd = {}
@@ -74,30 +151,16 @@
         return new
 
     def clean_caches(self):
+        del self._lazy_setfields[:]
         self.cached_fields.clear()
-        self.known_heap_fields.clear()
         self.cached_arrayitems.clear()
 
-    def cache_field_value(self, descr, value, fieldvalue, write=False):
-        if write:
-            # when seeing a setfield, we have to clear the cache for the same
-            # field on any other structure, just in case they are aliasing
-            # each other
-            d = self.cached_fields[descr] = {}
-        else:
-            d = self.cached_fields.setdefault(descr, {})
-        d[value] = fieldvalue
-
-    def read_cached_field(self, descr, value):
-        # XXX self.cached_fields and self.lazy_setfields should probably
-        # be merged somehow
-        d = self.cached_fields.get(descr, None)
-        if d is None:
-            op = self.lazy_setfields.get(descr, None)
-            if op is None:
-                return None
-            return self.getvalue(op.getarg(1))
-        return d.get(value, None)
+    def field_cache(self, descr):
+        try:
+            cf = self.cached_fields[descr]
+        except KeyError:
+            cf = self.cached_fields[descr] = CachedField()
+        return cf
 
     def cache_arrayitem_value(self, descr, value, indexvalue, fieldvalue, write=False):
         d = self.cached_arrayitems.get(descr, None)
@@ -179,8 +242,8 @@
                 for fielddescr in effectinfo.write_descrs_fields:
                     self.force_lazy_setfield(fielddescr)
                     try:
-                        del self.cached_fields[fielddescr]
-                        del self.known_heap_fields[fielddescr]
+                        cf = self.cached_fields[fielddescr]
+                        cf._cached_fields.clear()
                     except KeyError:
                         pass
                 for arraydescr in effectinfo.write_descrs_arrays:
@@ -205,58 +268,55 @@
         assert value.is_constant()
         newvalue = self.getvalue(value.box)
         if value is not newvalue:
-            for d in self.cached_fields.values():
-                if value in d:
-                    d[newvalue] = d[value]
-        # FIXME: Update the other caches too?
-        
-        
-    def force_lazy_setfield(self, descr, before_guard=False):
+            for cf in self.cached_fields.itervalues():
+                if value in cf._cached_fields:
+                    cf._cached_fields[newvalue] = cf._cached_fields[value]
+
+    def force_lazy_setfield(self, descr):
         try:
-            op = self.lazy_setfields[descr]
+            cf = self.cached_fields[descr]
         except KeyError:
             return
-        del self.lazy_setfields[descr]
-        value = self.getvalue(op.getarg(0))
-        fieldvalue = self.getvalue(op.getarg(1))
-        try:
-            heapvalue = self.known_heap_fields[op.getdescr()][value]
-            if fieldvalue is heapvalue:
-                return
-        except KeyError:
-            pass
-        self.next_optimization.propagate_forward(op)
+        cf.force_lazy_setfield(self)
 
+    def fixup_guard_situation(self):
         # hackish: reverse the order of the last two operations if it makes
         # sense to avoid a situation like "int_eq/setfield_gc/guard_true",
         # which the backend (at least the x86 backend) does not handle well.
         newoperations = self.optimizer.newoperations
-        if before_guard and len(newoperations) >= 2:
-            lastop = newoperations[-1]
-            prevop = newoperations[-2]
-            # - is_comparison() for cases like "int_eq/setfield_gc/guard_true"
-            # - CALL_MAY_FORCE: "call_may_force/setfield_gc/guard_not_forced"
-            # - is_ovf(): "int_add_ovf/setfield_gc/guard_no_overflow"
-            opnum = prevop.getopnum()
-            lastop_args = lastop.getarglist()
-            if ((prevop.is_comparison() or opnum == rop.CALL_MAY_FORCE
-                 or prevop.is_ovf())
-                and prevop.result not in lastop_args):
-                newoperations[-2] = lastop
-                newoperations[-1] = prevop
+        if len(newoperations) < 2:
+            return
+        lastop = newoperations[-1]
+        if lastop.getopnum() != rop.SETFIELD_GC:
+            return
+        # - is_comparison() for cases like "int_eq/setfield_gc/guard_true"
+        # - CALL_MAY_FORCE: "call_may_force/setfield_gc/guard_not_forced"
+        # - is_ovf(): "int_add_ovf/setfield_gc/guard_no_overflow"
+        prevop = newoperations[-2]
+        opnum = prevop.getopnum()
+        if not (prevop.is_comparison() or opnum == rop.CALL_MAY_FORCE
+                or prevop.is_ovf()):
+            return
+        if prevop.result in lastop.getarglist():
+            return
+        newoperations[-2] = lastop
+        newoperations[-1] = prevop
 
     def force_all_lazy_setfields(self):
-        if len(self.lazy_setfields_descrs) > 0:
-            for descr in self.lazy_setfields_descrs:
-                self.force_lazy_setfield(descr)
-            del self.lazy_setfields_descrs[:]
+        if len(self._lazy_setfields) > 0:
+            for cf in self._lazy_setfields:
+                if not we_are_translated():
+                    assert cf in self.cached_fields.values()
+                cf.force_lazy_setfield(self)
+            del self._lazy_setfields[:]
 
     def force_lazy_setfields_for_guard(self):
         pendingfields = []
-        for descr in self.lazy_setfields_descrs:
-            try:
-                op = self.lazy_setfields[descr]
-            except KeyError:
+        for cf in self._lazy_setfields:
+            if not we_are_translated():
+                assert cf in self.cached_fields.values()
+            op = cf._lazy_setfield
+            if op is None:
                 continue
             # the only really interesting case that we need to handle in the
             # guards' resume data is that of a virtual object that is stored
@@ -266,41 +326,27 @@
             fieldvalue = self.getvalue(op.getarg(1))
             if fieldvalue.is_virtual():
                 # this is the case that we leave to resume.py
-                pendingfields.append((descr, value.box,
+                pendingfields.append((op.getdescr(), value.box,
                                       fieldvalue.get_key_box()))
             else:
-                self.force_lazy_setfield(descr, before_guard=True)
+                cf.force_lazy_setfield(self)
+                self.fixup_guard_situation()
         return pendingfields
 
-    def force_lazy_setfield_if_necessary(self, op, value, write=False):
-        try:
-            op1 = self.lazy_setfields[op.getdescr()]
-        except KeyError:
-            if write:
-                self.lazy_setfields_descrs.append(op.getdescr())
-        else:
-            if self.getvalue(op1.getarg(0)) is not value:
-                self.force_lazy_setfield(op.getdescr())
-
     def optimize_GETFIELD_GC(self, op):
-        value = self.getvalue(op.getarg(0))
-        self.force_lazy_setfield_if_necessary(op, value)
-        # check if the field was read from another getfield_gc just before
-        # or has been written to recently
-        fieldvalue = self.read_cached_field(op.getdescr(), value)
+        structvalue = self.getvalue(op.getarg(0))
+        cf = self.field_cache(op.getdescr())
+        fieldvalue = cf.getfield_from_cache(self, structvalue)
         if fieldvalue is not None:
             self.make_equal_to(op.result, fieldvalue)
             return
         # default case: produce the operation
-        value.ensure_nonnull()
+        structvalue.ensure_nonnull()
         ###self.optimizer.optimize_default(op)
         self.emit_operation(op)
         # then remember the result of reading the field
         fieldvalue = self.getvalue(op.result)
-        self.cache_field_value(op.getdescr(), value, fieldvalue)
-        # keep track of what's on the heap
-        d = self.known_heap_fields.setdefault(op.getdescr(), {})
-        d[value] = fieldvalue
+        cf.remember_field_value(structvalue, fieldvalue)
 
     def optimize_SETFIELD_GC(self, op):
         if self.has_pure_result(rop.GETFIELD_GC_PURE, [op.getarg(0)],
@@ -309,14 +355,8 @@
                      (op.getdescr().repr_of_descr()))
             raise BogusPureField
         #
-        value = self.getvalue(op.getarg(0))
-        fieldvalue = self.getvalue(op.getarg(1))
-        cached_fieldvalue = self.read_cached_field(op.getdescr(), value)
-        if fieldvalue is not cached_fieldvalue:
-            self.force_lazy_setfield_if_necessary(op, value, write=True)
-            self.lazy_setfields[op.getdescr()] = op
-            # remember the result of future reads of the field
-            self.cache_field_value(op.getdescr(), value, fieldvalue, write=True)
+        cf = self.field_cache(op.getdescr())
+        cf.do_setfield(self, op)
 
     def optimize_GETARRAYITEM_GC(self, op):
         value = self.getvalue(op.getarg(0))


More information about the Pypy-commit mailing list