Add Intervals::compactConstraint method to optimize constraints #100
Conversation
Seldaek
force-pushed
the
compact
branch
2 times, most recently
from
05fe888
to
b97485f
Compare
|
Need to add more tests for compacting still (help welcome..) and also tests for AnyDevConstraint's |
… over optimizing !=x to produce correct constraints
Seldaek
force-pushed
the
compact
branch
2 times, most recently
from
d200742
to
a52d7c8
Compare
|
Ok more tests added, it now verifies for all Constraint/MultiConstraint::matches tests that after compactConstraint things still match the same way. @naderman please take another look. |
…equality with matches on original constraints
|
Resulting multiconstraint flag for conjunctive/disjunctive now entirely depends on whether it uses != or == in the numeric part. So what about the constraint |
…= dev joined as a conjunctive sub-constraint, and skip useless numeric constraints when combined with != dev
|
Right, fixed a bunch more things in the branch handling area and many new tests. 👍 |
There are no intervals that equal zero or positive infinity, those are individual values
gets rid of isConjunctive flag in the compact function
Intervals: Use exclude/include name list Instead of constraints for branches
| // they are zero/+inf | ||
| if ($interval->getEnd()->getOperator() === '<' && $i+1 < $count) { | ||
| $nextInterval = $intervals['numeric'][$i+1]; | ||
| if ($interval->getEnd()->getVersion() === $nextInterval->getStart()->getVersion() && $nextInterval->getStart()->getOperator() === '>') { |
There was a problem hiding this comment.
$interval->getEnd() and getStart are called several times in this "else" branch. For better performances, we could store it in a variable
| // but this needs to happen as a conjunctive expression together with the start of the current interval | ||
| // and end of next interval, so [>=M, <N] || [>N, <P] => [>=M, !=N, <P] but M/P can be skipped if | ||
| // they are zero/+inf | ||
| if ($interval->getEnd()->getOperator() === '<' && $i+1 < $count) { |
There was a problem hiding this comment.
inverse the 2 test $i+1 < $count is costless than $interval->getEnd()->getOperator()
|
Yeah not sure if those are worth it.. feel free to explore later if you like once we determine whether this is used at all and where. I'd say not really worth obsessing over optimizing away a couple getter fn calls at this point. One day we'll have readonly props and this can all be public without getters 🙄 |
Could be handy for @Toflar's PR to avoid generating giant constraints when concatenating many MultiConstraints. Remains to be seen whether there is really enough CPU-time benefit though..