From 05f0d6b01ae1574126876fc4363b1dd985bad177 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 10 Jan 2024 08:55:44 +0100 Subject: [PATCH 01/20] Implemented a transitive closure that uses less intermediate containers --- .../PersistentHashIndexedBinaryRelation.java | 144 +++++------------- 1 file changed, 42 insertions(+), 102 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 86d7a0c0..bd86e11b 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -14,9 +14,7 @@ import static io.usethesource.vallang.impl.persistent.SetWriter.USE_MULTIMAP_BINARY_RELATIONS; import static io.usethesource.vallang.impl.persistent.SetWriter.asInstanceOf; import static io.usethesource.vallang.impl.persistent.SetWriter.isTupleOfArityTwo; - import java.util.Arrays; -import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Objects; @@ -24,10 +22,8 @@ import java.util.function.BiFunction; import java.util.stream.Stream; import java.util.stream.StreamSupport; - import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.Nullable; - import io.usethesource.capsule.Set; import io.usethesource.capsule.Set.Immutable; import io.usethesource.capsule.SetMultimap; @@ -39,12 +35,8 @@ import io.usethesource.vallang.ISetWriter; import io.usethesource.vallang.ITuple; import io.usethesource.vallang.IValue; -import io.usethesource.vallang.IValueFactory; -import io.usethesource.vallang.IWriter; -import io.usethesource.vallang.impl.util.collections.ShareableValuesHashSet; import io.usethesource.vallang.type.Type; import io.usethesource.vallang.util.AbstractTypeBag; -import io.usethesource.vallang.util.RotatingQueue; /** * Implements both ISet and IRelation, by indexing on the first column @@ -547,113 +539,61 @@ public ISet empty() { @Override public ISet closure() { - IWriter resultWriter = writer(); - resultWriter.insertAll(this); - resultWriter.insertAll(computeClosureDelta()); - return resultWriter.done(); + var result = computeClosure(content); + + // TODO: see if we can inline the bag calculation, it might however + // cost a lot more TypeBag allocations + final AbstractTypeBag keyTypeBag = result.entrySet().stream().map(Map.Entry::getKey) + .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + + final AbstractTypeBag valTypeBag = result.entrySet().stream().map(Map.Entry::getValue) + .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + + return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } - + @Override public ISet closureStar() { // calculate - ShareableValuesHashSet closureDelta = computeClosureDelta(); + var result = computeClosure(content); - IWriter resultWriter = writer(); - resultWriter.insertAll(this); - resultWriter.insertAll(closureDelta); - - for (IValue element : carrier()) { - resultWriter.insertTuple(element, element); + for (IValue carrier: result.keySet()) { + result.__insert(carrier, carrier); } + + final AbstractTypeBag keyTypeBag = result.entrySet().stream().map(Map.Entry::getKey) + .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - return resultWriter.done(); - } - - private ShareableValuesHashSet computeClosureDelta() { - IValueFactory vf = ValueFactory.getInstance(); - RotatingQueue iLeftKeys = new RotatingQueue<>(); - RotatingQueue> iLefts = new RotatingQueue<>(); - - Map> interestingLeftSides = new HashMap<>(); - Map potentialRightSides = new HashMap<>(); - - // Index - for (IValue val : this) { - ITuple tuple = (ITuple) val; - IValue key = tuple.get(0); - IValue value = tuple.get(1); - RotatingQueue leftValues = interestingLeftSides.get(key); - ShareableValuesHashSet rightValues; - - if (leftValues != null) { - rightValues = potentialRightSides.get(key); - } else { - leftValues = new RotatingQueue<>(); - iLeftKeys.put(key); - iLefts.put(leftValues); - interestingLeftSides.put(key, leftValues); - - rightValues = new ShareableValuesHashSet(); - potentialRightSides.put(key, rightValues); - } + final AbstractTypeBag valTypeBag = result.entrySet().stream().map(Map.Entry::getValue) + .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - leftValues.put(value); - if (rightValues == null) { - rightValues = new ShareableValuesHashSet(); - } - - rightValues.add(value); - } + return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); + } - int size = potentialRightSides.size(); - int nextSize = 0; - - // Compute - final ShareableValuesHashSet newTuples = new ShareableValuesHashSet(); - do { - Map rightSides = potentialRightSides; - potentialRightSides = new HashMap<>(); - - for (; size > 0; size--) { - IValue leftKey = iLeftKeys.get(); - RotatingQueue leftValues = iLefts.get(); - RotatingQueue interestingLeftValues = null; - - assert leftKey != null : "@AssumeAssertion(nullness) this only happens at the end of the queue"; - assert leftValues != null : "@AssumeAssertion(nullness) this only happens at the end of the queue"; - - IValue rightKey; - while ((rightKey = leftValues.get()) != null) { - ShareableValuesHashSet rightValues = rightSides.get(rightKey); - if (rightValues != null) { - Iterator rightValuesIterator = rightValues.iterator(); - while (rightValuesIterator.hasNext()) { - IValue rightValue = rightValuesIterator.next(); - if (newTuples.add(vf.tuple(leftKey, rightValue))) { - if (interestingLeftValues == null) { - nextSize++; - - iLeftKeys.put(leftKey); - interestingLeftValues = new RotatingQueue<>(); - iLefts.put(interestingLeftValues); - } - interestingLeftValues.put(rightValue); - - ShareableValuesHashSet potentialRightValues = potentialRightSides.get(rightKey); - if (potentialRightValues == null) { - potentialRightValues = new ShareableValuesHashSet(); - potentialRightSides.put(rightKey, potentialRightValues); - } - potentialRightValues.add(rightValue); + private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { + final SetMultimap.Transient result = content.asTransient(); + + SetMultimap todo = content.inverseMap(); + while (!todo.isEmpty()) { + final SetMultimap.Transient nextTodo = PersistentTrieSetMultimap.transientOf(Object::equals); + + // a pity we don't have a iterator over > + // not sure if the nativeIterator would be good for that + for (IValue lhs : todo.keySet()) { + Immutable values = content.get(lhs); + if (!values.isEmpty()) { + for (IValue key : todo.get(lhs)) { + for (IValue val: values) { + if (result.__insert(key, val)) { + nextTodo.__insert(val, key); } } } } } - size = nextSize; - nextSize = 0; - } while (size > 0); + todo = nextTodo; + } + return result; + } - return newTuples; - } } From 00591c9b7b16832637af1c9674b31144de1b9d4a Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 10 Jan 2024 09:09:08 +0100 Subject: [PATCH 02/20] Refactored typebag recalculation code --- .../PersistentHashIndexedBinaryRelation.java | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index bd86e11b..260f05ab 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -20,6 +20,7 @@ import java.util.Objects; import java.util.Optional; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -78,11 +79,9 @@ public IRelation asRelation() { private static final boolean checkDynamicType(final AbstractTypeBag keyTypeBag, final AbstractTypeBag valTypeBag, final SetMultimap.Immutable content) { - AbstractTypeBag expectedKeyTypeBag = content.entrySet().stream().map(Map.Entry::getKey) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - AbstractTypeBag expectedValTypeBag = content.entrySet().stream().map(Map.Entry::getValue) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + final AbstractTypeBag expectedKeyTypeBag = calcTypeBag(content, Map.Entry::getKey); + final AbstractTypeBag expectedValTypeBag = calcTypeBag(content, Map.Entry::getValue); boolean keyTypesEqual = expectedKeyTypeBag.equals(keyTypeBag); boolean valTypesEqual = expectedValTypeBag.equals(valTypeBag); @@ -436,11 +435,9 @@ public ISet compose(IRelation otherSetRelation) { final SetMultimap.Immutable data = xz.freeze(); - final AbstractTypeBag keyTypeBag = data.entrySet().stream().map(Map.Entry::getKey) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - - final AbstractTypeBag valTypeBag = data.entrySet().stream().map(Map.Entry::getValue) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + + final AbstractTypeBag keyTypeBag = calcTypeBag(data, Map.Entry::getKey); + final AbstractTypeBag valTypeBag = calcTypeBag(data, Map.Entry::getValue); return PersistentSetFactory.from(keyTypeBag, valTypeBag, data); } @@ -537,17 +534,19 @@ public ISet empty() { return ISet.super.empty(); } + private static AbstractTypeBag calcTypeBag(SetMultimap contents, Function, IValue> mapper) { + return contents.entrySet().stream().map(mapper) + .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + } + @Override public ISet closure() { var result = computeClosure(content); // TODO: see if we can inline the bag calculation, it might however // cost a lot more TypeBag allocations - final AbstractTypeBag keyTypeBag = result.entrySet().stream().map(Map.Entry::getKey) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - - final AbstractTypeBag valTypeBag = result.entrySet().stream().map(Map.Entry::getValue) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); + final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } @@ -560,12 +559,9 @@ public ISet closureStar() { for (IValue carrier: result.keySet()) { result.__insert(carrier, carrier); } - - final AbstractTypeBag keyTypeBag = result.entrySet().stream().map(Map.Entry::getKey) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); - final AbstractTypeBag valTypeBag = result.entrySet().stream().map(Map.Entry::getValue) - .map(IValue::getType).collect(AbstractTypeBag.toTypeBag()); + final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); + final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } From b1e32e209d960154ac3a1bc32aba65fc07e551c7 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 10 Jan 2024 09:25:49 +0100 Subject: [PATCH 03/20] Fixed reflexive closure --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 260f05ab..5d6fc91b 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -556,8 +556,9 @@ public ISet closureStar() { // calculate var result = computeClosure(content); - for (IValue carrier: result.keySet()) { - result.__insert(carrier, carrier); + for (var carrier: content.entrySet()) { + result.__insert(carrier.getKey(), carrier.getKey()); + result.__insert(carrier.getValue(), carrier.getValue()); } final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); From b10cece1da7a19a1ff9f25d1b3e3d54195302bb4 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 10:59:27 +0100 Subject: [PATCH 04/20] removed need for type bag computations --- .../PersistentHashIndexedBinaryRelation.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 5d6fc91b..40a8cb05 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -543,10 +543,28 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, public ISet closure() { var result = computeClosure(content); - // TODO: see if we can inline the bag calculation, it might however - // cost a lot more TypeBag allocations - final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); - final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); + Type tupleType = getElementType(); + assert tupleType.getArity() == 2; + Type keyType = tupleType.getFieldType(0); + Type valueType = tupleType.getFieldType(0); + + if (!keyType.comparable(valueType)) { + // if someone tries, this we have a very quick answer + return this; + } + + final AbstractTypeBag keyTypeBag = AbstractTypeBag.of(tupleType.getFieldType(0)); + final AbstractTypeBag valTypeBag = AbstractTypeBag.of(tupleType.getFieldType(1)); + + // Some theory here in comments, with _no code_ as a result. This prevents type bag calculation + // that is linear in the size of the resulting set. + // + // 1. If the type is symmetric (rel[x,x]), then obviously it can not change due to + // transitive closure. + // 2. If the type is not symmetric or even comparable, like `rel[int,str]` see above. Closure is a no-op then. + // 3. If the type is not symmetric but comparable, say `rel[int, num]`, then no new + // tuples can arise that are not tuple[int,int]. Hence the new type of the closure will + // be `lub(rel[int,int], rel[int, num]) == rel[int, num]` again. Same for `rel[num,int]`. return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } From 90ec6ea0cc6281b18110a8b6d124635619ef5eed Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 10:59:58 +0100 Subject: [PATCH 05/20] typo --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 40a8cb05..92894edc 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -549,7 +549,7 @@ public ISet closure() { Type valueType = tupleType.getFieldType(0); if (!keyType.comparable(valueType)) { - // if someone tries, this we have a very quick answer + // if someone tries, then we have a very quick answer return this; } From cb3569ea6f7c77ff407de18c9ab007958346b51b Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 11:01:01 +0100 Subject: [PATCH 06/20] skip computing closure for non-symmetric relations --- .../persistent/PersistentHashIndexedBinaryRelation.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 92894edc..93082a2d 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -541,8 +541,6 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, @Override public ISet closure() { - var result = computeClosure(content); - Type tupleType = getElementType(); assert tupleType.getArity() == 2; Type keyType = tupleType.getFieldType(0); @@ -553,13 +551,15 @@ public ISet closure() { return this; } + var result = computeClosure(content); + final AbstractTypeBag keyTypeBag = AbstractTypeBag.of(tupleType.getFieldType(0)); final AbstractTypeBag valTypeBag = AbstractTypeBag.of(tupleType.getFieldType(1)); // Some theory here in comments, with _no code_ as a result. This prevents type bag calculation // that is linear in the size of the resulting set. // - // 1. If the type is symmetric (rel[x,x]), then obviously it can not change due to + // 1. If the type is symmetric (`rel[x,x]`), then obviously it can not change due to // transitive closure. // 2. If the type is not symmetric or even comparable, like `rel[int,str]` see above. Closure is a no-op then. // 3. If the type is not symmetric but comparable, say `rel[int, num]`, then no new From b9aaedd9df9dc5d262ea284f6273afc36b2250d3 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 12:02:36 +0100 Subject: [PATCH 07/20] bugfix to get a good platform for the next optimization --- .../PersistentHashIndexedBinaryRelation.java | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 93082a2d..191f86af 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -540,7 +540,38 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, } @Override +/** + * we want to compute the closure of R, which in essence is a composition on itself. + * until nothing changes: + * + * solve(R) { + * R = R o R; + * } + * + * The algorithm below realizes the following things: + * + * - Instead of recomputing the compose for the whole of R, we only have to + * compose for the newly added edges (called todo in the algorithm). + * - Since the LHS of `R o R` will be using the range of R as a lookup in R + * we store the todo in inverse. + * + * In essence the algorithm becomes: + * + * result = R; + * todo = invert(R); + * + * while (todo != {}) { + * composed = fastCompose(todo, R); + * newEdges = composed - result; + * todo = invert(newEdges); + * result += newEdges; + * } + * + * fastCompose(todo, R) = { l * R[r] | <- todo}; + * + */ public ISet closure() { + Type tupleType = getElementType(); assert tupleType.getArity() == 2; Type keyType = tupleType.getFieldType(0); @@ -553,8 +584,8 @@ public ISet closure() { var result = computeClosure(content); - final AbstractTypeBag keyTypeBag = AbstractTypeBag.of(tupleType.getFieldType(0)); - final AbstractTypeBag valTypeBag = AbstractTypeBag.of(tupleType.getFieldType(1)); + final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); + final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); // Some theory here in comments, with _no code_ as a result. This prevents type bag calculation // that is linear in the size of the resulting set. @@ -606,6 +637,22 @@ private static SetMultimap.Transient computeClosure(final SetMul } } } + + // Iterator it = todo.tupleIterator((k,v) -> { + // // putting the side-effects here, make sure we do not have + // // to allocate intermediate entries or tuples + // if (result.__insert(k, v)) { + // nextTodo.__insert(v, k); + // } + + // return null; + // }); + + // // it.next() has side-effects on result and nextTodo. + // while (it.hasNext()) { + // it.next(); + // } + todo = nextTodo; } return result; From 7233d353a9a3e5e4fd2fe1329f3ec7656542b45d Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 13:50:23 +0100 Subject: [PATCH 08/20] better type bag approximation --- .../PersistentHashIndexedBinaryRelation.java | 14 +++++++++--- .../vallang/util/AbstractTypeBag.java | 22 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 191f86af..922512d1 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -571,7 +571,6 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, * */ public ISet closure() { - Type tupleType = getElementType(); assert tupleType.getArity() == 2; Type keyType = tupleType.getFieldType(0); @@ -584,8 +583,17 @@ public ISet closure() { var result = computeClosure(content); - final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); - final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); + final AbstractTypeBag keyTypeBag; + final AbstractTypeBag valTypeBag; + + if (keyType == valueType && (keyType.isInteger() || keyType.isReal() || keyType.isRational() || keyType.isString() || keyType.isSourceLocation())) { + keyTypeBag = AbstractTypeBag.of(keyType, result.size()); + valTypeBag = calcTypeBag(result, Map.Entry::getValue); + } + else { + keyTypeBag = calcTypeBag(result, Map.Entry::getKey); + valTypeBag = calcTypeBag(result, Map.Entry::getValue); + } // Some theory here in comments, with _no code_ as a result. This prevents type bag calculation // that is linear in the size of the resulting set. diff --git a/src/main/java/io/usethesource/vallang/util/AbstractTypeBag.java b/src/main/java/io/usethesource/vallang/util/AbstractTypeBag.java index 3fcd7da0..c228f3c2 100644 --- a/src/main/java/io/usethesource/vallang/util/AbstractTypeBag.java +++ b/src/main/java/io/usethesource/vallang/util/AbstractTypeBag.java @@ -33,6 +33,8 @@ */ public abstract class AbstractTypeBag implements Cloneable { + public abstract AbstractTypeBag increase(Type t, int count); + public abstract AbstractTypeBag increase(Type t); public abstract AbstractTypeBag decrease(Type t); @@ -47,6 +49,10 @@ public static AbstractTypeBag of(Type... ts) { return TypeBag.of(ts); } + public static AbstractTypeBag of(Type type, int count) { + return TypeBag.of(type, count); + } + public abstract int size(); /** @@ -76,13 +82,18 @@ public static final AbstractTypeBag of(final Type... ts) { return result; } + public static final AbstractTypeBag of(Type t, int count) { + return new TypeBag(Map.Immutable.of()) + .increase(t, count); + } + @Override - public AbstractTypeBag increase(Type t) { + public AbstractTypeBag increase(Type t, int count) { final Integer oldCount = countMap.get(t); final Map.Immutable newCountMap; if (oldCount == null) { - newCountMap = countMap.__put(t, 1); + newCountMap = countMap.__put(t, count); if (cachedLub == null) { return new TypeBag(newCountMap); @@ -92,11 +103,16 @@ public AbstractTypeBag increase(Type t) { return new TypeBag(newCountMap, newCachedLub); } } else { - newCountMap = countMap.__put(t, oldCount + 1); + newCountMap = countMap.__put(t, oldCount + count); return new TypeBag(newCountMap); } } + @Override + public AbstractTypeBag increase(Type t) { + return increase(t, 1); + } + @Override public AbstractTypeBag decrease(Type t) { final Integer oldCount = countMap.get(t); From 686270316338d16e87e9f07f5c6f0cd2812cdef9 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 14:03:15 +0100 Subject: [PATCH 09/20] refactorig --- .../PersistentHashIndexedBinaryRelation.java | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 922512d1..684491b1 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -14,6 +14,8 @@ import static io.usethesource.vallang.impl.persistent.SetWriter.USE_MULTIMAP_BINARY_RELATIONS; import static io.usethesource.vallang.impl.persistent.SetWriter.asInstanceOf; import static io.usethesource.vallang.impl.persistent.SetWriter.isTupleOfArityTwo; +import static io.usethesource.vallang.impl.persistent.ValueCollectors.toList; + import java.util.Arrays; import java.util.Iterator; import java.util.Map; @@ -37,6 +39,7 @@ import io.usethesource.vallang.ITuple; import io.usethesource.vallang.IValue; import io.usethesource.vallang.type.Type; +import io.usethesource.vallang.type.TypeFactory; import io.usethesource.vallang.util.AbstractTypeBag; /** @@ -586,28 +589,35 @@ public ISet closure() { final AbstractTypeBag keyTypeBag; final AbstractTypeBag valTypeBag; - if (keyType == valueType && (keyType.isInteger() || keyType.isReal() || keyType.isRational() || keyType.isString() || keyType.isSourceLocation())) { + if (keyType == valueType && isConcreteValueType(keyType)) { + // this means no other types can be introduced other than the originals, + // so iteration is no longer necessary to construct the new type bag keyTypeBag = AbstractTypeBag.of(keyType, result.size()); - valTypeBag = calcTypeBag(result, Map.Entry::getValue); + valTypeBag = AbstractTypeBag.of(valueType, result.size()); } else { keyTypeBag = calcTypeBag(result, Map.Entry::getKey); valTypeBag = calcTypeBag(result, Map.Entry::getValue); } - // Some theory here in comments, with _no code_ as a result. This prevents type bag calculation - // that is linear in the size of the resulting set. - // - // 1. If the type is symmetric (`rel[x,x]`), then obviously it can not change due to - // transitive closure. - // 2. If the type is not symmetric or even comparable, like `rel[int,str]` see above. Closure is a no-op then. - // 3. If the type is not symmetric but comparable, say `rel[int, num]`, then no new - // tuples can arise that are not tuple[int,int]. Hence the new type of the closure will - // be `lub(rel[int,int], rel[int, num]) == rel[int, num]` again. Same for `rel[num,int]`. - return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } + private boolean isConcreteValueType(Type keyType) { + if (keyType.isList() || keyType.isSet() || keyType.isMap()) { + return false; + } + + if (keyType.isAbstractData() && keyType.isParameterized()) { + return false; // could have abstract type parameters that can be different for different tuples + } + + Type voidType = TypeFactory.getInstance().voidType(); + + // this is a quick check for int, real, rat, loc, str (not num, not node, etc) + return keyType.glb(voidType) == voidType; + } + @Override public ISet closureStar() { // calculate From 2ea42ee74c6394827995f28616ca8ebd9b5cf5d1 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 10 Jan 2024 14:42:31 +0100 Subject: [PATCH 10/20] implemented closureStar quicker and clean up experiments --- .../PersistentHashIndexedBinaryRelation.java | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 684491b1..b82c7e51 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -620,6 +620,11 @@ private boolean isConcreteValueType(Type keyType) { @Override public ISet closureStar() { + Type tupleType = getElementType(); + assert tupleType.getArity() == 2; + Type keyType = tupleType.getFieldType(0); + Type valueType = tupleType.getFieldType(0); + // calculate var result = computeClosure(content); @@ -628,8 +633,19 @@ public ISet closureStar() { result.__insert(carrier.getValue(), carrier.getValue()); } - final AbstractTypeBag keyTypeBag = calcTypeBag(result, Map.Entry::getKey); - final AbstractTypeBag valTypeBag = calcTypeBag(result, Map.Entry::getValue); + final AbstractTypeBag keyTypeBag; + final AbstractTypeBag valTypeBag; + + if (keyType == valueType && isConcreteValueType(keyType)) { + // this means no other types can be introduced other than the originals, + // so iteration is no longer necessary to construct the new type bag + keyTypeBag = AbstractTypeBag.of(keyType, result.size()); + valTypeBag = AbstractTypeBag.of(valueType, result.size()); + } + else { + keyTypeBag = calcTypeBag(result, Map.Entry::getKey); + valTypeBag = calcTypeBag(result, Map.Entry::getValue); + } return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } @@ -656,23 +672,9 @@ private static SetMultimap.Transient computeClosure(final SetMul } } - // Iterator it = todo.tupleIterator((k,v) -> { - // // putting the side-effects here, make sure we do not have - // // to allocate intermediate entries or tuples - // if (result.__insert(k, v)) { - // nextTodo.__insert(v, k); - // } - - // return null; - // }); - - // // it.next() has side-effects on result and nextTodo. - // while (it.hasNext()) { - // it.next(); - // } - todo = nextTodo; } + return result; } From e3e1a4717aa517ce3a247ac1d46976edea43b381 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 10 Jan 2024 17:55:44 +0100 Subject: [PATCH 11/20] Moved comment to the right function --- .../PersistentHashIndexedBinaryRelation.java | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index b82c7e51..54c9bcdd 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -543,36 +543,7 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, } @Override -/** - * we want to compute the closure of R, which in essence is a composition on itself. - * until nothing changes: - * - * solve(R) { - * R = R o R; - * } - * - * The algorithm below realizes the following things: - * - * - Instead of recomputing the compose for the whole of R, we only have to - * compose for the newly added edges (called todo in the algorithm). - * - Since the LHS of `R o R` will be using the range of R as a lookup in R - * we store the todo in inverse. - * - * In essence the algorithm becomes: - * - * result = R; - * todo = invert(R); - * - * while (todo != {}) { - * composed = fastCompose(todo, R); - * newEdges = composed - result; - * todo = invert(newEdges); - * result += newEdges; - * } - * - * fastCompose(todo, R) = { l * R[r] | <- todo}; - * - */ + public ISet closure() { Type tupleType = getElementType(); assert tupleType.getArity() == 2; @@ -651,6 +622,36 @@ public ISet closureStar() { } private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { + /* + * we want to compute the closure of R, which in essence is a composition on itself. + * until nothing changes: + * + * solve(R) { + * R = R o R; + * } + * + * The algorithm below realizes the following things: + * + * - Instead of recomputing the compose for the whole of R, we only have to + * compose for the newly added edges (called todo in the algorithm). + * - Since the LHS of `R o R` will be using the range of R as a lookup in R + * we store the todo in inverse. + * + * In essence the algorithm becomes: + * + * result = R; + * todo = invert(R); + * + * while (todo != {}) { + * composed = fastCompose(todo, R); + * newEdges = composed - result; + * todo = invert(newEdges); + * result += newEdges; + * } + * + * fastCompose(todo, R) = { l * R[r] | <- todo}; + * + */ final SetMultimap.Transient result = content.asTransient(); SetMultimap todo = content.inverseMap(); From 337de0ac4f003516828920e808da1ac2a00401b0 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 10 Jan 2024 17:55:55 +0100 Subject: [PATCH 12/20] Avoid duplicate type bag --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 54c9bcdd..bc48eae1 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -564,7 +564,7 @@ public ISet closure() { // this means no other types can be introduced other than the originals, // so iteration is no longer necessary to construct the new type bag keyTypeBag = AbstractTypeBag.of(keyType, result.size()); - valTypeBag = AbstractTypeBag.of(valueType, result.size()); + valTypeBag = keyTypeBag; } else { keyTypeBag = calcTypeBag(result, Map.Entry::getKey); @@ -611,7 +611,7 @@ public ISet closureStar() { // this means no other types can be introduced other than the originals, // so iteration is no longer necessary to construct the new type bag keyTypeBag = AbstractTypeBag.of(keyType, result.size()); - valTypeBag = AbstractTypeBag.of(valueType, result.size()); + valTypeBag = keyTypeBag; } else { keyTypeBag = calcTypeBag(result, Map.Entry::getKey); From 209ba1d130be631d650dea417d3fb033bed6bc27 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 11 Jan 2024 20:57:41 +0100 Subject: [PATCH 13/20] Added two modes of how to compute transitive closure --- .../PersistentHashIndexedBinaryRelation.java | 86 +++++++++++++++++-- 1 file changed, 77 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index bc48eae1..c885b5d1 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -15,8 +15,9 @@ import static io.usethesource.vallang.impl.persistent.SetWriter.asInstanceOf; import static io.usethesource.vallang.impl.persistent.SetWriter.isTupleOfArityTwo; import static io.usethesource.vallang.impl.persistent.ValueCollectors.toList; - +import java.util.ArrayDeque; import java.util.Arrays; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.Objects; @@ -30,6 +31,9 @@ import io.usethesource.capsule.Set; import io.usethesource.capsule.Set.Immutable; import io.usethesource.capsule.SetMultimap; +import io.usethesource.capsule.Map.Transient; +import io.usethesource.capsule.core.PersistentTrieMap; +import io.usethesource.capsule.core.PersistentTrieSet; import io.usethesource.capsule.core.PersistentTrieSetMultimap; import io.usethesource.capsule.util.ArrayUtilsInt; import io.usethesource.capsule.util.stream.CapsuleCollectors; @@ -622,6 +626,20 @@ public ISet closureStar() { } private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { + // TODO: consider switching between the modes during the job (if we can detect it correctly) + if (isDeep(content)) { + return computeClosureDepthFirst(content); + } + return computeClosureBreathFirst(content); + } + + private static boolean isDeep(io.usethesource.capsule.SetMultimap.Immutable content) { + // TODO: improve heuristic + return content.size() > 12; + } + + @SuppressWarnings("unchecked") + private static SetMultimap.Transient computeClosureBreathFirst(final SetMultimap.Immutable content) { /* * we want to compute the closure of R, which in essence is a composition on itself. * until nothing changes: @@ -658,18 +676,24 @@ private static SetMultimap.Transient computeClosure(final SetMul while (!todo.isEmpty()) { final SetMultimap.Transient nextTodo = PersistentTrieSetMultimap.transientOf(Object::equals); - // a pity we don't have a iterator over > - // not sure if the nativeIterator would be good for that - for (IValue lhs : todo.keySet()) { + var todoIt = todo.nativeEntryIterator(); + while (todoIt.hasNext()) { + var next = todoIt.next(); + IValue lhs = next.getKey(); Immutable values = content.get(lhs); if (!values.isEmpty()) { - for (IValue key : todo.get(lhs)) { - for (IValue val: values) { - if (result.__insert(key, val)) { - nextTodo.__insert(val, key); - } + Object keys = next.getValue(); + if (keys instanceof IValue) { + singleCompose(result, nextTodo, values, (IValue)keys); + } + else if (keys instanceof Set) { + for (IValue key : (Set)keys) { + singleCompose(result, nextTodo, values, key); } } + else { + throw new IllegalArgumentException("Unexpected map entry"); + } } } @@ -679,4 +703,48 @@ private static SetMultimap.Transient computeClosure(final SetMul return result; } + private static void singleCompose(final SetMultimap.Transient result, + final SetMultimap.Transient nextTodo, Immutable values, IValue key) { + for (IValue val: values) { + if (result.__insert(key, val)) { + nextTodo.__insert(val, key); + } + } + } + + @SuppressWarnings("unchecked") + private static SetMultimap.Transient computeClosureDepthFirst(final SetMultimap.Immutable content) { + final SetMultimap.Transient result = content.asTransient(); + var todo = new ArrayDeque(); + var done = new HashSet(); // keep track of LHS we already did, so we don't have to go into the depth of them anymore + var mainIt = content.nativeEntryIterator(); + while (mainIt.hasNext()) { + final var focus = mainIt.next(); + final IValue lhs = focus.getKey(); + final Object values =focus.getValue(); + + todo.clear(); + if (values instanceof IValue) { + todo.push((IValue)values); + } + else if (values instanceof Set) { + todo.addAll((Set)values); + } + else { + throw new IllegalArgumentException("Unexpected map entry"); + } + // we mark ourselves as done before we did it, + // so we don't do the that causes an extra round + done.add(lhs); + IValue rhs; + while ((rhs = todo.poll()) != null) { + for (IValue composed : result.get(rhs)) { + if (result.__insert(lhs, composed) && !done.contains(composed)) { + todo.push(composed); + } + } + } + } + return result; + } } From 7c173bee05623da08e720859d68586ed79598e22 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 12 Jan 2024 08:45:49 +0100 Subject: [PATCH 14/20] Fixed bug in depth first closure calculation --- .../PersistentHashIndexedBinaryRelation.java | 5 ++-- .../vallang/specification/IRelationTests.java | 26 ++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index c885b5d1..07d14976 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -734,12 +734,13 @@ else if (values instanceof Set) { throw new IllegalArgumentException("Unexpected map entry"); } // we mark ourselves as done before we did it, - // so we don't do the that causes an extra round + // so we don't do the that causes an useless round done.add(lhs); IValue rhs; while ((rhs = todo.poll()) != null) { + boolean rhsFull = done.contains(rhs); for (IValue composed : result.get(rhs)) { - if (result.__insert(lhs, composed) && !done.contains(composed)) { + if (result.__insert(lhs, composed) && !rhsFull) { todo.push(composed); } } diff --git a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java index 4a14f8b2..e192b7ba 100644 --- a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java +++ b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java @@ -12,6 +12,7 @@ import io.usethesource.vallang.ExpectedType; import io.usethesource.vallang.GivenValue; import io.usethesource.vallang.IList; +import io.usethesource.vallang.IRelation; import io.usethesource.vallang.ISet; import io.usethesource.vallang.ISourceLocation; import io.usethesource.vallang.ITuple; @@ -55,7 +56,30 @@ public void transClosureLocs(@ExpectedType("rel[loc,loc]") ISet src) { assertEquals(src.asRelation().closure().intersect(src), src); } - + @ParameterizedTest @ArgumentsSource(ValueProvider.class) + public void deepClosure(IValueFactory vf) { + IValue prev = vf.integer(0); + var rBuilder = vf.setWriter(); + for (int i=1; i < 100; i++) { + IValue next = vf.integer(i); + rBuilder.appendTuple(prev, next); + prev = next; + } + var r = rBuilder.done().asRelation(); + + assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + } + + private ISet slowClosure(IValueFactory vf, IRelation r) { + var prev = vf.set(); + ISet result = r.asContainer(); + while (!prev.equals(result)) { + prev = result; + result = result.union( r.compose(result.asRelation())); + } + return result; + } + @ParameterizedTest @ArgumentsSource(ValueProvider.class) public void carrierForTriples(@ExpectedType("rel[int,int,int]") ISet src) { ISet carrier = src.asRelation().carrier(); From efd2ee7d8fa61f454c03a1c8cb0284510ece96c7 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 10:46:08 +0100 Subject: [PATCH 15/20] removed unused imports --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 07d14976..e4d0168c 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -14,7 +14,6 @@ import static io.usethesource.vallang.impl.persistent.SetWriter.USE_MULTIMAP_BINARY_RELATIONS; import static io.usethesource.vallang.impl.persistent.SetWriter.asInstanceOf; import static io.usethesource.vallang.impl.persistent.SetWriter.isTupleOfArityTwo; -import static io.usethesource.vallang.impl.persistent.ValueCollectors.toList; import java.util.ArrayDeque; import java.util.Arrays; import java.util.HashSet; @@ -31,9 +30,6 @@ import io.usethesource.capsule.Set; import io.usethesource.capsule.Set.Immutable; import io.usethesource.capsule.SetMultimap; -import io.usethesource.capsule.Map.Transient; -import io.usethesource.capsule.core.PersistentTrieMap; -import io.usethesource.capsule.core.PersistentTrieSet; import io.usethesource.capsule.core.PersistentTrieSetMultimap; import io.usethesource.capsule.util.ArrayUtilsInt; import io.usethesource.capsule.util.stream.CapsuleCollectors; From e157760df811c2069da9aa28571921ad28c14446 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 12:13:23 +0100 Subject: [PATCH 16/20] measured and compared two closure algorithms for different sizes and shapes of relations. The depth first pretty much always wins, except for small graphs which are one big long chain. Since this never really happens, we go for the depth first version always --- .../PersistentHashIndexedBinaryRelation.java | 96 +------------------ 1 file changed, 4 insertions(+), 92 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index e4d0168c..7990e5e3 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -555,7 +555,7 @@ public ISet closure() { return this; } - var result = computeClosure(content); + var result = computeClosureDepthFirst(content); final AbstractTypeBag keyTypeBag; final AbstractTypeBag valTypeBag; @@ -597,7 +597,7 @@ public ISet closureStar() { Type valueType = tupleType.getFieldType(0); // calculate - var result = computeClosure(content); + var result = computeClosureDepthFirst(content); for (var carrier: content.entrySet()) { result.__insert(carrier.getKey(), carrier.getKey()); @@ -621,93 +621,6 @@ public ISet closureStar() { return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } - private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { - // TODO: consider switching between the modes during the job (if we can detect it correctly) - if (isDeep(content)) { - return computeClosureDepthFirst(content); - } - return computeClosureBreathFirst(content); - } - - private static boolean isDeep(io.usethesource.capsule.SetMultimap.Immutable content) { - // TODO: improve heuristic - return content.size() > 12; - } - - @SuppressWarnings("unchecked") - private static SetMultimap.Transient computeClosureBreathFirst(final SetMultimap.Immutable content) { - /* - * we want to compute the closure of R, which in essence is a composition on itself. - * until nothing changes: - * - * solve(R) { - * R = R o R; - * } - * - * The algorithm below realizes the following things: - * - * - Instead of recomputing the compose for the whole of R, we only have to - * compose for the newly added edges (called todo in the algorithm). - * - Since the LHS of `R o R` will be using the range of R as a lookup in R - * we store the todo in inverse. - * - * In essence the algorithm becomes: - * - * result = R; - * todo = invert(R); - * - * while (todo != {}) { - * composed = fastCompose(todo, R); - * newEdges = composed - result; - * todo = invert(newEdges); - * result += newEdges; - * } - * - * fastCompose(todo, R) = { l * R[r] | <- todo}; - * - */ - final SetMultimap.Transient result = content.asTransient(); - - SetMultimap todo = content.inverseMap(); - while (!todo.isEmpty()) { - final SetMultimap.Transient nextTodo = PersistentTrieSetMultimap.transientOf(Object::equals); - - var todoIt = todo.nativeEntryIterator(); - while (todoIt.hasNext()) { - var next = todoIt.next(); - IValue lhs = next.getKey(); - Immutable values = content.get(lhs); - if (!values.isEmpty()) { - Object keys = next.getValue(); - if (keys instanceof IValue) { - singleCompose(result, nextTodo, values, (IValue)keys); - } - else if (keys instanceof Set) { - for (IValue key : (Set)keys) { - singleCompose(result, nextTodo, values, key); - } - } - else { - throw new IllegalArgumentException("Unexpected map entry"); - } - } - } - - todo = nextTodo; - } - - return result; - } - - private static void singleCompose(final SetMultimap.Transient result, - final SetMultimap.Transient nextTodo, Immutable values, IValue key) { - for (IValue val: values) { - if (result.__insert(key, val)) { - nextTodo.__insert(val, key); - } - } - } - @SuppressWarnings("unchecked") private static SetMultimap.Transient computeClosureDepthFirst(final SetMultimap.Immutable content) { final SetMultimap.Transient result = content.asTransient(); @@ -730,13 +643,12 @@ else if (values instanceof Set) { throw new IllegalArgumentException("Unexpected map entry"); } // we mark ourselves as done before we did it, - // so we don't do the that causes an useless round + // so we don't do the that causes an extra round done.add(lhs); IValue rhs; while ((rhs = todo.poll()) != null) { - boolean rhsFull = done.contains(rhs); for (IValue composed : result.get(rhs)) { - if (result.__insert(lhs, composed) && !rhsFull) { + if (result.__insert(lhs, composed) && !done.contains(composed)) { todo.push(composed); } } From 862e33efcb7554bc80708637b20c74d591180103 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 12 Jan 2024 13:16:36 +0100 Subject: [PATCH 17/20] Brought back fix --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 7990e5e3..6655af9d 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -647,8 +647,9 @@ else if (values instanceof Set) { done.add(lhs); IValue rhs; while ((rhs = todo.poll()) != null) { + boolean rhsDone = done.contains(rhs); for (IValue composed : result.get(rhs)) { - if (result.__insert(lhs, composed) && !done.contains(composed)) { + if (result.__insert(lhs, composed) && !rhsDone) { todo.push(composed); } } From ac0745223c9ab700c85ad6385ed70a23a541e77d Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 12 Jan 2024 13:25:40 +0100 Subject: [PATCH 18/20] Added more tests for the closure --- .../vallang/specification/IRelationTests.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java index e192b7ba..c15e3b3e 100644 --- a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java +++ b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java @@ -70,6 +70,47 @@ public void deepClosure(IValueFactory vf) { assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); } + @ParameterizedTest @ArgumentsSource(ValueProvider.class) + public void broadClosure(IValueFactory vf) { + IValue prev = vf.integer(0); + var rBuilder = vf.setWriter(); + for (int i=1; i < 100; i++) { + IValue next = vf.integer(i); + if (i % 5 != 0) { + rBuilder.appendTuple(prev, next); + } + prev = next; + } + var r = rBuilder.done().asRelation(); + + assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + } + + @ParameterizedTest @ArgumentsSource(ValueProvider.class) + public void severalDeep(IValueFactory vf) { + IValue prev = vf.integer(0); + var rBuilder = vf.setWriter(); + for (int i=1; i < 100; i++) { + IValue next = vf.integer(i); + if (i % 50 != 0) { + rBuilder.appendTuple(prev, next); + } + prev = next; + } + // now let's add some side paths + prev = vf.integer(10); + for (int i=11; i < 100; i+=2) { + IValue next = vf.integer(i); + if (i % 50 != 0) { + rBuilder.appendTuple(prev, next); + } + prev = next; + } + var r = rBuilder.done().asRelation(); + + assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + } + private ISet slowClosure(IValueFactory vf, IRelation r) { var prev = vf.set(); ISet result = r.asContainer(); From 60ecedfce9cb32c8816c51928628bf1e40442501 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 14:23:36 +0100 Subject: [PATCH 19/20] removed newline --- .../impl/persistent/PersistentHashIndexedBinaryRelation.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 6655af9d..93b55317 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -543,7 +543,6 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, } @Override - public ISet closure() { Type tupleType = getElementType(); assert tupleType.getArity() == 2; From ad6e05c3d3d1cbc9102f4f637c9dbdded2a12eaf Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 14:26:37 +0100 Subject: [PATCH 20/20] brought back the breadth first version for relations smaller than 256 elements --- .../PersistentHashIndexedBinaryRelation.java | 87 ++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 93b55317..6d9b4627 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -554,7 +554,7 @@ public ISet closure() { return this; } - var result = computeClosureDepthFirst(content); + var result = computeClosure(content); final AbstractTypeBag keyTypeBag; final AbstractTypeBag valTypeBag; @@ -595,8 +595,7 @@ public ISet closureStar() { Type keyType = tupleType.getFieldType(0); Type valueType = tupleType.getFieldType(0); - // calculate - var result = computeClosureDepthFirst(content); + var result = computeClosure(content); for (var carrier: content.entrySet()) { result.__insert(carrier.getKey(), carrier.getKey()); @@ -620,6 +619,13 @@ public ISet closureStar() { return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } + private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { + return content.size() > 256 + ? computeClosureDepthFirst(content) + : computeClosureBreadthFirst(content) + ; + } + @SuppressWarnings("unchecked") private static SetMultimap.Transient computeClosureDepthFirst(final SetMultimap.Immutable content) { final SetMultimap.Transient result = content.asTransient(); @@ -656,4 +662,79 @@ else if (values instanceof Set) { } return result; } + + @SuppressWarnings("unchecked") + private static SetMultimap.Transient computeClosureBreadthFirst(final SetMultimap.Immutable content) { + /* + * we want to compute the closure of R, which in essence is a composition on itself. + * until nothing changes: + * + * solve(R) { + * R = R o R; + * } + * + * The algorithm below realizes the following things: + * + * - Instead of recomputing the compose for the whole of R, we only have to + * compose for the newly added edges (called todo in the algorithm). + * - Since the LHS of `R o R` will be using the range of R as a lookup in R + * we store the todo in inverse. + * + * In essence the algorithm becomes: + * + * result = R; + * todo = invert(R); + * + * while (todo != {}) { + * composed = fastCompose(todo, R); + * newEdges = composed - result; + * todo = invert(newEdges); + * result += newEdges; + * } + * + * fastCompose(todo, R) = { l * R[r] | <- todo}; + * + */ + final SetMultimap.Transient result = content.asTransient(); + + SetMultimap todo = content.inverseMap(); + while (!todo.isEmpty()) { + final SetMultimap.Transient nextTodo = PersistentTrieSetMultimap.transientOf(Object::equals); + + var todoIt = todo.nativeEntryIterator(); + while (todoIt.hasNext()) { + var next = todoIt.next(); + IValue lhs = next.getKey(); + Immutable values = content.get(lhs); + if (!values.isEmpty()) { + Object keys = next.getValue(); + if (keys instanceof IValue) { + singleCompose(result, nextTodo, values, (IValue)keys); + } + else if (keys instanceof Set) { + for (IValue key : (Set)keys) { + singleCompose(result, nextTodo, values, key); + } + } + else { + throw new IllegalArgumentException("Unexpected map entry"); + } + } + } + + todo = nextTodo; + } + + return result; + } + + private static void singleCompose(final SetMultimap.Transient result, + final SetMultimap.Transient nextTodo, Immutable values, IValue key) { + for (IValue val: values) { + if (result.__insert(key, val)) { + nextTodo.__insert(val, key); + } + } + } + }