Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions benchmark/src/main/resources/benchmark.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@
<xs:element minOccurs="0" name="constraintStreamAutomaticNodeSharing" type="xs:boolean"/>


<xs:element minOccurs="0" name="constraintStreamProfilingMode" type="tns:constraintProfilingMode"/>


<xs:element minOccurs="0" name="incrementalScoreCalculatorClass" type="xs:string"/>


Expand Down Expand Up @@ -2819,6 +2822,27 @@
</xs:simpleType>


<xs:simpleType name="constraintProfilingMode">


<xs:restriction base="xs:string">


<xs:enumeration value="NONE"/>


<xs:enumeration value="BY_METHOD"/>


<xs:enumeration value="BY_LINE"/>


</xs:restriction>


</xs:simpleType>


<xs:simpleType name="terminationCompositionStyle">


Expand Down
11 changes: 11 additions & 0 deletions core/src/build/revapi-differences.json
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,17 @@
"new": "class ai.timefold.solver.core.config.heuristic.selector.value.ValueSelectorConfig",
"annotation": "@org.jspecify.annotations.NullMarked",
"justification": "Update config"
},
{
"ignore": true,
"code": "java.annotation.attributeValueChanged",
"old": "class ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig",
"new": "class ai.timefold.solver.core.config.score.director.ScoreDirectorFactoryConfig",
"annotationType": "jakarta.xml.bind.annotation.XmlType",
"attribute": "propOrder",
"oldValue": "{\"easyScoreCalculatorClass\", \"easyScoreCalculatorCustomProperties\", \"constraintProviderClass\", \"constraintProviderCustomProperties\", \"constraintStreamImplType\", \"incrementalScoreCalculatorClass\", \"incrementalScoreCalculatorCustomProperties\", \"scoreDrlList\", \"initializingScoreTrend\", \"assertionScoreDirectorFactory\"}",
"newValue": "{\"easyScoreCalculatorClass\", \"easyScoreCalculatorCustomProperties\", \"constraintProviderClass\", \"constraintProviderCustomProperties\", \"constraintStreamImplType\", \"constraintStreamAutomaticNodeSharing\", \"constraintStreamProfilingMode\", \"incrementalScoreCalculatorClass\", \"incrementalScoreCalculatorCustomProperties\", \"scoreDrlList\", \"initializingScoreTrend\", \"assertionScoreDirectorFactory\"}",
"justification": "Add constraint profiling to config"
}
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package ai.timefold.solver.core.config.score.director;

/**
* Controls how constraints are profiled.
* Enabling profiling have a minor performance impact.
*/
public enum ConstraintProfilingMode {
Copy link
Collaborator

@triceo triceo Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is needless complexity. Do not profile by method or line - profile by constraint. That is what people actually want to see; the list of constraints and how much time they take to run.

Also, the names of constraints typically do not change, while method names and lines do as code is refactored over time. People who use bytecode obfuscation would get absolutely no information out of this, and I wonder what happens with automatic node sharing etc., where we mutate code heavily without giving access to that new source code.

All in all, this information is too unreliable to present it to the user. The only reliable source is the constraint name; the rest, if it needs to exist, should remain hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look at lambdas, it looks at the .filter`.join`/etc. code, which is not modified by node sharing (i.e. it looks at the filter call, not what within the filter call).
Arguably, having method name + line number is useful; it tells you what part of a constraint is taking the most runtime (i.e. is it a filter, join, or groupBy).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line number still changes with automatic node sharing - new fields are added at the top of the class, moving others down. (Although how does it even work with bytecode generation, when there is no source code? It seems that this metadata has to be fundamentally unreliable.)

Even with the changes that the Java compiler does when it produces bytecode, line numbers are still unreliable. Code can be added here and there. The idea of trying to do this per building block is IMO noble, but falls apart too easily. I don't think it's necessary for this concept to succeed - when the users see the expensive constraint, that'll already be plenty information to proceed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line numbers are recorded in bytecode; so they line numbers in stack traces do not change if that bytecode is modified (provided you don't delete/change the metadata instruction that records the line number).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for how line numbers look in bytecode:

// Some previous instructions
IADD
INVOKE_VIRTUAL
Label LINE_START
LINENUMBER 10 LINE_START
// Some instructions after
NOP
INVOKE_INTERFACE
FADD
Label ANOTHER_LINE
LINENUMBER 11 ANOTHER_LINE

In this example, line 10 correspond to NOP, INVOKE_INTERFACE, FADD.

