From 7c39bca31bf4c83e67d687c273b0443884a61c42 Mon Sep 17 00:00:00 2001 From: Carl Friedrich Bolz-Tereick Date: Thu, 28 Jan 2021 15:20:43 +0100 Subject: fix test_unbox_reorder_bug3 also rip out size prediction: - with the JIT it's a lot less useful - it was always a mess with inlined fields - unboxing makes it even less clear --- pypy/objspace/std/mapdict.py | 46 ++++++++++++---------------------- pypy/objspace/std/test/test_mapdict.py | 21 ---------------- 2 files changed, 16 insertions(+), 51 deletions(-) (limited to 'pypy') diff --git a/pypy/objspace/std/mapdict.py b/pypy/objspace/std/mapdict.py index 4763452a22..20cfe42d12 100644 --- a/pypy/objspace/std/mapdict.py +++ b/pypy/objspace/std/mapdict.py @@ -36,7 +36,6 @@ LIMIT_MAP_ATTRIBUTES = 80 class AbstractAttribute(object): _immutable_fields_ = ['terminator'] cache_attrs = None - _size_estimate = 0 def __init__(self, space, terminator): self.space = space @@ -131,10 +130,6 @@ class AbstractAttribute(object): def set_terminator(self, obj, terminator): raise NotImplementedError("abstract base class") - @jit.elidable - def size_estimate(self): - return self._size_estimate >> NUM_DIGITS - def search(self, attrtype): return None @@ -152,14 +147,6 @@ class AbstractAttribute(object): def add_attr(self, obj, name, attrkind, w_value): space = self.space self._reorder_and_add(obj, name, attrkind, w_value) - if not jit.we_are_jitted(): - oldattr = self - attr = obj._get_mapdict_map() - size_est = (oldattr._size_estimate + attr.size_estimate() - - oldattr.size_estimate()) - assert size_est >= (oldattr.storage_needed() * NUM_DIGITS_POW2) - oldattr._size_estimate = size_est - @jit.elidable def _find_branch_to_move_into(self, name, attrkind, unbox_type): @@ -186,10 +173,6 @@ class AbstractAttribute(object): current_order = current.order current = current.back - @jit.look_inside_iff(lambda self, obj, name, attrkind, w_value: - jit.isconstant(self) and - jit.isconstant(name) and - jit.isconstant(attrkind)) def _reorder_and_add(self, obj, name, attrkind, w_value): # the idea is as follows: the subtrees of any map are ordered by # insertion. the invariant is that subtrees that are inserted later @@ -384,7 +367,6 @@ class PlainAttribute(AbstractAttribute): self.storageindex = back.storage_needed() self._num_attributes = back.num_attributes() + 1 self.back = back - self._size_estimate = self.storage_needed() * NUM_DIGITS_POW2 self.ever_mutated = False self.order = order @@ -491,7 +473,6 @@ class UnboxedPlainAttribute(PlainAttribute): self._compute_storageindex_listindex() self._num_attributes = back.num_attributes() + 1 self.typ = typ - self._size_estimate = self.storage_needed() * NUM_DIGITS_POW2 def _compute_storageindex_listindex(self): attr = self.back @@ -586,15 +567,16 @@ class UnboxedPlainAttribute(PlainAttribute): obj._mapdict_write_storage(self.storageindex, unboxed) else: unboxed = unerase_unboxed(obj._mapdict_read_storage(self.storageindex)) + + obj._set_mapdict_map(self) if len(unboxed) <= self.listindex: # size can only increase by 1 assert len(unboxed) == self.listindex unboxed = unboxed + [val] + obj._mapdict_write_storage(self.storageindex, erase_unboxed(unboxed)) else: - # the box is already large enough, due to reordering + # the unboxed list is already large enough, due to reordering unboxed[self.listindex] = val - obj._mapdict_write_storage(self.storageindex, erase_unboxed(unboxed)) - obj._set_mapdict_map(self) def repr(self): return "" % ( @@ -813,7 +795,7 @@ class MapdictStorageMixin(object): def _mapdict_init_empty(self, map): from rpython.rlib.debug import make_sure_not_resized self.map = map - self.storage = make_sure_not_resized([erase_item(None)] * map.size_estimate()) + self.storage = make_sure_not_resized([]) def _mapdict_read_storage(self, storageindex): assert storageindex >= 0 @@ -830,7 +812,7 @@ class MapdictStorageMixin(object): def _set_mapdict_increase_storage(self, map, value): """ increase storage size, adding value """ len_storage = len(self.storage) - new_storage = self.storage + [erase_item(None)] * (map.size_estimate() - len_storage) + new_storage = self.storage + [erase_item(None)] * (map.storage_needed() - len_storage) new_storage[len_storage] = value self.map = map self.storage = new_storage @@ -869,6 +851,10 @@ def _make_storage_mixin_size_n(n=SUBCLASSES_NUM_FIELDS): def _get_mapdict_map(self): return jit.promote(self.map) def _set_mapdict_map(self, map): + if self._has_storage_list() and map.storage_needed() <= n: + # weird corner case interacting with unboxing, see test_unbox_reorder_bug3 + if map.storage_needed() == n: + setattr(self, valnmin1, self._mapdict_get_storage_list()[0]) self.map = map def _mapdict_init_empty(self, map): for i in rangen: @@ -890,7 +876,7 @@ def _make_storage_mixin_size_n(n=SUBCLASSES_NUM_FIELDS): return getattr(self, "_value%s" % i) if self._has_storage_list(): return self._mapdict_get_storage_list()[storageindex - nmin1] - erased = getattr(self, "_value%s" % nmin1) + erased = getattr(self, valnmin1) return erased def _mapdict_write_storage(self, storageindex, value): @@ -903,7 +889,7 @@ def _make_storage_mixin_size_n(n=SUBCLASSES_NUM_FIELDS): if self._has_storage_list(): self._mapdict_get_storage_list()[storageindex - nmin1] = value return - setattr(self, "_value%s" % nmin1, value) + setattr(self, valnmin1, value) def _mapdict_storage_length(self): if self._has_storage_list(): @@ -939,14 +925,14 @@ def _make_storage_mixin_size_n(n=SUBCLASSES_NUM_FIELDS): setattr(self, "_value%s" % nmin1, erased) def _set_mapdict_increase_storage(self, map, value): - len_storage = self.map.storage_needed() - if len_storage == n: + storage_needed = map.storage_needed() + if self.map.storage_needed() == n: erased = getattr(self, "_value%s" % nmin1) new_storage = [erased, value] else: - new_storage = [erase_item(None)] * (map.size_estimate() - self._mapdict_storage_length()) + new_storage = [erase_item(None)] * (storage_needed - self._mapdict_storage_length()) new_storage = self._mapdict_get_storage_list() + new_storage - new_storage[len_storage - nmin1] = value + new_storage[storage_needed - n] = value self.map = map erased = erase_list(new_storage) setattr(self, "_value%s" % nmin1, erased) diff --git a/pypy/objspace/std/test/test_mapdict.py b/pypy/objspace/std/test/test_mapdict.py index 48e3f44656..6cf363fcdb 100644 --- a/pypy/objspace/std/test/test_mapdict.py +++ b/pypy/objspace/std/test/test_mapdict.py @@ -535,27 +535,6 @@ def test_materialize_r_dict(): assert obj.checkstorage == [50, 60, 70, w_d] -def test_size_prediction(): - for i in range(10): - c = Class() - assert c.terminator.size_estimate() == 0 - for j in range(1000): - obj = c.instantiate() - for a in "abcdefghij"[:i]: - obj.setdictvalue(space, a, 50) - assert c.terminator.size_estimate() == i - for i in range(1, 10): - c = Class() - assert c.terminator.size_estimate() == 0 - for j in range(1000): - obj = c.instantiate() - for a in "abcdefghij"[:i]: - obj.setdictvalue(space, a, 50) - obj = c.instantiate() - for a in "klmnopqars": - obj.setdictvalue(space, a, 50) - assert c.terminator.size_estimate() in [(i + 10) // 2, (i + 11) // 2] - # ___________________________________________________________ # unboxed tests -- cgit v1.2.3-65-gdbad