Skip to content

Commit 55d0876

Browse files
triceotriceo
committed
Optimize joins
1 parent e093e19 commit 55d0876

File tree

1 file changed

+60
-48
lines changed

1 file changed

+60
-48
lines changed

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractJoinNode.java‎

Lines changed: 60 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleStorePositionTracker;
1111
import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
1212

13-
import org.jspecify.annotations.Nullable;
14-
1513
/**
1614
* This class has two direct children: {@link AbstractIndexedJoinNode} and {@link AbstractUnindexedJoinNode}.
1715
* The logic in either is identical, except that the latter removes all indexing work.
@@ -92,13 +90,50 @@ protected final void innerUpdateLeft(LeftTuple_ leftTuple, Consumer<Consumer<Uni
9290
if (!isFiltering) {
9391
outTupleSetLeft.forEach(outTuple -> updateOutTupleLeft(outTuple, leftTuple));
9492
} else {
95-
rightTupleConsumer.accept(rightTuple -> {
96-
IndexedSet<OutTuple_> outTupleSetRight = rightTuple.getStore(inputStoreIndexRightOutTupleSet);
97-
processOutTupleUpdate(leftTuple, rightTuple, outTupleSetRight, outTupleSetLeft, outputStoreIndexRightOutSet);
98-
});
93+
if (!leftTuple.state.isActive()) {
94+
// Assume the following scenario:
95+
// - The join is of two entities of the same type, both filtering out unassigned.
96+
// - One entity became unassigned, so the outTuple is getting retracted.
97+
// - The other entity is still assigned and is being updated.
98+
//
99+
// This means the filter would be called with (unassignedEntity, assignedEntity),
100+
// which breaks the expectation that the filter is only called on two assigned entities
101+
// and requires adding null checks to the filter for something that should intuitively be impossible.
102+
// We avoid this situation as it is clear that the outTuple must be retracted anyway,
103+
// and therefore any further updates to it are pointless.
104+
//
105+
// It is possible that the same problem would exist coming from the other side as well,
106+
// and therefore the right tuple would have to be checked for active state as well.
107+
// However, no such issue could have been reproduced; when in doubt, leave it out.
108+
return;
109+
}
110+
rightTupleConsumer.accept(rightTuple -> findAndProcessOutTuple(leftTuple, rightTuple, outTupleSetLeft,
111+
rightTuple.getStore(inputStoreIndexRightOutTupleSet), outputStoreIndexRightOutSet));
112+
}
113+
}
114+
115+
private void findAndProcessOutTuple(LeftTuple_ leftTuple, UniTuple<Right_> rightTuple, IndexedSet<OutTuple_> sourceSet,
116+
IndexedSet<OutTuple_> referenceSet, int outputStoreIndex) {
117+
var outTuple = findOutTuple(sourceSet, referenceSet, outputStoreIndex);
118+
if (testFiltering(leftTuple, rightTuple)) {
119+
if (outTuple == null) {
120+
insertOutTuple(leftTuple, rightTuple);
121+
} else {
122+
updateOutTupleLeft(outTuple, leftTuple);
123+
}
124+
} else {
125+
if (outTuple != null) {
126+
retractOutTuple(outTuple);
127+
}
99128
}
100129
}
101130

131+
private static <OutTuple_ extends AbstractTuple> OutTuple_ findOutTuple(IndexedSet<OutTuple_> sourceSet,
132+
IndexedSet<OutTuple_> referenceSet, int outputStoreIndex) {
133+
// Hack: outTuple has no left/right input tuple reference, use the left/right outSet reference instead.
134+
return sourceSet.findFirst(tuple -> referenceSet == tuple.getStore(outputStoreIndex));
135+
}
136+
102137
private void updateOutTupleLeft(OutTuple_ outTuple, LeftTuple_ leftTuple) {
103138
setOutTupleLeftFacts(outTuple, leftTuple);
104139
doUpdateOutTuple(outTuple);
@@ -126,52 +161,29 @@ protected final void innerUpdateRight(UniTuple<Right_> rightTuple, Consumer<Cons
126161
});
127162
} else {
128163
leftTupleConsumer.accept(leftTuple -> {
129-
IndexedSet<OutTuple_> outTupleSetLeft = leftTuple.getStore(inputStoreIndexLeftOutTupleSet);
130-
processOutTupleUpdate(leftTuple, rightTuple, outTupleSetLeft, outTupleSetRight, outputStoreIndexLeftOutSet);
164+
if (!leftTuple.state.isActive()) {
165+
// Assume the following scenario:
166+
// - The join is of two entities of the same type, both filtering out unassigned.
167+
// - One entity became unassigned, so the outTuple is getting retracted.
168+
// - The other entity is still assigned and is being updated.
169+
//
170+
// This means the filter would be called with (unassignedEntity, assignedEntity),
171+
// which breaks the expectation that the filter is only called on two assigned entities
172+
// and requires adding null checks to the filter for something that should intuitively be impossible.
173+
// We avoid this situation as it is clear that the outTuple must be retracted anyway,
174+
// and therefore any further updates to it are pointless.
175+
//
176+
// It is possible that the same problem would exist coming from the other side as well,
177+
// and therefore the right tuple would have to be checked for active state as well.
178+
// However, no such issue could have been reproduced; when in doubt, leave it out.
179+
return;
180+
}
181+
findAndProcessOutTuple(leftTuple, rightTuple, outTupleSetRight,
182+
leftTuple.getStore(inputStoreIndexLeftOutTupleSet), outputStoreIndexLeftOutSet);
131183
});
132184
}
133185
}
134186

135-
private void processOutTupleUpdate(LeftTuple_ leftTuple, UniTuple<Right_> rightTuple,
136-
IndexedSet<OutTuple_> referenceOutTupleSet, IndexedSet<OutTuple_> outTupleSet,
137-
int outputStoreIndexOutSet) {
138-
if (!leftTuple.state.isActive()) {
139-
// Assume the following scenario:
140-
// - The join is of two entities of the same type, both filtering out unassigned.
141-
// - One entity became unassigned, so the outTuple is getting retracted.
142-
// - The other entity is still assigned and is being updated.
143-
//
144-
// This means the filter would be called with (unassignedEntity, assignedEntity),
145-
// which breaks the expectation that the filter is only called on two assigned entities
146-
// and requires adding null checks to the filter for something that should intuitively be impossible.
147-
// We avoid this situation as it is clear that the outTuple must be retracted anyway,
148-
// and therefore any further updates to it are pointless.
149-
//
150-
// It is possible that the same problem would exist coming from the other side as well,
151-
// and therefore the right tuple would have to be checked for active state as well.
152-
// However, no such issue could have been reproduced; when in doubt, leave it out.
153-
return;
154-
}
155-
var outTuple = findOutTuple(outTupleSet, referenceOutTupleSet, outputStoreIndexOutSet);
156-
if (testFiltering(leftTuple, rightTuple)) {
157-
if (outTuple == null) {
158-
insertOutTuple(leftTuple, rightTuple);
159-
} else {
160-
updateOutTupleLeft(outTuple, leftTuple);
161-
}
162-
} else {
163-
if (outTuple != null) {
164-
retractOutTuple(outTuple);
165-
}
166-
}
167-
}
168-
169-
private static <Tuple_ extends AbstractTuple> @Nullable Tuple_ findOutTuple(IndexedSet<Tuple_> outTupleSet,
170-
IndexedSet<Tuple_> referenceOutTupleSet, int outputStoreIndexOutSet) {
171-
// Hack: the outTuple has no left/right input tuple reference, use the left/right outSet reference instead.
172-
return outTupleSet.findFirst(outTuple -> referenceOutTupleSet == outTuple.getStore(outputStoreIndexOutSet));
173-
}
174-
175187
protected final void retractOutTuple(OutTuple_ outTuple) {
176188
IndexedSet<OutTuple_> outSetLeft = outTuple.removeStore(outputStoreIndexLeftOutSet);
177189
outSetLeft.remove(outTuple);

0 commit comments

Comments
 (0)