/**
* Disables profiling.
*/
NONE,

/**
* Profile by the method an operation was defined in.
*/
BY_METHOD,

/**
* Profile by the line an operation was defined on.
*/
BY_LINE
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"constraintProviderCustomProperties",
"constraintStreamImplType",
"constraintStreamAutomaticNodeSharing",
"constraintStreamProfilingMode",
"incrementalScoreCalculatorClass",
"incrementalScoreCalculatorCustomProperties",
"scoreDrlList",
Expand All @@ -46,6 +47,7 @@ public class ScoreDirectorFactoryConfig extends AbstractConfig<ScoreDirectorFact
protected Map<String, String> constraintProviderCustomProperties = null;
protected ConstraintStreamImplType constraintStreamImplType;
protected Boolean constraintStreamAutomaticNodeSharing;
protected ConstraintProfilingMode constraintStreamProfilingMode;

protected Class<? extends IncrementalScoreCalculator> incrementalScoreCalculatorClass = null;

Expand Down Expand Up @@ -126,6 +128,14 @@ public void setConstraintStreamAutomaticNodeSharing(@Nullable Boolean constraint
this.constraintStreamAutomaticNodeSharing = constraintStreamAutomaticNodeSharing;
}

public ConstraintProfilingMode getConstraintStreamProfilingMode() {
return constraintStreamProfilingMode;
}

public void setConstraintStreamProfilingMode(ConstraintProfilingMode constraintStreamProfilingMode) {
this.constraintStreamProfilingMode = constraintStreamProfilingMode;
}

public @Nullable Class<? extends IncrementalScoreCalculator> getIncrementalScoreCalculatorClass() {
return incrementalScoreCalculatorClass;
}
Expand Down Expand Up @@ -227,6 +237,12 @@ public void setAssertionScoreDirectorFactory(@Nullable ScoreDirectorFactoryConfi
return this;
}

public @NonNull ScoreDirectorFactoryConfig
withConstraintStreamProfiling(@NonNull ConstraintProfilingMode constraintStreamProfiling) {
this.constraintStreamProfilingMode = constraintStreamProfiling;
return this;
}

