From 253deb373d383d0088897e46160845aeb276c31b Mon Sep 17 00:00:00 2001 From: Anton Vasilyev Date: Mon, 1 Mar 2021 17:08:49 +0300 Subject: [PATCH 1/2] Replace PathCopyingPersistentTreeMap hash and size calculation from lazy linear algorithm to pre-calculations, which is efficient for public copying methods --- .../collect/PathCopyingPersistentTreeMap.java | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java b/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java index 7c0453f9b..368ddcdd1 100644 --- a/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java +++ b/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java @@ -73,7 +73,7 @@ public final class PathCopyingPersistentTreeMap, V> extends AbstractImmutableSortedMap implements PersistentSortedMap, Serializable { - private static final long serialVersionUID = 1041711151457528188L; + private static final long serialVersionUID = -8878491978270416107L; @SuppressWarnings("unused") @SuppressFBWarnings( @@ -86,18 +86,23 @@ private static final class Node extends SimpleImmutableEntry { private static final boolean RED = true; private static final boolean BLACK = false; - private static final long serialVersionUID = -7393505826652634501L; + private static final long serialVersionUID = 3222777055260841400L; private final @Nullable Node left; private final @Nullable Node right; private final boolean isRed; + private final int subTreeSize; + private final int subTreeHash; + // Leaf node Node(K pKey, V pValue) { super(pKey, pValue); left = null; right = null; isRed = RED; + subTreeSize = 1; + subTreeHash = super.hashCode(); } // Any node @@ -106,6 +111,22 @@ private static final class Node extends SimpleImmutableEntry { left = pLeft; right = pRight; isRed = pRed; + subTreeSize = + (pLeft == null ? 0 : pLeft.getSubTreeSize()) + + (pRight == null ? 0 : pRight.getSubTreeSize()) + + 1; + subTreeHash = + (pLeft == null ? 0 : pLeft.getSubTreeHash()) + + (pRight == null ? 0 : pRight.getSubTreeHash()) + + super.hashCode(); + } + + public int getSubTreeSize() { + return subTreeSize; + } + + public int getSubTreeHash() { + return subTreeHash; } boolean isLeaf() { @@ -156,7 +177,7 @@ static int countNodes(@Nullable Node n) { if (n == null) { return 0; } - return countNodes(n.left) + 1 + countNodes(n.right); + return n.getSubTreeSize(); } } @@ -232,10 +253,13 @@ public static , V> PersistentSortedMap cop @LazyInit private transient @Nullable NavigableSet> entrySet; - @LazyInit private transient int size; + private final int size; + private final int hashCode; private PathCopyingPersistentTreeMap(@Nullable Node pRoot) { root = pRoot; + size = pRoot == null ? 0 : pRoot.getSubTreeSize(); + hashCode = pRoot == null ? 0 : pRoot.getSubTreeHash(); } // private utility methods @@ -798,9 +822,8 @@ public boolean equals(Object pObj) { } @Override - @SuppressWarnings("RedundantOverride") // to document that using super.hashCode is intended public int hashCode() { - return super.hashCode(); + return hashCode; } @Override @@ -847,9 +870,6 @@ public boolean isEmpty() { @Override public int size() { - if (size <= 0) { - size = Node.countNodes(root); - } return size; } From 9e2bd175b3cf0907b6ab05815a500d8eadc5884d Mon Sep 17 00:00:00 2001 From: Anton Vasilyev Date: Tue, 2 Mar 2021 18:29:40 +0300 Subject: [PATCH 2/2] Compute PathCopyingPersistentTreeMap hash and size on public copying methods instead of lazy linear algorithm --- .../collect/PathCopyingPersistentTreeMap.java | 132 +++++++++++------- 1 file changed, 84 insertions(+), 48 deletions(-) diff --git a/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java b/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java index 368ddcdd1..9372276b5 100644 --- a/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java +++ b/src/org/sosy_lab/common/collect/PathCopyingPersistentTreeMap.java @@ -73,7 +73,7 @@ public final class PathCopyingPersistentTreeMap, V> extends AbstractImmutableSortedMap implements PersistentSortedMap, Serializable { - private static final long serialVersionUID = -8878491978270416107L; + private static final long serialVersionUID = -5708332286565509457L; @SuppressWarnings("unused") @SuppressFBWarnings( @@ -86,23 +86,18 @@ private static final class Node extends SimpleImmutableEntry { private static final boolean RED = true; private static final boolean BLACK = false; - private static final long serialVersionUID = 3222777055260841400L; + private static final long serialVersionUID = -7393505826652634501L; private final @Nullable Node left; private final @Nullable Node right; private final boolean isRed; - private final int subTreeSize; - private final int subTreeHash; - // Leaf node Node(K pKey, V pValue) { super(pKey, pValue); left = null; right = null; isRed = RED; - subTreeSize = 1; - subTreeHash = super.hashCode(); } // Any node @@ -111,22 +106,6 @@ private static final class Node extends SimpleImmutableEntry { left = pLeft; right = pRight; isRed = pRed; - subTreeSize = - (pLeft == null ? 0 : pLeft.getSubTreeSize()) - + (pRight == null ? 0 : pRight.getSubTreeSize()) - + 1; - subTreeHash = - (pLeft == null ? 0 : pLeft.getSubTreeHash()) - + (pRight == null ? 0 : pRight.getSubTreeHash()) - + super.hashCode(); - } - - public int getSubTreeSize() { - return subTreeSize; - } - - public int getSubTreeHash() { - return subTreeHash; } boolean isLeaf() { @@ -177,14 +156,44 @@ static int countNodes(@Nullable Node n) { if (n == null) { return 0; } - return n.getSubTreeSize(); + return countNodes(n.left) + 1 + countNodes(n.right); + } + } + + @Immutable(containerOf = {"K", "V"}) + private static final class ContainerOfNodeWithDiffs { + private final Node node; + private final int sizeDiff; + private final int hashDiff; + + private ContainerOfNodeWithDiffs(Node node, int sizeDiff, int hashDiff) { + this.node = node; + this.sizeDiff = sizeDiff; + this.hashDiff = hashDiff; + } + + static > ContainerOfNodeWithDiffs of( + Node node, int sizeDiff, int hashDiff) { + return new ContainerOfNodeWithDiffs<>(node, sizeDiff, hashDiff); + } + + Node getNode() { + return node; + } + + int getSizeDiff() { + return sizeDiff; + } + + int getHashDiff() { + return hashDiff; } } // static creation methods private static final PathCopyingPersistentTreeMap EMPTY_MAP = - new PathCopyingPersistentTreeMap(null); + new PathCopyingPersistentTreeMap(null, 0, 0); @SuppressWarnings("unchecked") public static , V> PersistentSortedMap of() { @@ -256,10 +265,10 @@ public static , V> PersistentSortedMap cop private final int size; private final int hashCode; - private PathCopyingPersistentTreeMap(@Nullable Node pRoot) { + private PathCopyingPersistentTreeMap(@Nullable Node pRoot, int pSize, int pHashCode) { root = pRoot; - size = pRoot == null ? 0 : pRoot.getSubTreeSize(); - hashCode = pRoot == null ? 0 : pRoot.getSubTreeHash(); + size = pSize; + hashCode = pHashCode; } // private utility methods @@ -532,11 +541,12 @@ void checkAssertions() { /** * Create a map instance with a given root node. * - * @param newRoot A node or null (meaning the empty tree). + * @param pRoot Container of a node or null (meaning the empty tree) with size and hash changes. * @return A map instance with the given tree. */ @SuppressWarnings("ReferenceEquality") // cannot use equals() for check whether tree is the same - private PersistentSortedMap mapFromTree(@Var Node newRoot) { + private PersistentSortedMap mapFromTree(ContainerOfNodeWithDiffs pRoot) { + @Var Node newRoot = pRoot.getNode(); if (newRoot == root) { return this; } else if (newRoot == null) { @@ -544,42 +554,57 @@ private PersistentSortedMap mapFromTree(@Var Node newRoot) { } else { // Root is always black. newRoot = newRoot.withColor(Node.BLACK); - return new PathCopyingPersistentTreeMap<>(newRoot); + return new PathCopyingPersistentTreeMap<>( + newRoot, size + pRoot.getSizeDiff(), hashCode + pRoot.getHashDiff()); } } @Override public PersistentSortedMap putAndCopy(K key, V value) { - return mapFromTree(putAndCopy0(checkNotNull(key), value, root)); + return mapFromTree(putAndCopy0(checkNotNull(key), value, root, 0, 0)); } - private static , V> Node putAndCopy0( - K key, V value, @Var Node current) { + private static , V> ContainerOfNodeWithDiffs putAndCopy0( + K key, V value, @Var Node current, @Var int sizeDiff, @Var int hashDiff) { // Inserting is easy: // We find the place where to insert, // and afterwards fix the invariants by some rotations or re-colorings. if (current == null) { - return new Node<>(key, value); + Node node = new Node<>(key, value); + sizeDiff = sizeDiff + 1; + hashDiff = hashDiff + node.hashCode(); + return ContainerOfNodeWithDiffs.of(node, sizeDiff, hashDiff); } int comp = key.compareTo(current.getKey()); if (comp < 0) { // key < current.data - Node newLeft = putAndCopy0(key, value, current.left); + ContainerOfNodeWithDiffs containerOfNodeWithDiffs = + putAndCopy0(key, value, current.left, sizeDiff, hashDiff); + sizeDiff = containerOfNodeWithDiffs.getSizeDiff(); + hashDiff = containerOfNodeWithDiffs.getHashDiff(); + Node newLeft = containerOfNodeWithDiffs.getNode(); current = current.withLeftChild(newLeft); } else if (comp > 0) { // key > current.data - Node newRight = putAndCopy0(key, value, current.right); + ContainerOfNodeWithDiffs containerOfNodeWithDiffs = + putAndCopy0(key, value, current.right, sizeDiff, hashDiff); + sizeDiff = containerOfNodeWithDiffs.getSizeDiff(); + hashDiff = containerOfNodeWithDiffs.getHashDiff(); + Node newRight = containerOfNodeWithDiffs.getNode(); current = current.withRightChild(newRight); } else { + // replace current node with a new one + hashDiff = hashDiff - current.hashCode(); current = new Node<>(key, value, current.left, current.right, current.getColor()); + hashDiff = hashDiff + current.hashCode(); } // restore invariants - return restoreInvariants(current); + return ContainerOfNodeWithDiffs.of(restoreInvariants(current), sizeDiff, hashDiff); } @SuppressWarnings("unchecked") @@ -588,12 +613,12 @@ public PersistentSortedMap removeAndCopy(Object key) { if (isEmpty()) { return this; } - return mapFromTree(removeAndCopy0((K) checkNotNull(key), root)); + return mapFromTree(removeAndCopy0((K) checkNotNull(key), root, 0, 0)); } @Nullable - private static , V> Node removeAndCopy0( - K key, @Var Node current) { + private static , V> ContainerOfNodeWithDiffs removeAndCopy0( + K key, @Var Node current, @Var int sizeDiff, @Var int hashDiff) { // Removing a node is more difficult. // We can remove a leaf if it is red. // So we try to always have a red node while going downwards. @@ -612,7 +637,7 @@ private static , V> Node removeAndCopy0( // key < current.data if (current.left == null) { // Target key is not in map. - return current; + return ContainerOfNodeWithDiffs.of(current, sizeDiff, hashDiff); } // Go down leftwards, keeping a red node. @@ -623,14 +648,18 @@ private static , V> Node removeAndCopy0( } // recursive descent - Node newLeft = removeAndCopy0(key, current.left); + ContainerOfNodeWithDiffs containerOfNodeWithDiffs = + removeAndCopy0(key, current.left, sizeDiff, hashDiff); + sizeDiff = containerOfNodeWithDiffs.getSizeDiff(); + hashDiff = containerOfNodeWithDiffs.getHashDiff(); + Node newLeft = containerOfNodeWithDiffs.getNode(); current = current.withLeftChild(newLeft); } else { // key >= current.data if ((comp > 0) && (current.right == null)) { // Target key is not in map. - return current; + return ContainerOfNodeWithDiffs.of(current, sizeDiff, hashDiff); } if (Node.isRed(current.left)) { @@ -645,7 +674,9 @@ private static , V> Node removeAndCopy0( if ((comp == 0) && (current.right == null)) { assert current.left == null; // We can delete the node easily, it's a leaf. - return null; + hashDiff = hashDiff - current.hashCode(); + sizeDiff = sizeDiff - 1; + return ContainerOfNodeWithDiffs.of(null, sizeDiff, hashDiff); } if (!Node.isRed(current.right) && !Node.isRed(current.right.left)) { @@ -661,7 +692,8 @@ private static , V> Node removeAndCopy0( // We have to delete current, but is has children. // We replace current with the smallest node in the right subtree (the "successor"), // and delete that (leaf) node there. - + hashDiff = hashDiff - current.hashCode(); + sizeDiff = sizeDiff - 1; @Var Node successor = current.right; while (successor.left != null) { successor = successor.left; @@ -682,12 +714,16 @@ private static , V> Node removeAndCopy0( // key > current.data // Go down rightwards. - Node newRight = removeAndCopy0(key, current.right); + ContainerOfNodeWithDiffs containerOfNodeWithDiffs = + removeAndCopy0(key, current.right, sizeDiff, hashDiff); + sizeDiff = containerOfNodeWithDiffs.getSizeDiff(); + hashDiff = containerOfNodeWithDiffs.getHashDiff(); + Node newRight = containerOfNodeWithDiffs.getNode(); current = current.withRightChild(newRight); } } - return restoreInvariants(current); + return ContainerOfNodeWithDiffs.of(restoreInvariants(current), sizeDiff, hashDiff); } /**