refactor: introduce incremental constraint collectors for improved performance

[triceo]

This benchmark run proves there are no regressions from adding the incremental support to the GroupNode:
https://github.com/TimefoldAI/timefold-solver-benchmarks/actions/runs/25095715799/job/73533079940

This benchmark proves no regressions after the new collector implementations were integrated:
https://github.com/TimefoldAI/timefold-solver-benchmarks/actions/runs/25427319378

The most expensive collectors are not actually part of those benchmarks, so the biggest benefit of this PR does not show up there.

[triceo]

@Christopher-Chianelli Ready for review now.

I wasn't able to keep the uni changes separate from the bi/tri/quad, so this PR is large. But if you review the uni parts, you can just skim the rest; they are mechanical copies with updates for the higher arities.

[triceo]

It would be great if we had update() variants for consecutive sequences and connected intervals; those collectors are expensive - if we can save some work there, it will immediately show.

[Christopher-Chianelli]

@triceo GitHub still not allowing me to create any review comments, so I putting my review comments directly on the the PR.

UniConstraintCollector - Does a user need to implement both accumulator and incrementalAccumulator? I notice some of our builtin ConstraintCollectors (such as AndThenUniCollector) implement both?

[Christopher-Chianelli]

• Per the Javadoc of ...AccumulatedValue:

/**
 * <li>{@link #add(Object)} will be called externally exactly once, when the value enters the group.
 * An instance of {@link UniConstraintCollectorAccumulatedValue} will only be created if there is a value to add.</li>
 * /

• But in ConditionalUniCollector:

        private final UniConstraintCollectorAccumulatedValue<A> innerValue;
        private boolean active = false;

        AccumulatedValue(ResultContainer_ container) {
            this.innerValue = innerIncremental.intoGroup(container);
        }

        @Override
        public void add(A a) {
            if (!predicate.test(a)) {
                return;
            }
            active = true;
            innerValue.add(a);
        }

        @Override
        public void update(A a) {
            boolean nowActive = predicate.test(a);
            if (active && nowActive) {
                innerValue.update(a);
            } else if (active) {
                active = false;
                innerValue.remove();
            } else if (nowActive) {
                active = true;
                innerValue.add(a);
            }
        }

i.e. add and remove can both be called multiple times.

[triceo]

i.e. add and remove can both be called multiple times.

They can, but they will not be. That is a contract that we guarantee.
(The alternative was to create more garbage, essentially add() would create a new container - and that was negatively affecting benchmarks.)

[Christopher-Chianelli]

i.e. add and remove can both be called multiple times.

They can, but they will not be. That is a contract that we guarantee. (The alternative was to create more garbage, essentially add() would create a new container - and that was negatively affecting benchmarks.)

Something like this would cause add/remove to be called multiple times currently:

constraintFactory.forEach(Employee.class)
    .join(Shift.class, equal(id(), Shift::getEmployee)
    // Employee has an InverseCollectionShadowVariable to get shift count
    .groupBy((employee, shift) -> employee, conditionally((employee, shift) -> employee.getShiftCount() > 5, sum((employee, shift) -> shift.getDuration())))
    .filter((employee, hours) -> hours != null && hours > 24)
    .penalize(HardSoftScore.ONE_HARD, (employee, hours) -> hours - 24)
    .asConstraint("Overworked employee")

In particular, the conditionally can and will call the sum's add multiple times. It seems our actually contract is:

• add can only be called when the value is not "added" (and once called, the value is "added")
• update can only be called when the value is "added" (and does not affect the state of "added")
• remove can only be called when the value is "added" (and once called, the value is not "added")

i.e. this is explictly allowed:

var a = new Data();
value.add(a);
a.change();
value.update(a);
value.remove(a);
value.add(a);

[triceo]

Something like this would cause add/remove to be called multiple times currently:

Good point. For collectors which contain other collectors (and only for them), the contract as specified doesn't actually hold.

Two ways out of this:

• These nesting collectors would correctly recreate the inner instances on every remove+add, so that the contract still holds.
• Or we update the contract to what you outlined in your previous message.

Since we want to prevent unnecessary garbage from being collected, the second option is probably the way to go.

[triceo]

@Christopher-Chianelli This is the finalized code, with the incremental switches completely removed.

Please review if you find some time between Team Days and your PTO, as I'd like to merge this week. Do not work on this during Team Days, please - if you don't review before your PTO, I will handle this with Fred instead.

[sonarqubecloud]

Quality Gate failed

Failed conditions
20.4% Duplication on New Code (required ≤ 5%)

See analysis details on SonarQube Cloud