public @NonNull ScoreDirectorFactoryConfig
withIncrementalScoreCalculatorClass(
@NonNull Class<? extends IncrementalScoreCalculator> incrementalScoreCalculatorClass) {
Expand Down Expand Up @@ -288,6 +304,8 @@ public ScoreDirectorFactoryConfig withScoreDrls(String... scoreDrls) {
constraintStreamImplType, inheritedConfig.getConstraintStreamImplType());
constraintStreamAutomaticNodeSharing = ConfigUtils.inheritOverwritableProperty(constraintStreamAutomaticNodeSharing,
inheritedConfig.getConstraintStreamAutomaticNodeSharing());
constraintStreamProfilingMode = ConfigUtils.inheritOverwritableProperty(constraintStreamProfilingMode,
inheritedConfig.getConstraintStreamProfilingMode());
incrementalScoreCalculatorClass = ConfigUtils.inheritOverwritableProperty(
incrementalScoreCalculatorClass, inheritedConfig.getIncrementalScoreCalculatorClass());
incrementalScoreCalculatorCustomProperties = ConfigUtils.inheritMergeableMapProperty(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,8 @@ public void settle() {
nodeNetwork.settle();
}

public final void summarizeProfileIfPresent() {
nodeNetwork.summarizeProfileIfPresent();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@

import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
import ai.timefold.solver.core.impl.bavet.common.BavetRootNode;
import ai.timefold.solver.core.impl.bavet.common.ConstraintProfiler;
import ai.timefold.solver.core.impl.bavet.common.Propagator;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Represents Bavet's network of nodes, specific to a particular session.
* Nodes only used by disabled constraints have already been removed.
Expand All @@ -19,10 +23,11 @@
* @param layeredNodes nodes grouped first by their layer, then by their index within the layer;
* propagation needs to happen in this order.
*/
@NullMarked
public record NodeNetwork(Map<Class<?>, List<BavetRootNode<?>>> declaredClassToNodeMap,
Propagator[][] layeredNodes) {
Propagator[][] layeredNodes, @Nullable ConstraintProfiler constraintProfiler) {

public static final NodeNetwork EMPTY = new NodeNetwork(Map.of(), new Propagator[0][0]);
public static final NodeNetwork EMPTY = new NodeNetwork(Map.of(), new Propagator[0][0], null);

public int forEachNodeCount() {
return declaredClassToNodeMap.size();
Expand Down Expand Up @@ -70,6 +75,12 @@ private static void settleLayer(Propagator[] nodesInLayer) {
}
}

public void summarizeProfileIfPresent() {
if (constraintProfiler != null) {
constraintProfiler.summarize();
}
}

@Override
public boolean equals(Object o) {
if (this == o)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
package ai.timefold.solver.core.impl.bavet.common;

import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Set;

import ai.timefold.solver.core.impl.score.stream.bavet.BavetConstraintSession;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* @see PropagationQueue Description of the propagation mechanism.
*/
@NullMarked
public abstract class AbstractNode {

private long id;
private long layerIndex = -1;
private @Nullable Set<ConstraintNodeLocation> locationSet;

/**
* Instead of calling the propagation directly from here,
Expand All @@ -29,6 +38,20 @@ public final void setId(long id) {
this.id = id;
}

public Set<ConstraintNodeLocation> getLocationSet() {
if (locationSet == null) {
return Collections.emptySet();
}
return locationSet;
}

public void addLocationSet(Set<ConstraintNodeLocation> locationSet) {
if (this.locationSet == null) {
this.locationSet = new LinkedHashSet<>();
}
this.locationSet.addAll(locationSet);
}

public final void setLayerIndex(long layerIndex) {
if (layerIndex < 0) {
throw new IllegalArgumentException("Impossible state: layer index (" + layerIndex + ") must be at least 0.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,33 @@
import ai.timefold.solver.core.impl.bavet.common.tuple.RightTupleLifecycle;
import ai.timefold.solver.core.impl.bavet.common.tuple.TupleLifecycle;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

@NullMarked
public abstract class AbstractNodeBuildHelper<Stream_ extends BavetStream> {

private final Set<Stream_> activeStreamSet;
private final Map<AbstractNode, Stream_> nodeCreatorMap;
private final Map<Stream_, TupleLifecycle<? extends AbstractTuple>> tupleLifecycleMap;
private final Map<Stream_, Integer> storeIndexMap;

@Nullable
private final ConstraintProfiler constraintProfiler;

@Nullable
private List<AbstractNode> reversedNodeList;
private long nextLifecycleProfilingId = 0;

protected AbstractNodeBuildHelper(Set<Stream_> activeStreamSet) {
protected AbstractNodeBuildHelper(Set<Stream_> activeStreamSet,
@Nullable ConstraintProfiler constraintProfiler) {
this.activeStreamSet = activeStreamSet;
int activeStreamSetSize = activeStreamSet.size();
this.nodeCreatorMap = new HashMap<>(Math.max(16, activeStreamSetSize));
this.tupleLifecycleMap = new HashMap<>(Math.max(16, activeStreamSetSize));
this.storeIndexMap = new HashMap<>(Math.max(16, activeStreamSetSize / 2));
this.reversedNodeList = new ArrayList<>(activeStreamSetSize);
this.constraintProfiler = constraintProfiler;
}

public boolean isStreamActive(Stream_ stream) {
Expand All @@ -45,6 +56,7 @@ public void addNode(AbstractNode node, Stream_ creator) {

public void addNode(AbstractNode node, Stream_ creator, Stream_ parent) {
reversedNodeList.add(node);
node.addLocationSet(creator.getLocationSet());
nodeCreatorMap.put(node, creator);
if (!(node instanceof BavetRootNode<?>)) {
if (parent == null) {
Expand All @@ -57,13 +69,19 @@ public void addNode(AbstractNode node, Stream_ creator, Stream_ parent) {

public void addNode(AbstractNode node, Stream_ creator, Stream_ leftParent, Stream_ rightParent) {
reversedNodeList.add(node);
node.addLocationSet(creator.getLocationSet());
nodeCreatorMap.put(node, creator);
putInsertUpdateRetract(leftParent, TupleLifecycle.ofLeft((LeftTupleLifecycle<? extends AbstractTuple>) node));
putInsertUpdateRetract(rightParent, TupleLifecycle.ofRight((RightTupleLifecycle<? extends AbstractTuple>) node));
}

public <Tuple_ extends AbstractTuple> void putInsertUpdateRetract(Stream_ stream,
TupleLifecycle<Tuple_> tupleLifecycle) {
if (constraintProfiler != null) {
tupleLifecycle = TupleLifecycle.profiling(constraintProfiler, nextLifecycleProfilingId,
stream, tupleLifecycle);
nextLifecycleProfilingId++;
}
tupleLifecycleMap.put(stream, tupleLifecycle);
}

Expand Down Expand Up @@ -147,19 +165,22 @@ public AbstractNode findParentNode(Stream_ childNodeCreator) {
}

public static NodeNetwork buildNodeNetwork(List<AbstractNode> nodeList,
Map<Class<?>, List<BavetRootNode<?>>> declaredClassToNodeMap) {
Map<Class<?>, List<BavetRootNode<?>>> declaredClassToNodeMap,
AbstractNodeBuildHelper<?> nodeBuildHelper) {
var layerMap = new TreeMap<Long, List<Propagator>>();
for (var node : nodeList) {
layerMap.computeIfAbsent(node.getLayerIndex(), k -> new ArrayList<>())
.add(node.getPropagator());
var layer = node.getLayerIndex();
var propagator = node.getPropagator();
layerMap.computeIfAbsent(layer, k -> new ArrayList<>())
.add(propagator);
}
var layerCount = layerMap.size();
var layeredNodes = new Propagator[layerCount][];
for (var i = 0; i < layerCount; i++) {
var layer = layerMap.get((long) i);
layeredNodes[i] = layer.toArray(new Propagator[0]);
}
return new NodeNetwork(declaredClassToNodeMap, layeredNodes);
return new NodeNetwork(declaredClassToNodeMap, layeredNodes, nodeBuildHelper.constraintProfiler);
}

public <BuildHelper_ extends AbstractNodeBuildHelper<Stream_>> List<AbstractNode> buildNodeList(Set<Stream_> streamSet,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ai.timefold.solver.core.impl.bavet.common;

import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand All @@ -22,22 +23,51 @@ public abstract class BavetAbstractConstraintStream<Solution_>
extends AbstractConstraintStream<Solution_>
implements BavetStream {

private static final String BAVET_IMPL_PACKAGE = "ai.timefold.solver.core.impl.bavet";
private static final String BAVET_SCORE_IMPL_PACKAGE = "ai.timefold.solver.core.impl.score.stream";
private static final String BAVET_SCORE_API_PACKAGE = "ai.timefold.solver.core.api.score.stream";

protected final BavetConstraintFactory<Solution_> constraintFactory;
protected final BavetAbstractConstraintStream<Solution_> parent;
protected final List<BavetAbstractConstraintStream<Solution_>> childStreamList = new ArrayList<>(2);
protected final Set<ConstraintNodeLocation> streamLocationSet;

protected BavetAbstractConstraintStream(BavetConstraintFactory<Solution_> constraintFactory,
BavetAbstractConstraintStream<Solution_> parent) {
super(parent.getRetrievalSemantics());
this.constraintFactory = constraintFactory;
this.parent = parent;
this.streamLocationSet = new LinkedHashSet<>();
streamLocationSet.add(determineStreamLocation());
}

protected BavetAbstractConstraintStream(BavetConstraintFactory<Solution_> constraintFactory,
RetrievalSemantics retrievalSemantics) {
super(retrievalSemantics);
this.constraintFactory = constraintFactory;
this.parent = null;
this.streamLocationSet = new LinkedHashSet<>();
streamLocationSet.add(determineStreamLocation());
}

private static ConstraintNodeLocation determineStreamLocation() {
return StackWalker.getInstance().walk(stack -> stack
.dropWhile(stackFrame -> stackFrame.getClassName().startsWith(BAVET_IMPL_PACKAGE) ||
stackFrame.getClassName().startsWith(BAVET_SCORE_IMPL_PACKAGE) ||
stackFrame.getClassName().startsWith(BAVET_SCORE_API_PACKAGE))
.map(stackFrame -> new ConstraintNodeLocation(stackFrame.getClassName(),
stackFrame.getMethodName(),
stackFrame.getLineNumber()))
.findFirst()
.orElseGet(ConstraintNodeLocation::unknown));
}

public Set<ConstraintNodeLocation> getLocationSet() {
return streamLocationSet;
}

public void addLocationSet(Set<ConstraintNodeLocation> locationSet) {
streamLocationSet.addAll(locationSet);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package ai.timefold.solver.core.impl.bavet.common;

import java.util.Set;

public interface BavetStream {

<Stream_> Stream_ getParent();

default Set<ConstraintNodeLocation> getLocationSet() {
return Set.of(ConstraintNodeLocation.unknown());
}

}
Loading
Loading