Closed jdreesen closed 5 months ago
I have just improved the while
loop of the check
method.
The while
loop was inside the for
loop of the second level, while it only depends on the for
loop of the first level. This means that it was executed over and over again for each product configured in the condition, even though it was always the same.
This is from the production system of one of our clients who has many price rules with at least tens of products in the condition:
Nice, thanks a lot!
We have a case where our client has created multiple pricing rules, each with tens of products in the
CatalogProduct
condition.This turns out to be very expensive, because Pimcore loads every product of every rule (which means
n+1 = hundreds
of DB queries or cache requests) just to compare their IDs and figure out if the condition matches and the rule should be applied.But we already have the IDs because they get serialized with the condition anyway! So we could lazy-load the products only when really necessary and just use the IDs in the
check()
method.A problem is, that the
$products
and$productIds
properties areprotected
because theparent
class needs to access them to prepare them for (de)serialization.Sadly, they're not marked as
@internal
either, so it would be a breaking change to just alter their behavior.This is why we need to implement a magic getter (this would be a good case for
property hooks
which will come in PHP 8.4 by the way!) for them until they can be marked as@internal
(orAbstractObjectListCondition
gets converted into atrait
and the properties could beprivate
) in the next major, and we can move the lazy initialization intogetProducts()
.