CODEWITHSUNDEEP
optaplanner
Raphawel
Raphawel

Reputation: 19

Joiners with filtering performs very slowly

I have a constraint with some joiners but the performance are very poor. Is it a way to improve it ?

I need to have the count of WorkingDay ( with ::hasPermission ) within the previous four days of the current day analyzed.

Here is my current constraint :

       private Constraint fiveConsecutiveWorkingDaysMax(ConstraintFactory constraintFactory) {
    return constraintFactory
            .from(WorkingDay.class)
            .filter(WorkingDay::hasPermission)
            .join(WorkingDay.class,
                    Joiners.equal(WorkingDay::hasPermission),
                    Joiners.equal(WorkingDay::getAgent),
                    Joiners.filtering((wd1, wd2) -> { 
                        LocalDate fourDaysBefore = wd1.getDayJava().minusDays(4);
                        Boolean wd2IsBeforeWd1 = wd2.getDayJava().isBefore(wd1.getDayJava());
                        Boolean wd2IsAfterFourDaysBeforeWd1 = wd2.getDayJava().compareTo(fourDaysBefore) >= 0;
                        return (wd2IsBeforeWd1 && wd2IsAfterFourDaysBeforeWd1);
                    }))

            .groupBy((wd1, wd2) -> wd2, ConstraintCollectors.countBi())
            .filter((wd2, count) -> count >= 4)
            .penalizeConfigurable(FIVE_CONSECUTIVE_WORKING_DAYS_MAX); 
}

Thanx for your help

Upvotes: 1

Views: 204

Answers (2)

Lukáš Petrovický
Lukáš Petrovický

Reputation: 5702

There is potential for improvement here. First, we pre-filter the right hand side of the join to reduce the size of the cartesian product:

return constraintFactory
    .forEach(WorkingDay.class)
    .filter(WorkingDay::hasPermission)
    .join(constraintFactory.forEach(WorkingDay.class)
               .filter(WorkingDay::hasPermission),
            Joiners.equal(WorkingDay::getAgent),
            Joiners.filtering((wd1, wd2) -> { 
                LocalDate fourDaysBefore = wd1.getDayJava().minusDays(4);
                Boolean wd2IsBeforeWd1 = wd2.getDayJava().isBefore(wd1.getDayJava());
                Boolean wd2IsAfterFourDaysBeforeWd1 = wd2.getDayJava().compareTo(fourDaysBefore) >= 0;
                return (wd2IsBeforeWd1 && wd2IsAfterFourDaysBeforeWd1);
            }))
    ...

This has the added benefit of simplifying the index as it removes one equals joiner. Next, part of the filter can be replaced by a joiner as well:

return constraintFactory
    .forEach(WorkingDay.class)
    .filter(WorkingDay::hasPermission)
    .join(constraintFactory.forEach(WorkingDay.class)
               .filter(WorkingDay::hasPermission),
            Joiners.equal(WorkingDay::getAgent),
            Joiners.greaterThan(wd -> wd.getDayJava()),
            Joiners.filtering((wd1, wd2) -> { 
                LocalDate fourDaysBefore = wd1.getDayJava().minusDays(4);
                Boolean wd2IsAfterFourDaysBeforeWd1 = wd2.getDayJava().compareTo(fourDaysBefore) >= 0;
                return wd2IsAfterFourDaysBeforeWd1;
            }))
    ...

Finally, the method does needless boxing of boolean into Boolean, wasting CPU cycles and memory. This is a micro-optimization, but if the filter happens often enough, the benefit will be measurable.

A constraint refactored like this should perform better. That said, large joins are still going to take considerable time and the only way to work around that is to figure out a way to make them smaller.

Also, as Geoffrey said, I'd consider penalizing by the actual count, as what you have here is a textbook example of a score trap.

Upvotes: 1

Geoffrey De Smet
Geoffrey De Smet

Reputation: 27327

I don't see why this should be slow. Except maybe because the Cartesian Product explodes for a long time window. How many days is your time window?

Do note that the nurse rostering example has a totally different approach to detecting consecutive working days, using a custom collector. You might want to look at that in optaplanner-examples.

Upvotes: 0

Related Questions