Commits

Armin Rigo  committed 5c08e05

Tests and fixes for heapcache.py. The issue is that sometimes the
'heap_cache' is cleared but not the 'new_boxes'. This causes a
problem explained in the test's comments. Same issue with LABELS,
causing 'reset(reset_virtuals=False)'.

  • Participants
  • Parent commits 6f0d0e5

Comments (0)

Files changed (3)

File rpython/jit/metainterp/heapcache.py

         # escaped the trace or not (True means the box never escaped, False
         # means it did escape), its presences in the mapping shows that it was
         # allocated inside the trace
+        self.new_boxes = {}
         if reset_virtuals:
-            self.new_boxes = {}
+            self.likely_virtuals = {}      # only for jit.isvirtual()
         # Tracks which boxes should be marked as escaped when the key box
         # escapes.
         self.dependencies = {}
             if unescaped:
                 self.new_boxes[box] = False
         try:
+            del self.likely_virtuals[box]
+        except KeyError:
+            pass
+        try:
             deps = self.dependencies.pop(box)
         except KeyError:
             pass
             opnum == rop.SETARRAYITEM_RAW or
             opnum == rop.SETINTERIORFIELD_GC or
             opnum == rop.COPYSTRCONTENT or
-            opnum == rop.COPYUNICODECONTENT):
+            opnum == rop.COPYUNICODECONTENT or
+            opnum == rop.STRSETITEM or
+            opnum == rop.UNICODESETITEM or
+            opnum == rop.SETFIELD_RAW or
+            opnum == rop.SETARRAYITEM_RAW or
+            opnum == rop.SETINTERIORFIELD_RAW or
+            opnum == rop.RAW_STORE):
             return
         if (rop._OVF_FIRST <= opnum <= rop._OVF_LAST or
             rop._NOSIDEEFFECT_FIRST <= opnum <= rop._NOSIDEEFFECT_LAST or
                                 if not self.is_unescaped(frombox):
                                     del cache[frombox]
                     return
-            else:
-                # Only invalidate things that are either escaped or arguments
-                for descr, boxes in self.heap_cache.iteritems():
-                    for box in boxes.keys():
-                        if not self.is_unescaped(box) or box in argboxes:
-                            del boxes[box]
-                for descr, indices in self.heap_array_cache.iteritems():
-                    for boxes in indices.itervalues():
-                        for box in boxes.keys():
-                            if not self.is_unescaped(box) or box in argboxes:
-                                del boxes[box]
-                return
 
-        # XXX when is it useful to clear() the complete dictionaries?
-        # isn't it enough in all cases to do the same as the two
-        # loops just above?
-        self.heap_cache.clear()
-        self.heap_array_cache.clear()
+        # Only invalidate things that are either escaped or arguments
+        for descr, boxes in self.heap_cache.iteritems():
+            for box in boxes.keys():
+                if not self.is_unescaped(box) or box in argboxes:
+                    del boxes[box]
+        for descr, indices in self.heap_array_cache.iteritems():
+            for boxes in indices.itervalues():
+                for box in boxes.keys():
+                    if not self.is_unescaped(box) or box in argboxes:
+                        del boxes[box]
 
     def is_class_known(self, box):
         return box in self.known_class_boxes
     def is_unescaped(self, box):
         return self.new_boxes.get(box, False)
 
+    def is_likely_virtual(self, box):
+        return box in self.likely_virtuals
+
     def new(self, box):
         self.new_boxes[box] = True
+        self.likely_virtuals[box] = None
 
     def new_array(self, box, lengthbox):
         self.new(box)

File rpython/jit/metainterp/pyjitpl.py

 
     @arguments("box")
     def _opimpl_isvirtual(self, box):
-        return ConstInt(self.metainterp.heapcache.is_unescaped(box))
+        return ConstInt(self.metainterp.heapcache.is_likely_virtual(box))
 
     opimpl_ref_isvirtual = _opimpl_isvirtual
 

File rpython/jit/metainterp/test/test_heapcache.py

             []
         )
         assert h.getarrayitem(box1, index1, descr1) is box3
+
+    def test_bug_missing_ignored_operations(self):
+        h = HeapCache()
+        h.new(box1)
+        h.new(box2)
+        h.setfield(box1, box2, descr1)
+        assert h.getfield(box1, descr1) is box2
+        h.invalidate_caches(rop.STRSETITEM, None, [])
+        h.invalidate_caches(rop.UNICODESETITEM, None, [])
+        h.invalidate_caches(rop.SETFIELD_RAW, None, [])
+        h.invalidate_caches(rop.SETARRAYITEM_RAW, None, [])
+        h.invalidate_caches(rop.SETINTERIORFIELD_RAW, None, [])
+        h.invalidate_caches(rop.RAW_STORE, None, [])
+        assert h.is_unescaped(box1)
+        assert h.is_unescaped(box2)
+        assert h.getfield(box1, descr1) is box2
+
+    def test_bug_heap_cache_is_cleared_but_not_is_unescaped(self):
+        # bug if only the getfield() link is cleared (heap_cache) but not
+        # the is_unescaped() flags: we can do later a GETFIELD(box1) which
+        # will give us a fresh box3, which is actually equal to box2.  This
+        # box3 is escaped, but box2 is still unescaped.  Bug shown e.g. by
+        # calling some residual code that changes the values on box3: then
+        # the content of box2 is still cached at the old value.
+        h = HeapCache()
+        h.new(box1)
+        h.new(box2)
+        h.setfield(box1, box2, descr1)
+        h.invalidate_caches(rop.SETFIELD_GC, None, [box1, box2])
+        assert h.getfield(box1, descr1) is box2
+        h.invalidate_caches(rop.CALL_MAY_FORCE, None, [])
+        assert h.is_unescaped(box1)
+        assert h.is_unescaped(box2)
+        assert h.getfield(box1, descr1) is box2
+
+    def test_is_likely_virtual(self):
+        h = HeapCache()
+        h.new(box1)
+        assert h.is_unescaped(box1)
+        assert h.is_likely_virtual(box1)
+        h.reset(reset_virtuals=False)
+        assert not h.is_unescaped(box1)
+        assert h.is_likely_virtual(box1)
+        h._escape(box1)
+        assert not h.is_unescaped(box1)
+        assert not h.is_likely_virtual(box1)