From f55d4c90cefc4d972f8cbc6dd03f8436a786299f Mon Sep 17 00:00:00 2001 From: bergmann Date: Mon, 19 Jun 2017 16:03:32 +0200 Subject: [PATCH] [518434] Fixes index listener notification order for fall-through Change-Id: Ia216424898dfe0f384381c693399f0a7bee33338 Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=518434 Signed-off-by: bergmann (cherry picked from commit a56b650b8ee5b5e914afdcaf399ed661a83b98d7) --- .../runtime/rete/index/IdentityIndexer.java | 15 ++- .../rete/index/MemoryIdentityIndexer.java | 11 ++- .../runtime/rete/index/MemoryNullIndexer.java | 9 +- .../query/runtime/rete/index/NullIndexer.java | 15 ++- .../index/SpecializedProjectionIndexer.java | 93 ++++++++++++++++++- .../runtime/rete/index/StandardIndexer.java | 7 +- .../rete/single/UniquenessEnforcerNode.java | 29 ++++-- 7 files changed, 154 insertions(+), 25 deletions(-) diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/IdentityIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/IdentityIndexer.java index dbb05aaf33..ec4b81dca5 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/IdentityIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/IdentityIndexer.java @@ -14,9 +14,11 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; +import java.util.List; import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; import org.eclipse.viatra.query.runtime.matchers.tuple.TupleMask; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer.ListenerSubscription; import org.eclipse.viatra.query.runtime.rete.network.Direction; import org.eclipse.viatra.query.runtime.rete.network.Node; import org.eclipse.viatra.query.runtime.rete.network.ReteContainer; @@ -28,13 +30,17 @@ * node). Do not attach parents directly! * * @author Gabor Bergmann + * @noimplement Rely on the provided implementations + * @noreference Use only via standard Node and Indexer interfaces + * @noinstantiate This class is not intended to be instantiated by clients. */ public abstract class IdentityIndexer extends SpecializedProjectionIndexer { protected abstract Collection getTuples(); - public IdentityIndexer(ReteContainer reteContainer, int tupleWidth, Supplier parent, Node activeNode) { - super(reteContainer, TupleMask.identity(tupleWidth), parent, activeNode); + public IdentityIndexer(ReteContainer reteContainer, int tupleWidth, Supplier parent, + Node activeNode, List sharedSubscriptionList) { + super(reteContainer, TupleMask.identity(tupleWidth), parent, activeNode, sharedSubscriptionList); } public Collection get(Tuple signature) { @@ -60,8 +66,9 @@ public Iterator iterator() { return getTuples().iterator(); } - public void propagate(Direction direction, Tuple updateElement) { - propagate(direction, updateElement, updateElement, true); + @Override + public void propagateToListener(IndexerListener listener, Direction direction, Tuple updateElement) { + listener.notifyIndexerUpdate(direction, updateElement, updateElement, true); } } \ No newline at end of file diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryIdentityIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryIdentityIndexer.java index 7a8d27c14a..795f2b33dc 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryIdentityIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryIdentityIndexer.java @@ -12,8 +12,10 @@ package org.eclipse.viatra.query.runtime.rete.index; import java.util.Collection; +import java.util.List; import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer.ListenerSubscription; import org.eclipse.viatra.query.runtime.rete.network.Receiver; import org.eclipse.viatra.query.runtime.rete.network.ReteContainer; import org.eclipse.viatra.query.runtime.rete.network.Supplier; @@ -23,6 +25,9 @@ * space. Can only exist in connection with a memory, and must be operated by another node. Do not attach parents * directly! * + * @noimplement Rely on the provided implementations + * @noreference Use only via standard Node and Indexer interfaces + * @noinstantiate This class is not intended to be instantiated by clients. * @author Gabor Bergmann */ @@ -38,9 +43,9 @@ public class MemoryIdentityIndexer extends IdentityIndexer { * @param parent * the parent node that owns the memory */ - public MemoryIdentityIndexer(ReteContainer reteContainer, int tupleWidth, Collection memory, - Supplier parent, Receiver activeNode) { - super(reteContainer, tupleWidth, parent, activeNode); + public MemoryIdentityIndexer(ReteContainer reteContainer, int tupleWidth, Collection memory, Supplier parent, + Receiver activeNode, List sharedSubscriptionList) { + super(reteContainer, tupleWidth, parent, activeNode, sharedSubscriptionList); this.memory = memory; } diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryNullIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryNullIndexer.java index f5ffae8fc0..05b4dd6b04 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryNullIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/MemoryNullIndexer.java @@ -12,8 +12,10 @@ package org.eclipse.viatra.query.runtime.rete.index; import java.util.Collection; +import java.util.List; import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer.ListenerSubscription; import org.eclipse.viatra.query.runtime.rete.network.Receiver; import org.eclipse.viatra.query.runtime.rete.network.ReteContainer; import org.eclipse.viatra.query.runtime.rete.network.Supplier; @@ -24,6 +26,9 @@ * directly! * * @author Gabor Bergmann + * @noimplement Rely on the provided implementations + * @noreference Use only via standard Node and Indexer interfaces + * @noinstantiate This class is not intended to be instantiated by clients. */ public class MemoryNullIndexer extends NullIndexer { @@ -39,8 +44,8 @@ public class MemoryNullIndexer extends NullIndexer { * the parent node that owns the memory */ public MemoryNullIndexer(ReteContainer reteContainer, int tupleWidth, Collection memory, Supplier parent, - Receiver activeNode) { - super(reteContainer, tupleWidth, parent, activeNode); + Receiver activeNode, List sharedSubscriptionList) { + super(reteContainer, tupleWidth, parent, activeNode, sharedSubscriptionList); this.memory = memory; } diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/NullIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/NullIndexer.java index 36762ab601..e2e1c3a233 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/NullIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/NullIndexer.java @@ -14,10 +14,12 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; +import java.util.List; import org.eclipse.viatra.query.runtime.matchers.tuple.FlatTuple; import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; import org.eclipse.viatra.query.runtime.matchers.tuple.TupleMask; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer.ListenerSubscription; import org.eclipse.viatra.query.runtime.rete.network.Direction; import org.eclipse.viatra.query.runtime.rete.network.Node; import org.eclipse.viatra.query.runtime.rete.network.ReteContainer; @@ -29,6 +31,9 @@ * active node). Do not attach parents directly! * * @author Gabor Bergmann + * @noimplement Rely on the provided implementations + * @noreference Use only via standard Node and Indexer interfaces + * @noinstantiate This class is not intended to be instantiated by clients. */ public abstract class NullIndexer extends SpecializedProjectionIndexer { @@ -39,8 +44,9 @@ public abstract class NullIndexer extends SpecializedProjectionIndexer { static Collection nullSingleton = Collections.singleton(nullSignature); static Collection emptySet = Collections.emptySet(); - public NullIndexer(ReteContainer reteContainer, int tupleWidth, Supplier parent, Node activeNode) { - super(reteContainer, TupleMask.linear(0, tupleWidth), parent, activeNode); + public NullIndexer(ReteContainer reteContainer, int tupleWidth, Supplier parent, + Node activeNode, List sharedSubscriptionList) { + super(reteContainer, TupleMask.linear(0, tupleWidth), parent, activeNode, sharedSubscriptionList); } public Collection get(Tuple signature) { @@ -69,10 +75,11 @@ public Iterator iterator() { return getTuples().iterator(); } - public void propagate(Direction direction, Tuple updateElement) { + @Override + public void propagateToListener(IndexerListener listener, Direction direction, Tuple updateElement) { boolean radical = (direction == Direction.REVOKE && isEmpty()) || (direction == Direction.INSERT && isSingleElement()); - propagate(direction, updateElement, nullSignature, radical); + listener.notifyIndexerUpdate(direction, updateElement, nullSignature, radical); } } \ No newline at end of file diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/SpecializedProjectionIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/SpecializedProjectionIndexer.java index 1ce8f75ac3..dca8482653 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/SpecializedProjectionIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/SpecializedProjectionIndexer.java @@ -11,7 +11,12 @@ package org.eclipse.viatra.query.runtime.rete.index; +import java.util.List; +import java.util.Objects; + +import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; import org.eclipse.viatra.query.runtime.matchers.tuple.TupleMask; +import org.eclipse.viatra.query.runtime.rete.network.Direction; import org.eclipse.viatra.query.runtime.rete.network.Node; import org.eclipse.viatra.query.runtime.rete.network.ReteContainer; import org.eclipse.viatra.query.runtime.rete.network.Supplier; @@ -19,22 +24,106 @@ /** * A specialized projection indexer that can be memory-less (relying on an external source of information). * - * @author Gabor Bergmann + *

All specialized projection indexers of a single node will share the same listener list, so + * that notification order is maintained (see Bug 518434). * + * @author Gabor Bergmann + * @noimplement Rely on the provided implementations + * @noreference Use only via standard Node and Indexer interfaces + * @noinstantiate This class is not intended to be instantiated by clients. */ public abstract class SpecializedProjectionIndexer extends StandardIndexer implements ProjectionIndexer { protected Node activeNode; + protected List sharedSubscriptionList; - public SpecializedProjectionIndexer(ReteContainer reteContainer, TupleMask mask, Supplier parent, Node activeNode) { + /** + * @since 1.7 + */ + public SpecializedProjectionIndexer(ReteContainer reteContainer, TupleMask mask, Supplier parent, + Node activeNode, List sharedSubscriptionList) { super(reteContainer, mask); this.parent = parent; this.activeNode = activeNode; + this.sharedSubscriptionList = sharedSubscriptionList; } @Override public Node getActiveNode() { return activeNode; } + + @Override + protected void propagate(Direction direction, Tuple updateElement, Tuple signature, boolean change) { + throw new UnsupportedOperationException(); + } + + @Override + public void attachListener(IndexerListener listener) { + super.attachListener(listener); + ListenerSubscription subscription = new ListenerSubscription(this, listener); + // See Bug 518434 + // Must add to the first position, so that the later listeners are notified earlier. + // Thus if the beta node added as listener is also an indirect descendant of the same indexer on its opposite slot, + // then the beta node is connected later than its ancestor's listener, therefore it will be notified earlier, + // eliminating duplicate insertions and lost deletions that would result from fall-through update propagation + sharedSubscriptionList.add(0, subscription); + } + @Override + public void detachListener(IndexerListener listener) { + super.detachListener(listener); + ListenerSubscription subscription = new ListenerSubscription(this, listener); + sharedSubscriptionList.remove(subscription); + } + + /** + * @since 1.7 + */ + public abstract void propagateToListener(IndexerListener listener, Direction direction, Tuple updateElement); + + /** + * Infrastructure to share subscriptions between specialized indexers of the same parent node. + * + * @author Gabor Bergmann + * @since 1.7 + */ + public static class ListenerSubscription { + protected SpecializedProjectionIndexer indexer; + protected IndexerListener listener; + + public ListenerSubscription(SpecializedProjectionIndexer indexer, IndexerListener listener) { + super(); + this.indexer = indexer; + this.listener = listener; + } + + /** + * Call this from parent node. + */ + public void propagate(Direction direction, Tuple updateElement) { + indexer.propagateToListener(listener, direction, updateElement); + } + + @Override + public int hashCode() { + return Objects.hash(indexer, listener); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + ListenerSubscription other = (ListenerSubscription) obj; + return Objects.equals(listener, other.listener) && + Objects.equals(indexer, other.indexer); + } + + + } + } diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/StandardIndexer.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/StandardIndexer.java index dba2575286..d83c07387c 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/StandardIndexer.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/index/StandardIndexer.java @@ -60,7 +60,12 @@ public Supplier getParent() { } public void attachListener(IndexerListener listener) { - listeners.add(listener); + // See Bug 518434 + // Must add to the first position, so that the later listeners are notified earlier. + // Thus if the beta node added as listener is also an indirect descendant of the same indexer on its opposite slot, + // then the beta node is connected later than its ancestor's listener, therefore it will be notified earlier, + // eliminating duplicate insertions and lost deletions that would result from fall-through update propagation + listeners.add(0, listener); reteContainer.getTracker().registerDependency(this, listener.getOwner()); } diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/single/UniquenessEnforcerNode.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/single/UniquenessEnforcerNode.java index 5924ad9ca3..8a93b2580e 100644 --- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/single/UniquenessEnforcerNode.java +++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/single/UniquenessEnforcerNode.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.List; import org.eclipse.viatra.query.runtime.matchers.context.IPosetComparator; import org.eclipse.viatra.query.runtime.matchers.tuple.Tuple; @@ -21,6 +22,8 @@ import org.eclipse.viatra.query.runtime.rete.index.MemoryIdentityIndexer; import org.eclipse.viatra.query.runtime.rete.index.MemoryNullIndexer; import org.eclipse.viatra.query.runtime.rete.index.ProjectionIndexer; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer; +import org.eclipse.viatra.query.runtime.rete.index.SpecializedProjectionIndexer.ListenerSubscription; import org.eclipse.viatra.query.runtime.rete.network.DefaultMailbox; import org.eclipse.viatra.query.runtime.rete.network.Direction; import org.eclipse.viatra.query.runtime.rete.network.Mailbox; @@ -69,6 +72,11 @@ public class UniquenessEnforcerNode extends StandardNode protected final Mailbox mailbox; private final TupleMask nullMask; private final TupleMask identityMask; + + /** + * @since 1.7 + */ + protected final List specializedListeners; public UniquenessEnforcerNode(ReteContainer reteContainer, int tupleWidth) { this(reteContainer, tupleWidth, false); @@ -104,6 +112,7 @@ public UniquenessEnforcerNode(ReteContainer reteContainer, int tupleWidth, boole TupleMask coreMask, TupleMask posetMask, IPosetComparator posetComparator) { super(reteContainer); this.parents = new ArrayList(); + this.specializedListeners = new ArrayList(); this.memory = new TupleMemory(); this.rederivableMemory = new TupleMemory(); this.tupleWidth = tupleWidth; @@ -280,14 +289,16 @@ public void rederiveOne() { * @since 1.6 */ protected void propagate(Direction direction, Tuple update) { - propagateUpdate(direction, update); - - if (memoryIdentityIndexer != null) { - memoryIdentityIndexer.propagate(direction, update); - } - if (memoryNullIndexer != null) { - memoryNullIndexer.propagate(direction, update); + // See Bug 518434 + // trivial (non-active) indexers must be updated before other listeners + // so that if they are joined against each other, trivial indexers lookups + // will be consistent with their notifications; + // also, their subscriptions must share a single order + for (ListenerSubscription subscription : specializedListeners) { + subscription.propagate(direction, update); } + + propagateUpdate(direction, update); } @Override @@ -316,7 +327,7 @@ public void pullInto(Collection collector) { public MemoryNullIndexer getNullIndexer() { if (memoryNullIndexer == null) { - memoryNullIndexer = new MemoryNullIndexer(reteContainer, tupleWidth, memory, this, this); + memoryNullIndexer = new MemoryNullIndexer(reteContainer, tupleWidth, memory, this, this, specializedListeners); reteContainer.getTracker().registerDependency(this, memoryNullIndexer); } return memoryNullIndexer; @@ -324,7 +335,7 @@ public MemoryNullIndexer getNullIndexer() { public MemoryIdentityIndexer getIdentityIndexer() { if (memoryIdentityIndexer == null) { - memoryIdentityIndexer = new MemoryIdentityIndexer(reteContainer, tupleWidth, memory, this, this); + memoryIdentityIndexer = new MemoryIdentityIndexer(reteContainer, tupleWidth, memory, this, this, specializedListeners); reteContainer.getTracker().registerDependency(this, memoryIdentityIndexer); } return memoryIdentityIndexer;