diff --git a/Util/src/main/java/io/deephaven/util/datastructures/intrusive/IntrusiveArraySet.java b/Util/src/main/java/io/deephaven/util/datastructures/intrusive/IntrusiveArraySet.java index 604fc4a758c..9956607c6af 100644 --- a/Util/src/main/java/io/deephaven/util/datastructures/intrusive/IntrusiveArraySet.java +++ b/Util/src/main/java/io/deephaven/util/datastructures/intrusive/IntrusiveArraySet.java @@ -6,17 +6,14 @@ import org.jetbrains.annotations.NotNull; import java.lang.reflect.Array; -import java.util.Arrays; -import java.util.Collection; -import java.util.Iterator; -import java.util.Set; +import java.util.*; /** * An intrusive set that uses an array for its backing storage. - * + *

* You can insert, remove, or check for existence in O(1) time. Clearing the set is O(n); as we need to null out * references. - * + *

* If you attempt to perform an operation element which is not in this set, but is in another set with the same adapter; * then you are going to have a bad time. Tread carefully. * @@ -157,6 +154,7 @@ public boolean retainAll(@NotNull Collection collection) { int destinationSlot = 0; for (Object c : collection) { + // noinspection unchecked final int slot = adapter.getSlot((T) c); // check if it exists in our set if (slot >= 0 && slot < size && storage[slot] == c) { @@ -171,6 +169,7 @@ public boolean retainAll(@NotNull Collection collection) { // swap c and destination slot storage[slot] = storage[destinationSlot]; + // noinspection unchecked storage[destinationSlot] = (T) c; adapter.setSlot(storage[destinationSlot], destinationSlot); @@ -203,6 +202,20 @@ public void clear() { size = 0; } + /** + * Sort the contents of this IntrusiveArraySet according to {@code c}. Any subsequent mutation may render + * this IntrusiveArraySet unsorted, as removes are done in O(1) time by swapping with the last element. + * + * @param c The {@link Comparator} to sort by + */ + public void sort(@NotNull final Comparator c) { + final int s = size; + Arrays.sort(storage, 0, s, c); + for (int ii = 0; ii < s; ++ii) { + adapter.setSlot(storage[ii], ii); + } + } + /** * Adapter interface for elements to be entered into the set. */ diff --git a/Util/src/test/java/io/deephaven/util/datastructures/intrusive/TestIntrusiveArraySet.java b/Util/src/test/java/io/deephaven/util/datastructures/intrusive/TestIntrusiveArraySet.java index f8d52e7269f..a584b130fad 100644 --- a/Util/src/test/java/io/deephaven/util/datastructures/intrusive/TestIntrusiveArraySet.java +++ b/Util/src/test/java/io/deephaven/util/datastructures/intrusive/TestIntrusiveArraySet.java @@ -3,13 +3,16 @@ */ package io.deephaven.util.datastructures.intrusive; -import junit.framework.TestCase; +import org.jetbrains.annotations.NotNull; +import org.junit.Test; import java.util.*; import java.util.stream.Collectors; -public class TestIntrusiveArraySet extends TestCase { - private static class IntrusiveValue { +import static org.junit.Assert.*; + +public class TestIntrusiveArraySet { + private static class IntrusiveValue implements Comparable { private IntrusiveValue(int sentinel) { this.sentinel = sentinel; } @@ -28,6 +31,11 @@ public String toString() { ", slot=" + slot + '}'; } + + @Override + public int compareTo(@NotNull final IntrusiveValue other) { + return Integer.compare(sentinel, other.sentinel); + } } private static class Adapter implements IntrusiveArraySet.Adapter { @@ -43,6 +51,7 @@ public void setSlot(IntrusiveValue element, int slot) { } } + @Test public void testSimple() { final IntrusiveArraySet set = new IntrusiveArraySet<>(new Adapter(), IntrusiveValue.class); final IntrusiveValue twentyThree = new IntrusiveValue(23); @@ -63,10 +72,7 @@ public void testSimple() { assertTrue(set.contains(values.get(1))); assertTrue(set.containsAll(values)); - final List copy = new ArrayList<>(); - for (IntrusiveValue value : set) { - copy.add(value); - } + final List copy = new ArrayList<>(set); assertEquals(values, copy); @@ -75,12 +81,7 @@ public void testSimple() { set.add(nineteen); assertEquals(values.size(), set.size()); - for (final Iterator it = set.iterator(); it.hasNext();) { - final IntrusiveValue next = it.next(); - if (next.sentinel == 1 || next.sentinel == 7 || next.sentinel == 19) { - it.remove(); - } - } + set.removeIf(next -> next.sentinel == 1 || next.sentinel == 7 || next.sentinel == 19); assertFalse(set.contains(values.get(0))); assertFalse(set.contains(values.get(4))); @@ -94,7 +95,7 @@ public void testSimple() { copy.clear(); copy.addAll(set); - copy.sort(Comparator.comparing(v -> v.sentinel)); + copy.sort(Comparator.naturalOrder()); assertEquals(values, copy); set.retainAll(values); @@ -102,11 +103,11 @@ public void testSimple() { copy.clear(); copy.addAll(set); - copy.sort(Comparator.comparing(v -> v.sentinel)); + copy.sort(Comparator.naturalOrder()); assertEquals(values, copy); final List valueCopy = new ArrayList<>(values); - valueCopy.removeIf(x -> x.sentinel == 2 || x.sentinel < 13); + valueCopy.removeIf(x -> x.sentinel < 13); valueCopy.add(valueCopy.get(0)); valueCopy.add(valueCopy.get(0)); @@ -122,4 +123,40 @@ public void testSimple() { assertEquals(valueCopy, copy); } + + @Test + public void testSort() { + final IntrusiveArraySet set = new IntrusiveArraySet<>(new Adapter(), IntrusiveValue.class); + set.sort(Comparator.naturalOrder()); + + final List values = IntrusiveValue.make(78, 1, 22, 5, 4, 3, 66, 67, 0, 100); + set.ensureCapacity(values.size()); + set.addAll(values); + + assertArrayEquals(set.toArray(), values.toArray()); + + set.sort(Comparator.naturalOrder()); + values.sort(Comparator.naturalOrder()); + + assertArrayEquals(set.toArray(), values.toArray()); + + set.remove(values.remove(2)); + set.remove(values.remove(5)); + + set.sort(Comparator.naturalOrder()); + values.sort(Comparator.naturalOrder()); + + assertArrayEquals(set.toArray(), values.toArray()); + + values.addAll(IntrusiveValue.make(101, 102, 100, 99, 65, 87, 2, 2, 4)); + set.ensureCapacity(100); + set.addAll(values); + + assertArrayEquals(set.toArray(), values.toArray()); + + set.sort(Comparator.naturalOrder()); + values.sort(Comparator.naturalOrder()); + + assertArrayEquals(set.toArray(), values.toArray()); + } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java index 6601239a461..cc0627fb4ce 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java @@ -792,6 +792,11 @@ private static final class LinkedDirective { */ private final long rowKeyInParentUnsorted; + /** + * This LinkedDirective's slot in its parent's {@link #children}. + */ + private int slotInParent = -1; + /** * The action to take for this directive. Will be {@link VisitAction#Undefined undefined} on construction; this * is used to determine whether a directive was newly-created by a "compute if absent". Set to @@ -848,8 +853,24 @@ private VisitAction getAction() { return action; } - private LinkedDirective addChild(@NotNull final LinkedDirective childDirective) { - (children == null ? children = new ArrayList<>() : children).add(childDirective); + private LinkedDirective addOrReplaceChild( + @NotNull final LinkedDirective childDirective, + @Nullable final LinkedDirective removedChildDirective) { + if (children == null) { + Assert.eqNull(removedChildDirective, "removedChildDirective"); + children = new ArrayList<>(); + } + if (removedChildDirective == null) { + childDirective.slotInParent = children.size(); + children.add(childDirective); + } else { + Assert.eq(children.get(removedChildDirective.slotInParent), + "children.get(removedChildDirective.slotInParent)", + removedChildDirective, "removedChildDirective"); + Assert.eq(removedChildDirective.action, "removedChildDirective.action", Linkage); + childDirective.slotInParent = removedChildDirective.slotInParent; + children.set(childDirective.slotInParent, childDirective); + } return this; } @@ -1058,7 +1079,8 @@ private LinkedDirective linkKeyTableNodeDirectives( // with different filtering or different input data (e.g. in the case of a saved key table). continue; } - removeOldChildren(linkedDirectives.put(nodeKey, linked), linkedDirectives); + LinkedDirective removed = linkedDirectives.put(nodeKey, linked); + removeOldChildren(removed, linkedDirectives); linked.setAction(fromKeyTable.getAction()); // Find ancestors iteratively until we hit the root or a node directive that already exists boolean needToFindAncestors = true; @@ -1071,7 +1093,9 @@ private LinkedDirective linkKeyTableNodeDirectives( needToFindAncestors = false; } else if (parentNodeKeyFound) { nodeKey = parentNodeKeyHolder.getValue(); - linked = linkedDirectives.putIfAbsent(nodeKey, linkedDirectiveFactory).addChild(linked); + linked = linkedDirectives.putIfAbsent(nodeKey, linkedDirectiveFactory) + .addOrReplaceChild(linked, removed); + removed = null; if (linkedDirectiveInvalid(linked)) { failIfConcurrentAttemptInconsistent(); log.error().append("Could not find node for parent key=").append(NODE_KEY_FORMATTER, nodeKey)