Sylius version affected: 1.2.5
Description
Using the State Machine, there are three steps (add address, add shipping add payment) which are creating lots of database queries to sylius_zone_member and sylius_zone and has a huge performance impact. I would like to keep my zones in order to create some logic for shipping.
Steps to reproduce
We can use Sylius Front to reproduce the bug. After sending the form from the next URLs, you can see the perf (i have 325 zones but only 31 countries):
/checkout/address => 1602 queries (sylius_zone_member 325, sylius_zone > 1000)
/checkout/select-shipping => 2825 queries (sylius_zone_member 325, sylius_zone > 1000)
/checkout/select-payment => 1598 queries (sylius_zone_member 325, sylius_zone > 1000)
Possible Solution
and if you throw in some promotions... things will turn even worse!
Discussed this before once with @pamil I think (Slack) as I had something similar with promotions, loading them multiple times for the channel in a single pageview. I solved it by decorating that specific service and storing it internally instead of doing the database query over and over, but that's raising potential issues with PHP-PM users. My idea was to introduce a PerformancePlugin or something that does this on several occasions, but yeah, never got to it due to time constraints.
@stefandoorn storing them internally should be fine with PHP-PM if you reset them at the end of the request with kernel.reset. I'm wondering though if this is a sign of a deeper architectural issue (maybe it should join them in the query instead of lazy loading the zones)?
@gabiudrescu you're totally right with thousands of promotions, the cart is really slow. The current promotion providers get all promotions even if expired and not coupon based when you need a coupon. It should uses sql filters from start.
We should also add an active/enabled boolean.
We changed the sylius promotion provider to use this promotion repository method :
public function findActiveByChannelAndCoupon(Channel $channel, ?string $couponCode)
{
$qb = $this->createQueryBuilder('p');
$qb
->where(':channel MEMBER OF p.channels')
->andWhere('p.enabled = true')
->setParameter('channel', $channel)
;
if ($couponCode) {
$qb
->leftJoin('p.coupons', 'c')
->andWhere($qb->expr()->orX(
'p.couponBased = false',
'p.couponBased = true AND c.code = :coupon'
))
->setParameter('coupon', $couponCode)
;
} else {
$qb
->andWhere('p.couponBased = false');
}
return $qb
->addOrderBy('p.priority', 'ASC')
->getQuery()
->getResult()
;
}
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.
This issue was not tackled and it is important. cc @CoderMaggie
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.
nope, shouldn't close this.
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.
Most helpful comment
@stefandoorn storing them internally should be fine with PHP-PM if you reset them at the end of the request with
kernel.reset. I'm wondering though if this is a sign of a deeper architectural issue (maybe it should join them in the query instead of lazy loading the zones)?