From 8972d38749a799eadc5769fdb8f1e6f6d9cf1408 Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Sun, 12 Apr 2020 22:45:52 -0300 Subject: [PATCH 1/6] Add minimum error test case --- tests/test_issue24.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/test_issue24.py diff --git a/tests/test_issue24.py b/tests/test_issue24.py new file mode 100644 index 000000000..642de6418 --- /dev/null +++ b/tests/test_issue24.py @@ -0,0 +1,36 @@ +import unittest + + +from immutables.map import Map as PyMap + + +class Issue24Base: + Map = None + + def test_issue24(self): + keys = range(27) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + self.assertTrue(17 in m) + with m.mutate() as mm: + for i in keys: + del mm[i] + self.assertEqual(len(mm), 0) + + +try: + from immutables._map import Map as CMap +except ImportError: + CMap = None + + +class Issue24PyTest(Issue24Base, unittest.TestCase): + Map = PyMap + + +class Issue24CTest(Issue24Base, unittest.TestCase): + Map = CMap + + +if __name__ == '__main__': + unittest.main() From b1af1c64657b8d988b255f93212eda5f9fec0362 Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Wed, 15 Apr 2020 20:52:26 -0300 Subject: [PATCH 2/6] Create pure python implementation with Array Nodes Add tests to the implemantation and improve issue24 test --- immutables/map_with_array_nodes.py | 1000 ++++++++++++++++++++++++++++ tests/test_issue24.py | 24 +- tests/test_map_py_array.py | 18 + tests/test_map_py_array_stress.py | 15 + 4 files changed, 1053 insertions(+), 4 deletions(-) create mode 100644 immutables/map_with_array_nodes.py create mode 100644 tests/test_map_py_array.py create mode 100644 tests/test_map_py_array_stress.py diff --git a/immutables/map_with_array_nodes.py b/immutables/map_with_array_nodes.py new file mode 100644 index 000000000..782d53045 --- /dev/null +++ b/immutables/map_with_array_nodes.py @@ -0,0 +1,1000 @@ +import abc +import collections.abc +import enum +import itertools +import reprlib +import sys + + +__all__ = ('Map',) + + +# Thread-safe counter. +_mut_id = itertools.count(1).__next__ +HAMT_ARRAY_NODE_SIZE = 32 + +# Python version of _map.c. The topmost comment there explains +# all datastructures and algorithms. +# The code here follows C code closely on purpose to make +# debugging and testing easier. + + +def map_hash(o): + x = hash(o) + return (x & 0xffffffff) ^ ((x >> 32) & 0xffffffff) + + +def map_mask(hash, shift): + return (hash >> shift) & 0x01f + + +def map_bitpos(hash, shift): + return 1 << map_mask(hash, shift) + + +def map_bitcount(v): + v = v - ((v >> 1) & 0x55555555) + v = (v & 0x33333333) + ((v >> 2) & 0x33333333) + v = (v & 0x0F0F0F0F) + ((v >> 4) & 0x0F0F0F0F) + v = v + (v >> 8) + v = (v + (v >> 16)) & 0x3F + return v + + +def map_bitindex(bitmap, bit): + return map_bitcount(bitmap & (bit - 1)) + + +W_EMPTY, W_NEWNODE, W_NOT_FOUND = range(3) +void = object() + + +class BitmapNode: + + def __init__(self, size, bitmap, array, mutid): + self.size = size + self.bitmap = bitmap + assert isinstance(array, list) and len(array) == size + self.array = array + self.mutid = mutid + + def clone(self, mutid): + return BitmapNode(self.size, self.bitmap, self.array.copy(), mutid) + + def assoc(self, shift, hash, key, val, mutid): + bit = map_bitpos(hash, shift) + idx = map_bitindex(self.bitmap, bit) + + if self.bitmap & bit: + key_idx = 2 * idx + val_idx = key_idx + 1 + + key_or_null = self.array[key_idx] + val_or_node = self.array[val_idx] + + if key_or_null is None: # this doesn't work key may be None + sub_node, added = val_or_node.assoc( + shift + 5, hash, key, val, mutid) + if val_or_node is sub_node: + return self, added + + if mutid and mutid == self.mutid: + self.array[val_idx] = sub_node + return self, added + else: + ret = self.clone(mutid) + ret.array[val_idx] = sub_node + return ret, added + + if key == key_or_null: + if val is val_or_node: + return self, False + + if mutid and mutid == self.mutid: + self.array[val_idx] = val + return self, False + else: + ret = self.clone(mutid) + ret.array[val_idx] = val + return ret, False + + existing_key_hash = map_hash(key_or_null) + if existing_key_hash == hash: + sub_node = CollisionNode( + 4, hash, [key_or_null, val_or_node, key, val], mutid) + else: + sub_node = BitmapNode(0, 0, [], mutid) + sub_node, _ = sub_node.assoc( + shift + 5, existing_key_hash, + key_or_null, val_or_node, + mutid) + sub_node, _ = sub_node.assoc( + shift + 5, hash, key, val, + mutid) + + if mutid and mutid == self.mutid: + self.array[key_idx] = None + self.array[val_idx] = sub_node + return self, True + else: + ret = self.clone(mutid) + ret.array[key_idx] = None + ret.array[val_idx] = sub_node + return ret, True + + else: + n = map_bitcount(self.bitmap) + + if n >= 16: # look for this + jdx = map_mask(hash, shift) + + # error maybe here + empty = BitmapNode(0, 0, [], mutid) + new_array = [None] * HAMT_ARRAY_NODE_SIZE + + new_array[jdx], _ = empty.assoc( + shift + 5, hash, key, val, mutid) + j = 0 + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + if (self.bitmap >> i) & 1 != 0: + assert new_array[i] is None + if self.array[j] is None: + new_array[i] = self.array[j+1] + else: + rehash = map_hash(self.array[j]) + new_array[i], _ = empty.assoc( + shift + 5, rehash, + self.array[j], self.array[j+1], + mutid) + j += 2 + new_node = ArrayNode(n + 1, new_array, mutid) + if __debug__: + new_node._validate() + return new_node, True + else: + key_idx = 2 * idx + val_idx = key_idx + 1 + + new_array = self.array[:key_idx] + new_array.append(key) + new_array.append(val) + new_array.extend(self.array[key_idx:]) + + # if mutid and mutid == self.mutid: + # self.size = 2 * (n + 1) + # self.bitmap |= bit + # self.array = new_array + # return self, True + # this fails because of the array code + # else: + return BitmapNode( + 2 * (n + 1), self.bitmap | bit, new_array, mutid), True + + def find(self, shift, hash, key): + bit = map_bitpos(hash, shift) + + if not (self.bitmap & bit): + raise KeyError + + idx = map_bitindex(self.bitmap, bit) + key_idx = idx * 2 + val_idx = key_idx + 1 + + assert val_idx < self.size + + key_or_null = self.array[key_idx] + val_or_node = self.array[val_idx] + + if key_or_null is None: # key maybe None + return val_or_node.find(shift + 5, hash, key) + + if key == key_or_null: + return val_or_node + + raise KeyError(key) + + def without(self, shift, hash, key, mutid): + bit = map_bitpos(hash, shift) + if not (self.bitmap & bit): + return W_NOT_FOUND, None + + idx = map_bitindex(self.bitmap, bit) + key_idx = 2 * idx + val_idx = key_idx + 1 + + key_or_null = self.array[key_idx] + val_or_node = self.array[val_idx] + + if key_or_null is None: # key maybe None + res, sub_node = val_or_node.without(shift + 5, hash, key, mutid) + + if res is W_EMPTY: + raise RuntimeError('unreachable code') # pragma: no cover + + elif res is W_NEWNODE: + if (type(sub_node) is BitmapNode and + sub_node.size == 2 and + sub_node.array[0] is not None): + + if mutid and mutid == self.mutid: + self.array[key_idx] = sub_node.array[0] + self.array[val_idx] = sub_node.array[1] + return W_NEWNODE, self + else: + clone = self.clone(mutid) + clone.array[key_idx] = sub_node.array[0] + clone.array[val_idx] = sub_node.array[1] + return W_NEWNODE, clone + + if mutid and mutid == self.mutid: + self.array[val_idx] = sub_node + return W_NEWNODE, self + else: + clone = self.clone(mutid) + clone.array[val_idx] = sub_node + return W_NEWNODE, clone + + else: + assert sub_node is None + return res, None + + else: + if key == key_or_null: + if self.size == 2: + return W_EMPTY, None + + new_array = self.array[:key_idx] + new_array.extend(self.array[val_idx + 1:]) + + if mutid and mutid == self.mutid: + self.size -= 2 + self.bitmap &= ~bit + self.array = new_array + return W_NEWNODE, self + else: + new_node = BitmapNode( + self.size - 2, self.bitmap & ~bit, new_array, mutid) + return W_NEWNODE, new_node + + else: + return W_NOT_FOUND, None + + def keys(self): + for i in range(0, self.size, 2): + key_or_null = self.array[i] + + if key_or_null is None: + val_or_node = self.array[i + 1] + yield from val_or_node.keys() + else: + yield key_or_null + + def values(self): + for i in range(0, self.size, 2): + key_or_null = self.array[i] + val_or_node = self.array[i + 1] + + if key_or_null is None: + yield from val_or_node.values() + else: + yield val_or_node + + def items(self): + for i in range(0, self.size, 2): + key_or_null = self.array[i] + val_or_node = self.array[i + 1] + + if key_or_null is None: + yield from val_or_node.items() + else: + yield key_or_null, val_or_node + + def dump(self, buf, level): # pragma: no cover + buf.append( + ' ' * (level + 1) + + 'BitmapNode(size={} count={} bitmap={} id={:0x}):'.format( + self.size, self.size / 2, bin(self.bitmap), id(self))) + + for i in range(0, self.size, 2): + key_or_null = self.array[i] + val_or_node = self.array[i + 1] + + pad = ' ' * (level + 2) + + if key_or_null is None: + buf.append(pad + 'None:') + val_or_node.dump(buf, level + 2) + else: + buf.append(pad + '{!r}: {!r}'.format(key_or_null, val_or_node)) + + +class CollisionNode: + + def __init__(self, size, hash, array, mutid): + self.size = size + self.hash = hash + self.array = array + self.mutid = mutid + + def find_index(self, key): + for i in range(0, self.size, 2): + if self.array[i] == key: + return i + return -1 + + def find(self, shift, hash, key): + for i in range(0, self.size, 2): + if self.array[i] == key: + return self.array[i + 1] + raise KeyError(key) + + def assoc(self, shift, hash, key, val, mutid): + if hash == self.hash: + key_idx = self.find_index(key) + + if key_idx == -1: + new_array = self.array.copy() + new_array.append(key) + new_array.append(val) + + if mutid and mutid == self.mutid: + self.size += 2 + self.array = new_array + return self, True + else: + new_node = CollisionNode( + self.size + 2, hash, new_array, mutid) + return new_node, True + + val_idx = key_idx + 1 + if self.array[val_idx] is val: + return self, False + + if mutid and mutid == self.mutid: + self.array[val_idx] = val + return self, False + else: + new_array = self.array.copy() + new_array[val_idx] = val + return CollisionNode(self.size, hash, new_array, mutid), False + + else: + new_node = BitmapNode( + 2, map_bitpos(self.hash, shift), [None, self], mutid) + return new_node.assoc(shift, hash, key, val, mutid) + + def without(self, shift, hash, key, mutid): + if hash != self.hash: + return W_NOT_FOUND, None + + key_idx = self.find_index(key) + if key_idx == -1: + return W_NOT_FOUND, None + + new_size = self.size - 2 + if new_size == 0: + # Shouldn't be ever reachable + return W_EMPTY, None # pragma: no cover + + if new_size == 2: + if key_idx == 0: + new_array = [self.array[2], self.array[3]] + else: + assert key_idx == 2 + new_array = [self.array[0], self.array[1]] + + new_node = BitmapNode( + 2, map_bitpos(hash, shift), new_array, mutid) + return W_NEWNODE, new_node + + new_array = self.array[:key_idx] + new_array.extend(self.array[key_idx + 2:]) + if mutid and mutid == self.mutid: + self.array = new_array + self.size -= 2 + return W_NEWNODE, self + else: + new_node = CollisionNode( + self.size - 2, self.hash, new_array, mutid) + return W_NEWNODE, new_node + + def keys(self): + for i in range(0, self.size, 2): + yield self.array[i] + + def values(self): + for i in range(1, self.size, 2): + yield self.array[i] + + def items(self): + for i in range(0, self.size, 2): + yield self.array[i], self.array[i + 1] + + def dump(self, buf, level): # pragma: no cover + pad = ' ' * (level + 1) + buf.append( + pad + 'CollisionNode(size={} id={:0x}):'.format( + self.size, id(self))) + + pad = ' ' * (level + 2) + for i in range(0, self.size, 2): + key = self.array[i] + val = self.array[i + 1] + + buf.append('{}{!r}: {!r}'.format(pad, key, val)) + + +class ArrayNode: + def __init__(self, count, array, mutid): + assert count >= 0 + assert count <= HAMT_ARRAY_NODE_SIZE + self.count = count + assert isinstance(array, list) and len(array) == HAMT_ARRAY_NODE_SIZE + self.array = array + self.mutid = mutid + + def _validate(self): + count = 0 + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + if self.array[i] is not None: + count += 1 + assert count == self.count + + def clone(self, mutid): + if __debug__: + self._validate() + return ArrayNode(self.count, self.array.copy(), mutid) + + def assoc(self, shift, hash, key, val, mutid): + idx = map_mask(hash, shift) + node = self.array[idx] + if node is None: + # error is here probably + child, added = BitmapNode(0, 0, [], mutid).assoc( + shift+5, hash, key, val, mutid) + if mutid and mutid == self.mutid: + new_node = self + else: + new_node = ArrayNode(self.count, self.array.copy(), mutid) + assert new_node.array[idx] is None + new_node.array[idx] = child + new_node.count += 1 + new_node._validate() + else: + child, added = node.assoc(shift+5, hash, key, val, mutid) + if child is self: # should be child == node? + return self, added + if mutid and mutid == self.mutid: + new_node = self + else: + new_node = self.clone(mutid) + new_node.array[idx] = child + new_node._validate() + return new_node, added + + def without(self, shift, hash, key, mutid): + idx = map_mask(hash, shift) + node = self.array[idx] + + if node is None: + return W_NOT_FOUND, None + res, sub_node = node.without(shift+5, hash, key, mutid) + if res is W_NOT_FOUND: + assert sub_node is None + return res, None + if res is W_NEWNODE: + assert sub_node is not None + if mutid and mutid == self.mutid: + target = self + else: + target = self.clone(mutid) + target.array[idx] = sub_node + return res, target + if res is W_EMPTY: + assert sub_node is None + new_count = self.count - 1 + if new_count == 0: + # this shouldn't happen + return W_EMPTY, None + if new_count >= 16: # should be > 16, shoudn't cause problems though + if mutid and mutid == self.mutid: + target = self + else: + target = self.clone(mutid) + target.array[idx] = None + target.count = new_count + return W_NEWNODE, target + + bitmap_size = new_count * 2 + bitmap = 0 + + new_array = [] + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + if i == idx: + continue + node = self.array[i] + if node is None: + continue + bitmap |= 1 << i + if type(node) is BitmapNode: + if node.size == 2 and node.array[0] is not None: + new_array.append(node.array[0]) + new_array.append(node.array[1]) + else: + new_array.append(None) + new_array.append(node) + else: + if __debug__: + if isinstance(node, CollisionNode): + assert node.size >= 4 + elif isinstance(node, ArrayNode): + assert node.count >= 16 + # should be > but ok + new_array.append(None) + new_array.append(node) + + return W_NEWNODE, BitmapNode(bitmap_size, bitmap, new_array, mutid) + raise RuntimeError('unreachable code') + + def find(self, shift, hash, key): + idx = map_mask(hash, shift) + node = self.array[idx] + if node is None: + raise KeyError(key) + return node.find(shift+5, hash, key) + + def keys(self): + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + node = self.array[i] + if node is not None: + yield from node.keys() + + def values(self): + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + node = self.array[i] + if node is not None: + yield from node.values() + + def items(self): + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + node = self.array[i] + if node is not None: + yield from node.items() + + def dump(self, buf, level): + pad = ' ' * (level + 1) + buf.append( + pad + 'ArrayNode(count={} id={:0x}):'.format( + self.count, id(self))) + + pad = ' ' * (level + 2) + for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): + node = self.array[i] + if node is None: + continue + buf.append('{}{}::'.format(pad, i)) + node.dump(buf, level + 1) + + +class MapKeys: + + def __init__(self, c, m): + self.__count = c + self.__root = m + + def __len__(self): + return self.__count + + def __iter__(self): + return iter(self.__root.keys()) + + +class MapValues: + + def __init__(self, c, m): + self.__count = c + self.__root = m + + def __len__(self): + return self.__count + + def __iter__(self): + return iter(self.__root.values()) + + +class MapItems: + + def __init__(self, c, m): + self.__count = c + self.__root = m + + def __len__(self): + return self.__count + + def __iter__(self): + return iter(self.__root.items()) + + +class Map: + + def __init__(self, col=None, **kw): + self.__count = 0 + self.__root = BitmapNode(0, 0, [], 0) + self.__hash = -1 + + if isinstance(col, Map): + self.__count = col.__count + self.__root = col.__root + self.__hash = col.__hash + col = None + elif isinstance(col, MapMutation): + raise TypeError('cannot create Maps from MapMutations') + + if col or kw: + init = self.update(col, **kw) + self.__count = init.__count + self.__root = init.__root + + @classmethod + def _new(cls, count, root): + m = Map.__new__(Map) + m.__count = count + m.__root = root + m.__hash = -1 + return m + + def __reduce__(self): + return (type(self), (dict(self.items()),)) + + def __len__(self): + return self.__count + + def __eq__(self, other): + if not isinstance(other, Map): + return NotImplemented + + if len(self) != len(other): + return False + + for key, val in self.__root.items(): + try: + oval = other.__root.find(0, map_hash(key), key) + except KeyError: + return False + else: + if oval != val: + return False + + return True + + def update(self, col=None, **kw): + it = None + if col is not None: + if hasattr(col, 'items'): + it = iter(col.items()) + else: + it = iter(col) + + if it is not None: + if kw: + it = iter(itertools.chain(it, kw.items())) + else: + if kw: + it = iter(kw.items()) + + if it is None: + + return self + + mutid = _mut_id() + root = self.__root + count = self.__count + + i = 0 + while True: + try: + tup = next(it) + except StopIteration: + break + + try: + tup = tuple(tup) + except TypeError: + raise TypeError( + 'cannot convert map update ' + 'sequence element #{} to a sequence'.format(i)) from None + key, val, *r = tup + if r: + raise ValueError( + 'map update sequence element #{} has length ' + '{}; 2 is required'.format(i, len(r) + 2)) + + root, added = root.assoc(0, map_hash(key), key, val, mutid) + if added: + count += 1 + + i += 1 + + return Map._new(count, root) + + def mutate(self): + return MapMutation(self.__count, self.__root) + + def set(self, key, val): + new_count = self.__count + new_root, added = self.__root.assoc(0, map_hash(key), key, val, 0) + + if new_root is self.__root: + assert not added + return self + + if added: + new_count += 1 + + return Map._new(new_count, new_root) + + def delete(self, key): + res, node = self.__root.without(0, map_hash(key), key, 0) + if res is W_EMPTY: + return Map() + elif res is W_NOT_FOUND: + raise KeyError(key) + else: + return Map._new(self.__count - 1, node) + + def get(self, key, default=None): + try: + return self.__root.find(0, map_hash(key), key) + except KeyError: + return default + + def __getitem__(self, key): + return self.__root.find(0, map_hash(key), key) + + def __contains__(self, key): + try: + self.__root.find(0, map_hash(key), key) + except KeyError: + return False + else: + return True + + def __iter__(self): + yield from self.__root.keys() + + def keys(self): + return MapKeys(self.__count, self.__root) + + def values(self): + return MapValues(self.__count, self.__root) + + def items(self): + return MapItems(self.__count, self.__root) + + def __hash__(self): + if self.__hash != -1: + return self.__hash + + MAX = sys.maxsize + MASK = 2 * MAX + 1 + + h = 1927868237 * (self.__count * 2 + 1) + h &= MASK + + for key, value in self.__root.items(): + hx = hash(key) + h ^= (hx ^ (hx << 16) ^ 89869747) * 3644798167 + h &= MASK + + hx = hash(value) + h ^= (hx ^ (hx << 16) ^ 89869747) * 3644798167 + h &= MASK + + h = h * 69069 + 907133923 + h &= MASK + + if h > MAX: + h -= MASK + 1 # pragma: no cover + if h == -1: + h = 590923713 # pragma: no cover + + self.__hash = h + return h + + @reprlib.recursive_repr("{...}") + def __repr__(self): + items = [] + for key, val in self.items(): + items.append("{!r}: {!r}".format(key, val)) + return ''.format( + ', '.join(items), id(self)) + + def __dump__(self): # pragma: no cover + buf = [] + self.__root.dump(buf, 0) + return '\n'.join(buf) + + def __class_getitem__(cls, item): + return cls + + +class MapMutation: + + def __init__(self, count, root): + self.__count = count + self.__root = root + self.__mutid = _mut_id() + + def set(self, key, val): + self[key] = val + + def __enter__(self): + return self + + def __exit__(self, *exc): + self.finish() + return False + + def __iter__(self): + raise TypeError('{} is not iterable'.format(type(self))) + + def __delitem__(self, key): + if self.__mutid == 0: + raise ValueError('mutation {!r} has been finished'.format(self)) + + res, new_root = self.__root.without( + 0, map_hash(key), key, self.__mutid) + if res is W_EMPTY: + self.__count = 0 + self.__root = BitmapNode(0, 0, [], self.__mutid) + elif res is W_NOT_FOUND: + raise KeyError(key) + else: + self.__root = new_root + self.__count -= 1 + + def __setitem__(self, key, val): + if self.__mutid == 0: + raise ValueError('mutation {!r} has been finished'.format(self)) + + self.__root, added = self.__root.assoc( + 0, map_hash(key), key, val, self.__mutid) + + if added: + self.__count += 1 + + def pop(self, key, *args): + if self.__mutid == 0: + raise ValueError('mutation {!r} has been finished'.format(self)) + + if len(args) > 1: + raise TypeError( + 'pop() accepts 1 to 2 positional arguments, ' + 'got {}'.format(len(args) + 1)) + elif len(args) == 1: + default = args[0] + else: + default = void + + val = self.get(key, default) + + try: + del self[key] + except KeyError: + if val is void: + raise + return val + else: + assert val is not void + return val + + def get(self, key, default=None): + try: + return self.__root.find(0, map_hash(key), key) + except KeyError: + return default + + def __getitem__(self, key): + return self.__root.find(0, map_hash(key), key) + + def __contains__(self, key): + try: + self.__root.find(0, map_hash(key), key) + except KeyError: + return False + else: + return True + + def update(self, col=None, **kw): + if self.__mutid == 0: + raise ValueError('mutation {!r} has been finished'.format(self)) + + it = None + if col is not None: + if hasattr(col, 'items'): + it = iter(col.items()) + else: + it = iter(col) + + if it is not None: + if kw: + it = iter(itertools.chain(it, kw.items())) + else: + if kw: + it = iter(kw.items()) + + if it is None: + + return self + + root = self.__root + count = self.__count + + i = 0 + while True: + try: + tup = next(it) + except StopIteration: + break + + try: + tup = tuple(tup) + except TypeError: + raise TypeError( + 'cannot convert map update ' + 'sequence element #{} to a sequence'.format(i)) from None + key, val, *r = tup + if r: + raise ValueError( + 'map update sequence element #{} has length ' + '{}; 2 is required'.format(i, len(r) + 2)) + + root, added = root.assoc(0, map_hash(key), key, val, self.__mutid) + if added: + count += 1 + + i += 1 + + self.__root = root + self.__count = count + + def finish(self): + self.__mutid = 0 + return Map._new(self.__count, self.__root) + + @reprlib.recursive_repr("{...}") + def __repr__(self): + items = [] + for key, val in self.__root.items(): + items.append("{!r}: {!r}".format(key, val)) + return ''.format( + ', '.join(items), id(self)) + + def __len__(self): + return self.__count + + def __reduce__(self): + raise TypeError("can't pickle {} objects".format(type(self).__name__)) + + def __hash__(self): + raise TypeError('unhashable type: {}'.format(type(self).__name__)) + + def __eq__(self, other): + if not isinstance(other, MapMutation): + return NotImplemented + + if len(self) != len(other): + return False + + for key, val in self.__root.items(): + try: + oval = other.__root.find(0, map_hash(key), key) + except KeyError: + return False + else: + if oval != val: + return False + + return True + +collections.abc.Mapping.register(Map) diff --git a/tests/test_issue24.py b/tests/test_issue24.py index 642de6418..791c7f33a 100644 --- a/tests/test_issue24.py +++ b/tests/test_issue24.py @@ -2,6 +2,8 @@ from immutables.map import Map as PyMap +from immutables.map_with_array_nodes import Map as PyAMap +from tests.test_map import BaseMapTest#, PyMapTest, CMapTest class Issue24Base: @@ -12,10 +14,16 @@ def test_issue24(self): new_entries = dict.fromkeys(keys, True) m = self.Map(new_entries) self.assertTrue(17 in m) - with m.mutate() as mm: - for i in keys: - del mm[i] - self.assertEqual(len(mm), 0) + if self.crasher: + with m.mutate() as mm, self.assertRaises(KeyError): + for i in keys: + del mm[i] + #self.assertEqual(len(mm), 0) + else: + with m.mutate() as mm: + for i in keys: + del mm[i] + #self.assertEqual(len(mm), 0) try: @@ -26,10 +34,18 @@ def test_issue24(self): class Issue24PyTest(Issue24Base, unittest.TestCase): Map = PyMap + crasher = False +@unittest.skipIf(CMap is None, 'C Map is not available') class Issue24CTest(Issue24Base, unittest.TestCase): Map = CMap + crasher = True + + +class Issue24ArrayTest(Issue24Base, unittest.TestCase): + Map = PyAMap + crasher = False if __name__ == '__main__': diff --git a/tests/test_map_py_array.py b/tests/test_map_py_array.py new file mode 100644 index 000000000..8bf34abd8 --- /dev/null +++ b/tests/test_map_py_array.py @@ -0,0 +1,18 @@ +import unittest + +from immutables.map_with_array_nodes import Map as PyAMap +from tests.test_map import BaseMapTest + + +class PyAMapTest(BaseMapTest, unittest.TestCase): + Map = PyAMap + + def test_map_stress(self): + pass + + def test_map_mut_stress(self): + pass + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_map_py_array_stress.py b/tests/test_map_py_array_stress.py new file mode 100644 index 000000000..b530a2519 --- /dev/null +++ b/tests/test_map_py_array_stress.py @@ -0,0 +1,15 @@ +import unittest + +from immutables.map_with_array_nodes import Map as PyAMap +from tests.test_map import BaseMapTest + + +class PyAMapTest(unittest.TestCase): + Map = PyAMap + + test_map_stress = BaseMapTest.test_map_stress + test_map_mut_stress = BaseMapTest.test_map_mut_stress + + +if __name__ == '__main__': + unittest.main() From 724fdcbf6554164ed0ae4e935b49f54a6c80a7d3 Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Wed, 15 Apr 2020 21:27:36 -0300 Subject: [PATCH 3/6] Increase counter on inplace insertion on array node Modify test_issue24 to verify it not raises --- immutables/_map.c | 1 + tests/test_issue24.py | 18 ++++-------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/immutables/_map.c b/immutables/_map.c index 7e6356222..4360d7d71 100644 --- a/immutables/_map.c +++ b/immutables/_map.c @@ -1806,6 +1806,7 @@ map_node_array_assoc(MapNode_Array *self, if (mutid != 0 && self->a_mutid == mutid) { new_node = self; + self->a_count++; /*must update count*/ Py_INCREF(self); } else { diff --git a/tests/test_issue24.py b/tests/test_issue24.py index 791c7f33a..609758dbe 100644 --- a/tests/test_issue24.py +++ b/tests/test_issue24.py @@ -3,7 +3,6 @@ from immutables.map import Map as PyMap from immutables.map_with_array_nodes import Map as PyAMap -from tests.test_map import BaseMapTest#, PyMapTest, CMapTest class Issue24Base: @@ -14,16 +13,10 @@ def test_issue24(self): new_entries = dict.fromkeys(keys, True) m = self.Map(new_entries) self.assertTrue(17 in m) - if self.crasher: - with m.mutate() as mm, self.assertRaises(KeyError): - for i in keys: - del mm[i] - #self.assertEqual(len(mm), 0) - else: - with m.mutate() as mm: - for i in keys: - del mm[i] - #self.assertEqual(len(mm), 0) + with m.mutate() as mm: + for i in keys: + del mm[i] + self.assertEqual(len(mm), 0) try: @@ -34,18 +27,15 @@ def test_issue24(self): class Issue24PyTest(Issue24Base, unittest.TestCase): Map = PyMap - crasher = False @unittest.skipIf(CMap is None, 'C Map is not available') class Issue24CTest(Issue24Base, unittest.TestCase): Map = CMap - crasher = True class Issue24ArrayTest(Issue24Base, unittest.TestCase): Map = PyAMap - crasher = False if __name__ == '__main__': From d07e8b17d5a4310bdac95bf39d0225aac50048ff Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Wed, 15 Apr 2020 21:37:02 -0300 Subject: [PATCH 4/6] Delete superfluous files --- immutables/map_with_array_nodes.py | 1000 ---------------------------- tests/test_map_py_array.py | 18 - tests/test_map_py_array_stress.py | 15 - 3 files changed, 1033 deletions(-) delete mode 100644 immutables/map_with_array_nodes.py delete mode 100644 tests/test_map_py_array.py delete mode 100644 tests/test_map_py_array_stress.py diff --git a/immutables/map_with_array_nodes.py b/immutables/map_with_array_nodes.py deleted file mode 100644 index 782d53045..000000000 --- a/immutables/map_with_array_nodes.py +++ /dev/null @@ -1,1000 +0,0 @@ -import abc -import collections.abc -import enum -import itertools -import reprlib -import sys - - -__all__ = ('Map',) - - -# Thread-safe counter. -_mut_id = itertools.count(1).__next__ -HAMT_ARRAY_NODE_SIZE = 32 - -# Python version of _map.c. The topmost comment there explains -# all datastructures and algorithms. -# The code here follows C code closely on purpose to make -# debugging and testing easier. - - -def map_hash(o): - x = hash(o) - return (x & 0xffffffff) ^ ((x >> 32) & 0xffffffff) - - -def map_mask(hash, shift): - return (hash >> shift) & 0x01f - - -def map_bitpos(hash, shift): - return 1 << map_mask(hash, shift) - - -def map_bitcount(v): - v = v - ((v >> 1) & 0x55555555) - v = (v & 0x33333333) + ((v >> 2) & 0x33333333) - v = (v & 0x0F0F0F0F) + ((v >> 4) & 0x0F0F0F0F) - v = v + (v >> 8) - v = (v + (v >> 16)) & 0x3F - return v - - -def map_bitindex(bitmap, bit): - return map_bitcount(bitmap & (bit - 1)) - - -W_EMPTY, W_NEWNODE, W_NOT_FOUND = range(3) -void = object() - - -class BitmapNode: - - def __init__(self, size, bitmap, array, mutid): - self.size = size - self.bitmap = bitmap - assert isinstance(array, list) and len(array) == size - self.array = array - self.mutid = mutid - - def clone(self, mutid): - return BitmapNode(self.size, self.bitmap, self.array.copy(), mutid) - - def assoc(self, shift, hash, key, val, mutid): - bit = map_bitpos(hash, shift) - idx = map_bitindex(self.bitmap, bit) - - if self.bitmap & bit: - key_idx = 2 * idx - val_idx = key_idx + 1 - - key_or_null = self.array[key_idx] - val_or_node = self.array[val_idx] - - if key_or_null is None: # this doesn't work key may be None - sub_node, added = val_or_node.assoc( - shift + 5, hash, key, val, mutid) - if val_or_node is sub_node: - return self, added - - if mutid and mutid == self.mutid: - self.array[val_idx] = sub_node - return self, added - else: - ret = self.clone(mutid) - ret.array[val_idx] = sub_node - return ret, added - - if key == key_or_null: - if val is val_or_node: - return self, False - - if mutid and mutid == self.mutid: - self.array[val_idx] = val - return self, False - else: - ret = self.clone(mutid) - ret.array[val_idx] = val - return ret, False - - existing_key_hash = map_hash(key_or_null) - if existing_key_hash == hash: - sub_node = CollisionNode( - 4, hash, [key_or_null, val_or_node, key, val], mutid) - else: - sub_node = BitmapNode(0, 0, [], mutid) - sub_node, _ = sub_node.assoc( - shift + 5, existing_key_hash, - key_or_null, val_or_node, - mutid) - sub_node, _ = sub_node.assoc( - shift + 5, hash, key, val, - mutid) - - if mutid and mutid == self.mutid: - self.array[key_idx] = None - self.array[val_idx] = sub_node - return self, True - else: - ret = self.clone(mutid) - ret.array[key_idx] = None - ret.array[val_idx] = sub_node - return ret, True - - else: - n = map_bitcount(self.bitmap) - - if n >= 16: # look for this - jdx = map_mask(hash, shift) - - # error maybe here - empty = BitmapNode(0, 0, [], mutid) - new_array = [None] * HAMT_ARRAY_NODE_SIZE - - new_array[jdx], _ = empty.assoc( - shift + 5, hash, key, val, mutid) - j = 0 - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - if (self.bitmap >> i) & 1 != 0: - assert new_array[i] is None - if self.array[j] is None: - new_array[i] = self.array[j+1] - else: - rehash = map_hash(self.array[j]) - new_array[i], _ = empty.assoc( - shift + 5, rehash, - self.array[j], self.array[j+1], - mutid) - j += 2 - new_node = ArrayNode(n + 1, new_array, mutid) - if __debug__: - new_node._validate() - return new_node, True - else: - key_idx = 2 * idx - val_idx = key_idx + 1 - - new_array = self.array[:key_idx] - new_array.append(key) - new_array.append(val) - new_array.extend(self.array[key_idx:]) - - # if mutid and mutid == self.mutid: - # self.size = 2 * (n + 1) - # self.bitmap |= bit - # self.array = new_array - # return self, True - # this fails because of the array code - # else: - return BitmapNode( - 2 * (n + 1), self.bitmap | bit, new_array, mutid), True - - def find(self, shift, hash, key): - bit = map_bitpos(hash, shift) - - if not (self.bitmap & bit): - raise KeyError - - idx = map_bitindex(self.bitmap, bit) - key_idx = idx * 2 - val_idx = key_idx + 1 - - assert val_idx < self.size - - key_or_null = self.array[key_idx] - val_or_node = self.array[val_idx] - - if key_or_null is None: # key maybe None - return val_or_node.find(shift + 5, hash, key) - - if key == key_or_null: - return val_or_node - - raise KeyError(key) - - def without(self, shift, hash, key, mutid): - bit = map_bitpos(hash, shift) - if not (self.bitmap & bit): - return W_NOT_FOUND, None - - idx = map_bitindex(self.bitmap, bit) - key_idx = 2 * idx - val_idx = key_idx + 1 - - key_or_null = self.array[key_idx] - val_or_node = self.array[val_idx] - - if key_or_null is None: # key maybe None - res, sub_node = val_or_node.without(shift + 5, hash, key, mutid) - - if res is W_EMPTY: - raise RuntimeError('unreachable code') # pragma: no cover - - elif res is W_NEWNODE: - if (type(sub_node) is BitmapNode and - sub_node.size == 2 and - sub_node.array[0] is not None): - - if mutid and mutid == self.mutid: - self.array[key_idx] = sub_node.array[0] - self.array[val_idx] = sub_node.array[1] - return W_NEWNODE, self - else: - clone = self.clone(mutid) - clone.array[key_idx] = sub_node.array[0] - clone.array[val_idx] = sub_node.array[1] - return W_NEWNODE, clone - - if mutid and mutid == self.mutid: - self.array[val_idx] = sub_node - return W_NEWNODE, self - else: - clone = self.clone(mutid) - clone.array[val_idx] = sub_node - return W_NEWNODE, clone - - else: - assert sub_node is None - return res, None - - else: - if key == key_or_null: - if self.size == 2: - return W_EMPTY, None - - new_array = self.array[:key_idx] - new_array.extend(self.array[val_idx + 1:]) - - if mutid and mutid == self.mutid: - self.size -= 2 - self.bitmap &= ~bit - self.array = new_array - return W_NEWNODE, self - else: - new_node = BitmapNode( - self.size - 2, self.bitmap & ~bit, new_array, mutid) - return W_NEWNODE, new_node - - else: - return W_NOT_FOUND, None - - def keys(self): - for i in range(0, self.size, 2): - key_or_null = self.array[i] - - if key_or_null is None: - val_or_node = self.array[i + 1] - yield from val_or_node.keys() - else: - yield key_or_null - - def values(self): - for i in range(0, self.size, 2): - key_or_null = self.array[i] - val_or_node = self.array[i + 1] - - if key_or_null is None: - yield from val_or_node.values() - else: - yield val_or_node - - def items(self): - for i in range(0, self.size, 2): - key_or_null = self.array[i] - val_or_node = self.array[i + 1] - - if key_or_null is None: - yield from val_or_node.items() - else: - yield key_or_null, val_or_node - - def dump(self, buf, level): # pragma: no cover - buf.append( - ' ' * (level + 1) + - 'BitmapNode(size={} count={} bitmap={} id={:0x}):'.format( - self.size, self.size / 2, bin(self.bitmap), id(self))) - - for i in range(0, self.size, 2): - key_or_null = self.array[i] - val_or_node = self.array[i + 1] - - pad = ' ' * (level + 2) - - if key_or_null is None: - buf.append(pad + 'None:') - val_or_node.dump(buf, level + 2) - else: - buf.append(pad + '{!r}: {!r}'.format(key_or_null, val_or_node)) - - -class CollisionNode: - - def __init__(self, size, hash, array, mutid): - self.size = size - self.hash = hash - self.array = array - self.mutid = mutid - - def find_index(self, key): - for i in range(0, self.size, 2): - if self.array[i] == key: - return i - return -1 - - def find(self, shift, hash, key): - for i in range(0, self.size, 2): - if self.array[i] == key: - return self.array[i + 1] - raise KeyError(key) - - def assoc(self, shift, hash, key, val, mutid): - if hash == self.hash: - key_idx = self.find_index(key) - - if key_idx == -1: - new_array = self.array.copy() - new_array.append(key) - new_array.append(val) - - if mutid and mutid == self.mutid: - self.size += 2 - self.array = new_array - return self, True - else: - new_node = CollisionNode( - self.size + 2, hash, new_array, mutid) - return new_node, True - - val_idx = key_idx + 1 - if self.array[val_idx] is val: - return self, False - - if mutid and mutid == self.mutid: - self.array[val_idx] = val - return self, False - else: - new_array = self.array.copy() - new_array[val_idx] = val - return CollisionNode(self.size, hash, new_array, mutid), False - - else: - new_node = BitmapNode( - 2, map_bitpos(self.hash, shift), [None, self], mutid) - return new_node.assoc(shift, hash, key, val, mutid) - - def without(self, shift, hash, key, mutid): - if hash != self.hash: - return W_NOT_FOUND, None - - key_idx = self.find_index(key) - if key_idx == -1: - return W_NOT_FOUND, None - - new_size = self.size - 2 - if new_size == 0: - # Shouldn't be ever reachable - return W_EMPTY, None # pragma: no cover - - if new_size == 2: - if key_idx == 0: - new_array = [self.array[2], self.array[3]] - else: - assert key_idx == 2 - new_array = [self.array[0], self.array[1]] - - new_node = BitmapNode( - 2, map_bitpos(hash, shift), new_array, mutid) - return W_NEWNODE, new_node - - new_array = self.array[:key_idx] - new_array.extend(self.array[key_idx + 2:]) - if mutid and mutid == self.mutid: - self.array = new_array - self.size -= 2 - return W_NEWNODE, self - else: - new_node = CollisionNode( - self.size - 2, self.hash, new_array, mutid) - return W_NEWNODE, new_node - - def keys(self): - for i in range(0, self.size, 2): - yield self.array[i] - - def values(self): - for i in range(1, self.size, 2): - yield self.array[i] - - def items(self): - for i in range(0, self.size, 2): - yield self.array[i], self.array[i + 1] - - def dump(self, buf, level): # pragma: no cover - pad = ' ' * (level + 1) - buf.append( - pad + 'CollisionNode(size={} id={:0x}):'.format( - self.size, id(self))) - - pad = ' ' * (level + 2) - for i in range(0, self.size, 2): - key = self.array[i] - val = self.array[i + 1] - - buf.append('{}{!r}: {!r}'.format(pad, key, val)) - - -class ArrayNode: - def __init__(self, count, array, mutid): - assert count >= 0 - assert count <= HAMT_ARRAY_NODE_SIZE - self.count = count - assert isinstance(array, list) and len(array) == HAMT_ARRAY_NODE_SIZE - self.array = array - self.mutid = mutid - - def _validate(self): - count = 0 - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - if self.array[i] is not None: - count += 1 - assert count == self.count - - def clone(self, mutid): - if __debug__: - self._validate() - return ArrayNode(self.count, self.array.copy(), mutid) - - def assoc(self, shift, hash, key, val, mutid): - idx = map_mask(hash, shift) - node = self.array[idx] - if node is None: - # error is here probably - child, added = BitmapNode(0, 0, [], mutid).assoc( - shift+5, hash, key, val, mutid) - if mutid and mutid == self.mutid: - new_node = self - else: - new_node = ArrayNode(self.count, self.array.copy(), mutid) - assert new_node.array[idx] is None - new_node.array[idx] = child - new_node.count += 1 - new_node._validate() - else: - child, added = node.assoc(shift+5, hash, key, val, mutid) - if child is self: # should be child == node? - return self, added - if mutid and mutid == self.mutid: - new_node = self - else: - new_node = self.clone(mutid) - new_node.array[idx] = child - new_node._validate() - return new_node, added - - def without(self, shift, hash, key, mutid): - idx = map_mask(hash, shift) - node = self.array[idx] - - if node is None: - return W_NOT_FOUND, None - res, sub_node = node.without(shift+5, hash, key, mutid) - if res is W_NOT_FOUND: - assert sub_node is None - return res, None - if res is W_NEWNODE: - assert sub_node is not None - if mutid and mutid == self.mutid: - target = self - else: - target = self.clone(mutid) - target.array[idx] = sub_node - return res, target - if res is W_EMPTY: - assert sub_node is None - new_count = self.count - 1 - if new_count == 0: - # this shouldn't happen - return W_EMPTY, None - if new_count >= 16: # should be > 16, shoudn't cause problems though - if mutid and mutid == self.mutid: - target = self - else: - target = self.clone(mutid) - target.array[idx] = None - target.count = new_count - return W_NEWNODE, target - - bitmap_size = new_count * 2 - bitmap = 0 - - new_array = [] - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - if i == idx: - continue - node = self.array[i] - if node is None: - continue - bitmap |= 1 << i - if type(node) is BitmapNode: - if node.size == 2 and node.array[0] is not None: - new_array.append(node.array[0]) - new_array.append(node.array[1]) - else: - new_array.append(None) - new_array.append(node) - else: - if __debug__: - if isinstance(node, CollisionNode): - assert node.size >= 4 - elif isinstance(node, ArrayNode): - assert node.count >= 16 - # should be > but ok - new_array.append(None) - new_array.append(node) - - return W_NEWNODE, BitmapNode(bitmap_size, bitmap, new_array, mutid) - raise RuntimeError('unreachable code') - - def find(self, shift, hash, key): - idx = map_mask(hash, shift) - node = self.array[idx] - if node is None: - raise KeyError(key) - return node.find(shift+5, hash, key) - - def keys(self): - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - node = self.array[i] - if node is not None: - yield from node.keys() - - def values(self): - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - node = self.array[i] - if node is not None: - yield from node.values() - - def items(self): - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - node = self.array[i] - if node is not None: - yield from node.items() - - def dump(self, buf, level): - pad = ' ' * (level + 1) - buf.append( - pad + 'ArrayNode(count={} id={:0x}):'.format( - self.count, id(self))) - - pad = ' ' * (level + 2) - for i in range(0, HAMT_ARRAY_NODE_SIZE, 1): - node = self.array[i] - if node is None: - continue - buf.append('{}{}::'.format(pad, i)) - node.dump(buf, level + 1) - - -class MapKeys: - - def __init__(self, c, m): - self.__count = c - self.__root = m - - def __len__(self): - return self.__count - - def __iter__(self): - return iter(self.__root.keys()) - - -class MapValues: - - def __init__(self, c, m): - self.__count = c - self.__root = m - - def __len__(self): - return self.__count - - def __iter__(self): - return iter(self.__root.values()) - - -class MapItems: - - def __init__(self, c, m): - self.__count = c - self.__root = m - - def __len__(self): - return self.__count - - def __iter__(self): - return iter(self.__root.items()) - - -class Map: - - def __init__(self, col=None, **kw): - self.__count = 0 - self.__root = BitmapNode(0, 0, [], 0) - self.__hash = -1 - - if isinstance(col, Map): - self.__count = col.__count - self.__root = col.__root - self.__hash = col.__hash - col = None - elif isinstance(col, MapMutation): - raise TypeError('cannot create Maps from MapMutations') - - if col or kw: - init = self.update(col, **kw) - self.__count = init.__count - self.__root = init.__root - - @classmethod - def _new(cls, count, root): - m = Map.__new__(Map) - m.__count = count - m.__root = root - m.__hash = -1 - return m - - def __reduce__(self): - return (type(self), (dict(self.items()),)) - - def __len__(self): - return self.__count - - def __eq__(self, other): - if not isinstance(other, Map): - return NotImplemented - - if len(self) != len(other): - return False - - for key, val in self.__root.items(): - try: - oval = other.__root.find(0, map_hash(key), key) - except KeyError: - return False - else: - if oval != val: - return False - - return True - - def update(self, col=None, **kw): - it = None - if col is not None: - if hasattr(col, 'items'): - it = iter(col.items()) - else: - it = iter(col) - - if it is not None: - if kw: - it = iter(itertools.chain(it, kw.items())) - else: - if kw: - it = iter(kw.items()) - - if it is None: - - return self - - mutid = _mut_id() - root = self.__root - count = self.__count - - i = 0 - while True: - try: - tup = next(it) - except StopIteration: - break - - try: - tup = tuple(tup) - except TypeError: - raise TypeError( - 'cannot convert map update ' - 'sequence element #{} to a sequence'.format(i)) from None - key, val, *r = tup - if r: - raise ValueError( - 'map update sequence element #{} has length ' - '{}; 2 is required'.format(i, len(r) + 2)) - - root, added = root.assoc(0, map_hash(key), key, val, mutid) - if added: - count += 1 - - i += 1 - - return Map._new(count, root) - - def mutate(self): - return MapMutation(self.__count, self.__root) - - def set(self, key, val): - new_count = self.__count - new_root, added = self.__root.assoc(0, map_hash(key), key, val, 0) - - if new_root is self.__root: - assert not added - return self - - if added: - new_count += 1 - - return Map._new(new_count, new_root) - - def delete(self, key): - res, node = self.__root.without(0, map_hash(key), key, 0) - if res is W_EMPTY: - return Map() - elif res is W_NOT_FOUND: - raise KeyError(key) - else: - return Map._new(self.__count - 1, node) - - def get(self, key, default=None): - try: - return self.__root.find(0, map_hash(key), key) - except KeyError: - return default - - def __getitem__(self, key): - return self.__root.find(0, map_hash(key), key) - - def __contains__(self, key): - try: - self.__root.find(0, map_hash(key), key) - except KeyError: - return False - else: - return True - - def __iter__(self): - yield from self.__root.keys() - - def keys(self): - return MapKeys(self.__count, self.__root) - - def values(self): - return MapValues(self.__count, self.__root) - - def items(self): - return MapItems(self.__count, self.__root) - - def __hash__(self): - if self.__hash != -1: - return self.__hash - - MAX = sys.maxsize - MASK = 2 * MAX + 1 - - h = 1927868237 * (self.__count * 2 + 1) - h &= MASK - - for key, value in self.__root.items(): - hx = hash(key) - h ^= (hx ^ (hx << 16) ^ 89869747) * 3644798167 - h &= MASK - - hx = hash(value) - h ^= (hx ^ (hx << 16) ^ 89869747) * 3644798167 - h &= MASK - - h = h * 69069 + 907133923 - h &= MASK - - if h > MAX: - h -= MASK + 1 # pragma: no cover - if h == -1: - h = 590923713 # pragma: no cover - - self.__hash = h - return h - - @reprlib.recursive_repr("{...}") - def __repr__(self): - items = [] - for key, val in self.items(): - items.append("{!r}: {!r}".format(key, val)) - return ''.format( - ', '.join(items), id(self)) - - def __dump__(self): # pragma: no cover - buf = [] - self.__root.dump(buf, 0) - return '\n'.join(buf) - - def __class_getitem__(cls, item): - return cls - - -class MapMutation: - - def __init__(self, count, root): - self.__count = count - self.__root = root - self.__mutid = _mut_id() - - def set(self, key, val): - self[key] = val - - def __enter__(self): - return self - - def __exit__(self, *exc): - self.finish() - return False - - def __iter__(self): - raise TypeError('{} is not iterable'.format(type(self))) - - def __delitem__(self, key): - if self.__mutid == 0: - raise ValueError('mutation {!r} has been finished'.format(self)) - - res, new_root = self.__root.without( - 0, map_hash(key), key, self.__mutid) - if res is W_EMPTY: - self.__count = 0 - self.__root = BitmapNode(0, 0, [], self.__mutid) - elif res is W_NOT_FOUND: - raise KeyError(key) - else: - self.__root = new_root - self.__count -= 1 - - def __setitem__(self, key, val): - if self.__mutid == 0: - raise ValueError('mutation {!r} has been finished'.format(self)) - - self.__root, added = self.__root.assoc( - 0, map_hash(key), key, val, self.__mutid) - - if added: - self.__count += 1 - - def pop(self, key, *args): - if self.__mutid == 0: - raise ValueError('mutation {!r} has been finished'.format(self)) - - if len(args) > 1: - raise TypeError( - 'pop() accepts 1 to 2 positional arguments, ' - 'got {}'.format(len(args) + 1)) - elif len(args) == 1: - default = args[0] - else: - default = void - - val = self.get(key, default) - - try: - del self[key] - except KeyError: - if val is void: - raise - return val - else: - assert val is not void - return val - - def get(self, key, default=None): - try: - return self.__root.find(0, map_hash(key), key) - except KeyError: - return default - - def __getitem__(self, key): - return self.__root.find(0, map_hash(key), key) - - def __contains__(self, key): - try: - self.__root.find(0, map_hash(key), key) - except KeyError: - return False - else: - return True - - def update(self, col=None, **kw): - if self.__mutid == 0: - raise ValueError('mutation {!r} has been finished'.format(self)) - - it = None - if col is not None: - if hasattr(col, 'items'): - it = iter(col.items()) - else: - it = iter(col) - - if it is not None: - if kw: - it = iter(itertools.chain(it, kw.items())) - else: - if kw: - it = iter(kw.items()) - - if it is None: - - return self - - root = self.__root - count = self.__count - - i = 0 - while True: - try: - tup = next(it) - except StopIteration: - break - - try: - tup = tuple(tup) - except TypeError: - raise TypeError( - 'cannot convert map update ' - 'sequence element #{} to a sequence'.format(i)) from None - key, val, *r = tup - if r: - raise ValueError( - 'map update sequence element #{} has length ' - '{}; 2 is required'.format(i, len(r) + 2)) - - root, added = root.assoc(0, map_hash(key), key, val, self.__mutid) - if added: - count += 1 - - i += 1 - - self.__root = root - self.__count = count - - def finish(self): - self.__mutid = 0 - return Map._new(self.__count, self.__root) - - @reprlib.recursive_repr("{...}") - def __repr__(self): - items = [] - for key, val in self.__root.items(): - items.append("{!r}: {!r}".format(key, val)) - return ''.format( - ', '.join(items), id(self)) - - def __len__(self): - return self.__count - - def __reduce__(self): - raise TypeError("can't pickle {} objects".format(type(self).__name__)) - - def __hash__(self): - raise TypeError('unhashable type: {}'.format(type(self).__name__)) - - def __eq__(self, other): - if not isinstance(other, MapMutation): - return NotImplemented - - if len(self) != len(other): - return False - - for key, val in self.__root.items(): - try: - oval = other.__root.find(0, map_hash(key), key) - except KeyError: - return False - else: - if oval != val: - return False - - return True - -collections.abc.Mapping.register(Map) diff --git a/tests/test_map_py_array.py b/tests/test_map_py_array.py deleted file mode 100644 index 8bf34abd8..000000000 --- a/tests/test_map_py_array.py +++ /dev/null @@ -1,18 +0,0 @@ -import unittest - -from immutables.map_with_array_nodes import Map as PyAMap -from tests.test_map import BaseMapTest - - -class PyAMapTest(BaseMapTest, unittest.TestCase): - Map = PyAMap - - def test_map_stress(self): - pass - - def test_map_mut_stress(self): - pass - - -if __name__ == '__main__': - unittest.main() diff --git a/tests/test_map_py_array_stress.py b/tests/test_map_py_array_stress.py deleted file mode 100644 index b530a2519..000000000 --- a/tests/test_map_py_array_stress.py +++ /dev/null @@ -1,15 +0,0 @@ -import unittest - -from immutables.map_with_array_nodes import Map as PyAMap -from tests.test_map import BaseMapTest - - -class PyAMapTest(unittest.TestCase): - Map = PyAMap - - test_map_stress = BaseMapTest.test_map_stress - test_map_mut_stress = BaseMapTest.test_map_mut_stress - - -if __name__ == '__main__': - unittest.main() From 167bc912161cc1cdea5026c104e16d751154ac0e Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Wed, 15 Apr 2020 21:46:12 -0300 Subject: [PATCH 5/6] remove superfluous test case --- tests/test_issue24.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/test_issue24.py b/tests/test_issue24.py index 609758dbe..e0d6c89fa 100644 --- a/tests/test_issue24.py +++ b/tests/test_issue24.py @@ -2,7 +2,6 @@ from immutables.map import Map as PyMap -from immutables.map_with_array_nodes import Map as PyAMap class Issue24Base: @@ -34,9 +33,5 @@ class Issue24CTest(Issue24Base, unittest.TestCase): Map = CMap -class Issue24ArrayTest(Issue24Base, unittest.TestCase): - Map = PyAMap - - if __name__ == '__main__': unittest.main() From 85bce737190e441beb5e92257abea760cc558e9f Mon Sep 17 00:00:00 2001 From: TIGirardi <55336837+TIGirardi@users.noreply.github.com> Date: Sat, 18 Apr 2020 14:17:31 -0300 Subject: [PATCH 6/6] Decrement array node count on inplace deletion Also improve tests --- immutables/_map.c | 2 +- tests/test_issue24.py | 120 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/immutables/_map.c b/immutables/_map.c index 4360d7d71..89687d049 100644 --- a/immutables/_map.c +++ b/immutables/_map.c @@ -1941,9 +1941,9 @@ map_node_array_without(MapNode_Array *self, if (target == NULL) { return W_ERROR; } - target->a_count = new_count; } + target->a_count = new_count; Py_CLEAR(target->a_array[idx]); *new_node = (MapNode*)target; /* borrow */ diff --git a/tests/test_issue24.py b/tests/test_issue24.py index e0d6c89fa..afe6525a9 100644 --- a/tests/test_issue24.py +++ b/tests/test_issue24.py @@ -1,7 +1,11 @@ import unittest +from immutables.map import Map as PyMap, map_bitcount -from immutables.map import Map as PyMap + +class CollisionKey: + def __hash__(self): + return 0 class Issue24Base: @@ -17,6 +21,86 @@ def test_issue24(self): del mm[i] self.assertEqual(len(mm), 0) + def dump_check_node_kind(self, header, kind): + header = header.strip() + self.assertTrue(header.strip().startswith(kind)) + + def dump_check_node_size(self, header, size): + node_size = header.split('size=', 1)[1] + node_size = int(node_size.split(maxsplit=1)[0]) + self.assertEqual(node_size, size) + + def dump_check_bitmap_count(self, header, count): + header = header.split('bitmap=')[1] + bitmap = int(header.split(maxsplit=1)[0], 0) + self.assertEqual(map_bitcount(bitmap), count) + + def dump_check_bitmap_node_count(self, header, count): + self.dump_check_node_kind(header, 'Bitmap') + self.dump_check_node_size(header, count * 2) + self.dump_check_bitmap_count(header, count) + + def dump_check_collision_node_count(self, header, count): + self.dump_check_node_kind(header, 'Collision') + self.dump_check_node_size(header, 2 * count) + + def test_bitmap_node_update_in_place_count(self): + keys = range(7) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + d = m.__dump__().splitlines() + self.assertTrue(d) + if d[0].startswith('HAMT'): + header = d[1] # skip _map.Map.__dump__() header + else: + header = d[0] + self.dump_check_bitmap_node_count(header, 7) + + def test_bitmap_node_delete_in_place_count(self): + keys = range(7) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + with m.mutate() as mm: + del mm[0], mm[2], mm[3] + m2 = mm.finish() + d = m2.__dump__().splitlines() + self.assertTrue(d) + if d[0].startswith('HAMT'): + header = d[1] # skip _map.Map.__dump__() header + else: + header = d[0] + self.dump_check_bitmap_node_count(header, 4) + + def test_collision_node_update_in_place_count(self): + keys = (CollisionKey() for i in range(7)) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + d = m.__dump__().splitlines() + self.assertTrue(len(d) > 3) + # get node headers + if d[0].startswith('HAMT'): + h1, h2 = d[1], d[3] # skip _map.Map.__dump__() header + else: + h1, h2 = d[0], d[2] + self.dump_check_node_kind(h1, 'Bitmap') + self.dump_check_collision_node_count(h2, 7) + + def test_collision_node_delete_in_place_count(self): + keys = [CollisionKey() for i in range(7)] + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + with m.mutate() as mm: + del mm[keys[0]], mm[keys[2]], mm[keys[3]] + m2 = mm.finish() + d = m2.__dump__().splitlines() + self.assertTrue(len(d) > 3) + # get node headers + if d[0].startswith('HAMT'): + h1, h2 = d[1], d[3] # skip _map.Map.__dump__() header + else: + h1, h2 = d[0], d[2] + self.dump_check_node_kind(h1, 'Bitmap') + self.dump_check_collision_node_count(h2, 4) try: from immutables._map import Map as CMap @@ -32,6 +116,40 @@ class Issue24PyTest(Issue24Base, unittest.TestCase): class Issue24CTest(Issue24Base, unittest.TestCase): Map = CMap + def hamt_dump_check_first_return_second(self, m): + d = m.__dump__().splitlines() + self.assertTrue(len(d) > 2) + self.assertTrue(d[0].startswith('HAMT')) + return d[1] + + def test_array_node_update_in_place_count(self): + keys = range(27) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + header = self.hamt_dump_check_first_return_second(m) + self.dump_check_node_kind(header, 'Array') + for i in range(2, 18): + m = m.delete(i) + header = self.hamt_dump_check_first_return_second(m) + self.dump_check_bitmap_node_count(header, 11) + + def test_array_node_delete_in_place_count(self): + keys = range(27) + new_entries = dict.fromkeys(keys, True) + m = self.Map(new_entries) + header = self.hamt_dump_check_first_return_second(m) + self.dump_check_node_kind(header, 'Array') + with m.mutate() as mm: + for i in range(5): + del mm[i] + m2 = mm.finish() + header = self.hamt_dump_check_first_return_second(m2) + self.dump_check_node_kind(header, 'Array') + for i in range(6, 17): + m2 = m2.delete(i) + header = self.hamt_dump_check_first_return_second(m2) + self.dump_check_bitmap_node_count(header, 11) + if __name__ == '__main__': unittest.main()