[pypy-commit] pypy default: (cfbolz, arigo)
arigo
noreply at buildbot.pypy.org
Tue May 17 12:48:47 CEST 2011
Author: Armin Rigo <arigo at tunes.org>
Branch:
Changeset: r44235:8624cde59095
Date: 2011-05-17 12:57 +0200
http://bitbucket.org/pypy/pypy/changeset/8624cde59095/
Log: (cfbolz, arigo)
Decided that this code is too brittle. We found two kinds of bugs
in it: bugs that were already there, and bugs that arose because of
TypeCells.
* for the 1st kind of bug: just because the map of an object doesn't
change does not mean that getdictvalue() returns from the same spot
(case of DevolvedDictTerminator); and in that case, getdictvalue()
can actually have random side-effects like changing version_tag.
* for the 2nd kind of bug: the point is that w_type.lookup() can now
return a different result without the version_tag changing.
diff --git a/pypy/interpreter/pycode.py b/pypy/interpreter/pycode.py
--- a/pypy/interpreter/pycode.py
+++ b/pypy/interpreter/pycode.py
@@ -116,10 +116,6 @@
self._compute_flatcall()
- if self.space.config.objspace.std.withmapdict:
- from pypy.objspace.std.mapdict import init_mapdict_cache
- init_mapdict_cache(self)
-
def _freeze_(self):
if (self.magic == cpython_magic and
'__pypy__' not in sys.builtin_module_names):
diff --git a/pypy/interpreter/pyopcode.py b/pypy/interpreter/pyopcode.py
--- a/pypy/interpreter/pyopcode.py
+++ b/pypy/interpreter/pyopcode.py
@@ -723,13 +723,8 @@
def LOAD_ATTR(self, nameindex, next_instr):
"obj.attributename"
w_obj = self.popvalue()
- if (self.space.config.objspace.std.withmapdict
- and not jit.we_are_jitted()):
- from pypy.objspace.std.mapdict import LOAD_ATTR_caching
- w_value = LOAD_ATTR_caching(self.getcode(), w_obj, nameindex)
- else:
- w_attributename = self.getname_w(nameindex)
- w_value = self.space.getattr(w_obj, w_attributename)
+ w_attributename = self.getname_w(nameindex)
+ w_value = self.space.getattr(w_obj, w_attributename)
self.pushvalue(w_value)
LOAD_ATTR._always_inline_ = True
diff --git a/pypy/objspace/std/callmethod.py b/pypy/objspace/std/callmethod.py
--- a/pypy/objspace/std/callmethod.py
+++ b/pypy/objspace/std/callmethod.py
@@ -13,8 +13,6 @@
from pypy.interpreter import function
from pypy.objspace.descroperation import object_getattribute
from pypy.rlib import jit, rstack # for resume points
-from pypy.objspace.std.mapdict import LOOKUP_METHOD_mapdict, \
- LOOKUP_METHOD_mapdict_fill_cache_method
# This module exports two extra methods for StdObjSpaceFrame implementing
@@ -33,13 +31,6 @@
#
space = f.space
w_obj = f.popvalue()
-
- if space.config.objspace.std.withmapdict and not jit.we_are_jitted():
- # mapdict has an extra-fast version of this function
- from pypy.objspace.std.mapdict import LOOKUP_METHOD_mapdict
- if LOOKUP_METHOD_mapdict(f, nameindex, w_obj):
- return
-
w_name = f.getname_w(nameindex)
w_value = None
@@ -60,11 +51,6 @@
# nothing in the instance
f.pushvalue(w_descr)
f.pushvalue(w_obj)
- if (space.config.objspace.std.withmapdict and
- not jit.we_are_jitted()):
- # let mapdict cache stuff
- LOOKUP_METHOD_mapdict_fill_cache_method(
- f.getcode(), nameindex, w_obj, w_type, w_descr)
return
if w_value is None:
w_value = space.getattr(w_obj, w_name)
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
@@ -683,118 +683,3 @@
w_attr = self.space.wrap(attr)
return w_attr, self.w_obj.getdictvalue(self.space, attr)
return None, None
-
-# ____________________________________________________________
-# Magic caching
-
-class CacheEntry(object):
- version_tag = None
- index = 0
- w_method = None # for callmethod
- success_counter = 0
- failure_counter = 0
-
- def is_valid_for_obj(self, w_obj):
- map = w_obj._get_mapdict_map()
- return self.is_valid_for_map(map)
-
- @jit.dont_look_inside
- def is_valid_for_map(self, map):
- # note that 'map' can be None here
- mymap = self.map_wref()
- if mymap is not None and mymap is map:
- version_tag = map.terminator.w_cls.version_tag()
- if version_tag is self.version_tag:
- # everything matches, it's incredibly fast
- if map.space.config.objspace.std.withmethodcachecounter:
- self.success_counter += 1
- return True
- return False
-
-_invalid_cache_entry_map = objectmodel.instantiate(AbstractAttribute)
-_invalid_cache_entry_map.terminator = None
-INVALID_CACHE_ENTRY = CacheEntry()
-INVALID_CACHE_ENTRY.map_wref = weakref.ref(_invalid_cache_entry_map)
- # different from any real map ^^^
-
-def init_mapdict_cache(pycode):
- num_entries = len(pycode.co_names_w)
- pycode._mapdict_caches = [INVALID_CACHE_ENTRY] * num_entries
-
- at jit.dont_look_inside
-def _fill_cache(pycode, nameindex, map, version_tag, index, w_method=None):
- entry = pycode._mapdict_caches[nameindex]
- if entry is INVALID_CACHE_ENTRY:
- entry = CacheEntry()
- pycode._mapdict_caches[nameindex] = entry
- entry.map_wref = weakref.ref(map)
- entry.version_tag = version_tag
- entry.index = index
- entry.w_method = w_method
- if pycode.space.config.objspace.std.withmethodcachecounter:
- entry.failure_counter += 1
-
-def LOAD_ATTR_caching(pycode, w_obj, nameindex):
- # this whole mess is to make the interpreter quite a bit faster; it's not
- # used if we_are_jitted().
- entry = pycode._mapdict_caches[nameindex]
- map = w_obj._get_mapdict_map()
- if entry.is_valid_for_map(map) and entry.w_method is None:
- # everything matches, it's incredibly fast
- return w_obj._mapdict_read_storage(entry.index)
- return LOAD_ATTR_slowpath(pycode, w_obj, nameindex, map)
-LOAD_ATTR_caching._always_inline_ = True
-
-def LOAD_ATTR_slowpath(pycode, w_obj, nameindex, map):
- space = pycode.space
- w_name = pycode.co_names_w[nameindex]
- if map is not None:
- w_type = map.terminator.w_cls
- w_descr = w_type.getattribute_if_not_from_object()
- if w_descr is not None:
- return space._handle_getattribute(w_descr, w_obj, w_name)
-
- version_tag = w_type.version_tag()
- if version_tag is not None:
- name = space.str_w(w_name)
- w_descr = w_type.lookup(name)
- selector = ("", INVALID)
- if w_descr is not None and space.is_data_descr(w_descr):
- from pypy.interpreter.typedef import Member
- descr = space.interpclass_w(w_descr)
- if isinstance(descr, Member):
- selector = ("slot", SLOTS_STARTING_FROM + descr.index)
- else:
- selector = (name, DICT)
- if selector[1] != INVALID:
- index = map.index(selector)
- if index >= 0:
- # note that if map.terminator is a DevolvedDictTerminator,
- # map.index() will always return -1 if selector[1]==DICT
- _fill_cache(pycode, nameindex, map, version_tag, index)
- return w_obj._mapdict_read_storage(index)
- if space.config.objspace.std.withmethodcachecounter:
- INVALID_CACHE_ENTRY.failure_counter += 1
- return space.getattr(w_obj, w_name)
-LOAD_ATTR_slowpath._dont_inline_ = True
-
-def LOOKUP_METHOD_mapdict(f, nameindex, w_obj):
- space = f.space
- pycode = f.getcode()
- entry = pycode._mapdict_caches[nameindex]
- if entry.is_valid_for_obj(w_obj):
- w_method = entry.w_method
- if w_method is not None:
- f.pushvalue(w_method)
- f.pushvalue(w_obj)
- return True
- return False
-
-def LOOKUP_METHOD_mapdict_fill_cache_method(pycode, nameindex, w_obj, w_type, w_method):
- version_tag = w_type.version_tag()
- if version_tag is None:
- return
- map = w_obj._get_mapdict_map()
- if map is None or isinstance(map.terminator, DevolvedDictTerminator):
- return
- _fill_cache(pycode, nameindex, map, version_tag, -1, w_method)
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
@@ -604,274 +604,6 @@
assert a.__dict__ is d
assert isinstance(a, B)
-
-class AppTestWithMapDictAndCounters(object):
- def setup_class(cls):
- from pypy.interpreter import gateway
- cls.space = gettestobjspace(
- **{"objspace.std.withmapdict": True,
- "objspace.std.withmethodcachecounter": True,
- "objspace.opcodes.CALL_METHOD": True})
- #
- def check(space, w_func, name):
- w_code = space.getattr(w_func, space.wrap('func_code'))
- nameindex = map(space.str_w, w_code.co_names_w).index(name)
- entry = w_code._mapdict_caches[nameindex]
- entry.failure_counter = 0
- entry.success_counter = 0
- INVALID_CACHE_ENTRY.failure_counter = 0
- #
- w_res = space.call_function(w_func)
- assert space.eq_w(w_res, space.wrap(42))
- #
- entry = w_code._mapdict_caches[nameindex]
- if entry is INVALID_CACHE_ENTRY:
- failures = successes = 0
- else:
- failures = entry.failure_counter
- successes = entry.success_counter
- globalfailures = INVALID_CACHE_ENTRY.failure_counter
- return space.wrap((failures, successes, globalfailures))
- check.unwrap_spec = [gateway.ObjSpace, gateway.W_Root, str]
- cls.w_check = cls.space.wrap(gateway.interp2app(check))
-
- def test_simple(self):
- class A(object):
- pass
- a = A()
- 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)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
- #
- A.y = 5 # unrelated, but changes the version_tag
- 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)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
- #
- A.x = 8 # but shadowed by '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)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
-
- def test_property(self):
- class A(object):
- x = property(lambda self: 42)
- a = A()
- def f():
- return a.x
- #
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
-
- def test_slots(self):
- class A(object):
- __slots__ = ['x']
- a = A()
- 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)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
-
- def test_two_attributes(self):
- class A(object):
- pass
- a = A()
- a.x = 40
- a.y = -2
- def f():
- return a.x - a.y
- #
- 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)
- #
- res = self.check(f, 'y')
- assert res == (0, 1, 0)
- res = self.check(f, 'y')
- assert res == (0, 1, 0)
- res = self.check(f, 'y')
- assert res == (0, 1, 0)
-
- def test_two_maps(self):
- class A(object):
- pass
- a = A()
- 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)
- res = self.check(f, 'x')
- 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
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
- res = self.check(f, 'x')
- assert res == (0, 1, 0)
-
- def test_custom_metaclass(self):
- class A(object):
- class __metaclass__(type):
- pass
- a = A()
- 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)
-
- def test_old_style_base(self):
- class B:
- pass
- class C(object):
- pass
- class A(C, B):
- pass
- a = A()
- a.x = 42
- def f():
- return a.x
- #
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
- res = self.check(f, 'x')
- assert res == (0, 0, 1)
-
- def test_call_method_uses_cache(self):
- # bit sucky
- global C
-
- class C(object):
- def m(*args):
- return args
- C.sm = staticmethod(C.m.im_func)
- C.cm = classmethod(C.m.im_func)
-
- exec """if 1:
-
- def f():
- c = C()
- res = c.m(1)
- assert res == (c, 1)
- return 42
-
- def g():
- c = C()
- res = c.sm(1)
- assert res == (1, )
- return 42
-
- def h():
- c = C()
- res = c.cm(1)
- assert res == (C, 1)
- return 42
- """
- res = self.check(f, 'm')
- assert res == (1, 0, 0)
- res = self.check(f, 'm')
- assert res == (0, 1, 0)
- res = self.check(f, 'm')
- assert res == (0, 1, 0)
- res = self.check(f, 'm')
- assert res == (0, 1, 0)
-
- # static methods are not cached
- res = self.check(g, 'sm')
- assert res == (0, 0, 0)
- res = self.check(g, 'sm')
- assert res == (0, 0, 0)
-
- # neither are class methods
- res = self.check(h, 'cm')
- assert res == (0, 0, 0)
- res = self.check(h, 'cm')
- assert res == (0, 0, 0)
-
- def test_mix_cache_bug(self):
- # bit sucky
- global C
-
- class C(object):
- def m(*args):
- return args
-
- exec """if 1:
-
- def f():
- c = C()
- res = c.m(1)
- assert res == (c, 1)
- bm = c.m
- res = bm(1)
- assert res == (c, 1)
- return 42
-
- """
- res = self.check(f, 'm')
- assert res == (1, 1, 1)
- res = self.check(f, 'm')
- assert res == (0, 2, 1)
- res = self.check(f, 'm')
- assert res == (0, 2, 1)
- res = self.check(f, 'm')
- assert res == (0, 2, 1)
-
def test_dont_keep_class_alive(self):
import weakref
import gc
More information about the pypy-commit
mailing list