Skip to content

perf: more assorted improvements #1986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
triceo merged 4 commits into from
Dec 17, 2025
Merged

perf: more assorted improvements #1986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
baf197a
perf: increase performance of flattenLast()
triceo Dec 17, 2025
9359b13
perf: improve equality checks on composite keys
triceo Dec 17, 2025
72685db
perf: refactor comparison indexer for less indirection
triceo Dec 17, 2025
ac628e2
Review
triceo Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 74 additions & 45 deletions core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractFlattenLastNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -15,7 +14,6 @@
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleLifecycle;
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleState;
import ai.timefold.solver.core.impl.util.CollectionUtils;
import ai.timefold.solver.core.impl.util.MutableInt;

public abstract class AbstractFlattenLastNode<InTuple_ extends AbstractTuple, OutTuple_ extends AbstractTuple, EffectiveItem_, FlattenedItem_>
extends AbstractNode
Expand All @@ -36,27 +34,28 @@ protected AbstractFlattenLastNode(int flattenLastStoreIndex,
@Override
public final void insert(InTuple_ tuple) {
if (tuple.getStore(flattenLastStoreIndex) != null) {
throw new IllegalStateException("Impossible state: the input for the tuple (" + tuple
+ ") was already added in the tupleStore.");
throw new IllegalStateException(
"Impossible state: the input for the tuple (%s) was already added in the tupleStore."
.formatted(tuple));
}
Iterable<FlattenedItem_> iterable = mappingFunction.apply(getEffectiveFactIn(tuple));
var iterable = mappingFunction.apply(getEffectiveFactIn(tuple));
if (iterable instanceof Collection<FlattenedItem_> collection) {
// Optimization for Collection, where we know the size.
int size = collection.size();
var size = collection.size();
if (size == 0) {
return;
}
FlattenBagByItem<FlattenedItem_, OutTuple_> bagByItem = new FlattenBagByItem<>(size);
for (FlattenedItem_ item : collection) {
var bagByItem = new FlattenBagByItem<FlattenedItem_, OutTuple_>(size);
for (var item : collection) {
addTuple(tuple, item, bagByItem);
}
tuple.setStore(flattenLastStoreIndex, bagByItem);
} else {
Iterator<FlattenedItem_> iterator = iterable.iterator();
var iterator = iterable.iterator();
if (!iterator.hasNext()) {
return;
}
FlattenBagByItem<FlattenedItem_, OutTuple_> bagByItem = new FlattenBagByItem<>();
var bagByItem = new FlattenBagByItem<FlattenedItem_, OutTuple_>();
while (iterator.hasNext()) {
addTuple(tuple, iterator.next(), bagByItem);
}
Expand All @@ -82,23 +81,13 @@ public final void update(InTuple_ tuple) {
insert(tuple);
return;
}
Iterable<FlattenedItem_> iterable = mappingFunction.apply(getEffectiveFactIn(tuple));
for (var bag : bagByItem.getAllBags()) {
bag.reset();
}

for (var item : iterable) {
bagByItem.resetAll();
for (var item : mappingFunction.apply(getEffectiveFactIn(tuple))) {
addTuple(tuple, item, bagByItem);
}

var bagIterator = bagByItem.getAllBags().iterator();
while (bagIterator.hasNext()) {
var bag = bagIterator.next();
bag.removeExtras(this::removeTuple);
if (bag.newCount.intValue() == 0) {
bagIterator.remove();
}
}
bagByItem.getAllBags()
.removeIf(bag -> bag.removeExtras(this::removeTuple));
}

protected abstract EffectiveItem_ getEffectiveFactIn(InTuple_ tuple);
Expand All @@ -110,17 +99,14 @@ public final void retract(InTuple_ tuple) {
// No fail fast if null because we don't track which tuples made it through the filter predicate(s)
return;
}
for (var flattenLastEntry : bagByItem.getAllBags()) {
flattenLastEntry.reset();
flattenLastEntry.removeExtras(this::removeTuple);
}
bagByItem.applyToAll(this::removeTuple);
}

private void removeTuple(OutTuple_ outTuple) {
TupleState state = outTuple.state;
var state = outTuple.state;
if (!state.isActive()) {
throw new IllegalStateException("Impossible state: The tuple (" + outTuple +
") is in an unexpected state (" + outTuple.state + ").");
throw new IllegalStateException("Impossible state: The tuple (%s) is in an unexpected state (%s)."
.formatted(outTuple, state));
}
propagationQueue.retract(outTuple, state == TupleState.CREATING ? TupleState.ABORTING : TupleState.DYING);
}
Expand All @@ -132,6 +118,7 @@ public Propagator getPropagator() {

private record FlattenBagByItem<FlattenedItem_, OutTuple_>(
Map<FlattenedItem_, FlattenItemBag<FlattenedItem_, OutTuple_>> delegate) {

FlattenBagByItem() {
this(new LinkedHashMap<>());
}
Expand All @@ -148,6 +135,14 @@ Collection<FlattenItemBag<FlattenedItem_, OutTuple_>> getAllBags() {
return delegate.values();
}

void applyToAll(Consumer<OutTuple_> retractConsumer) {
delegate.forEach((key, value) -> value.clear(retractConsumer));
}

void resetAll() {
delegate.forEach((key, value) -> value.reset());
}

/**
* @param key the item to get the bag of
* @return the {@link FlattenItemBag} containing {@code key}, creating a new
Expand All @@ -158,10 +153,14 @@ FlattenItemBag<FlattenedItem_, OutTuple_> getBag(FlattenedItem_ key) {
}
}

private record FlattenItemBag<FlattenedItem_, OutTuple_>(FlattenedItem_ value, MutableInt newCount,
List<OutTuple_> outTupleList) {
private static final class FlattenItemBag<FlattenedItem_, OutTuple_> {

private final FlattenedItem_ value;
private final List<OutTuple_> outTupleList = new ArrayList<>();
private int newCount = 0;

FlattenItemBag(FlattenedItem_ value) {
this(value, new MutableInt(), new ArrayList<>());
this.value = value;
}

/**
Expand All @@ -172,12 +171,9 @@ private record FlattenItemBag<FlattenedItem_, OutTuple_>(FlattenedItem_ value, M
* Otherwise, the {@code insertConsumer} is called with a new tuple created
* with {@code outTupleSupplier}, and that tuple is added to {@link #outTupleList}.
*/
void add(Supplier<OutTuple_> outTupleSupplier,
Consumer<OutTuple_> insertConsumer,
Consumer<OutTuple_> updateConsumer) {
var listIndex = newCount.intValue();
newCount.increment();
if (newCount.intValue() > outTupleList.size()) {
void add(Supplier<OutTuple_> outTupleSupplier, Consumer<OutTuple_> insertConsumer, Consumer<OutTuple_> updateConsumer) {
var listIndex = newCount++;
if (newCount > outTupleList.size()) {
var inserted = outTupleSupplier.get();
outTupleList.add(inserted);
insertConsumer.accept(inserted);
Expand All @@ -190,20 +186,53 @@ void add(Supplier<OutTuple_> outTupleSupplier,
* Calls {@code retractConsumer} on the tuples in {@link #outTupleList}
* that are position at or after {@link #newCount}, and remove them
* from the list (causing the size of the list to be {@link #newCount}).
*
* @return true if after removal, {@link #newCount} is 0
*/
void removeExtras(Consumer<OutTuple_> retractConsumer) {
for (var i = newCount.intValue(); i < outTupleList.size(); i++) {
retractConsumer.accept(outTupleList.get(i));
boolean removeExtras(Consumer<OutTuple_> retractConsumer) {
var size = outTupleList.size();
for (var i = size - 1; i >= newCount; i--) {
// We go backwards to only shift the minimal amount of elements.
// Also, it makes the loop simpler, because elements that need to be removed do not change position.
retractConsumer.accept(outTupleList.remove(i));
}
outTupleList.subList(newCount.intValue(), outTupleList.size()).clear();
return newCount == 0;
}

/**
* Sets {@link #newCount} to 0, while retaining the created tuples in {@link #outTupleList}.
*/
void reset() {
newCount.setValue(0);
newCount = 0;
}

void clear(Consumer<OutTuple_> retractConsumer) {
outTupleList.forEach(retractConsumer);
outTupleList.clear();
newCount = 0;
}

@Override
public boolean equals(Object obj) {
return obj instanceof FlattenItemBag<?, ?> other &&
this.newCount == other.newCount &&
Objects.equals(this.value, other.value) &&
Objects.equals(this.outTupleList, other.outTupleList);
}

@Override
public int hashCode() {
return Objects.hash(newCount, value, outTupleList);
}

@Override
public String toString() {
return "FlattenItemBag[" +
"value=" + value + ", " +
"newCount=" + newCount + ", " +
"outTupleList=" + outTupleList + ']';
}

}

}
20 changes: 20 additions & 0 deletions core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/BiCompositeKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package ai.timefold.solver.core.impl.bavet.common.index;

import java.util.Objects;

record BiCompositeKey<A, B>(A propertyA, B propertyB) implements CompositeKey {

@SuppressWarnings("unchecked")
Expand All @@ -13,4 +15,22 @@ public <Key_> Key_ get(int id) {
};
}

@Override
public boolean equals(Object o) { // Often used in hash-based collections; the JDK-generated default is too slow.
if (this == o) {
return true;
}
return o instanceof BiCompositeKey<?, ?> that &&
Objects.equals(propertyA, that.propertyA) &&
Objects.equals(propertyB, that.propertyB);
}

@Override
public int hashCode() {
var hash = 7;
hash = 31 * hash + Objects.hashCode(propertyA);
hash = 31 * hash + Objects.hashCode(propertyB);
return hash;
}

}
Loading
Loading