-
-
Notifications
You must be signed in to change notification settings - Fork 946
Description
Bug report
Hello there,
While reading @staabm's blog posts about improving performance in PHPStan, I tried running PHPStan using this script in our project to see which files take the most time to analyze. There were ~10 files that took 1-2 secondes (which is okay, they are quite complex), lots that took less than 1 sec, and... one that took 17.30s alone.
I was surprised to find out that it was "only" an abstract class of 90 lines of code. I then ran the analysis 4 more times, and got the following results: 17.30, 13.67, 13.61, 16.09, 14.05. In comparison, even the most complicated file from our legacy folder with 2300 lines of crappy code takes "only" 11.69 seconds.
So "I think there's more to this class than meets the eye", as Gandalf would say... ;)
Here it is :
<?php
declare(strict_types=1);
namespace App\Infrastructure\Shared\Notifier\Notification;
use App\Domain\Shared\Event\Event;
use App\Infrastructure\Shared\Notifier\Recipient\EmailRecipient;
use App\Infrastructure\Shared\Notifier\Recipient\InternalMessageRecipient;
use App\Infrastructure\Shared\Notifier\Recipient\RecipientFactory;
use Carbon\Carbon;
use DateTime;
use Gammadia\Moment\Timezone;
use Symfony\Component\DependencyInjection\Attribute\Autoconfigure;
use Symfony\Component\Notifier\Notification\Notification as BaseNotification;
use Symfony\Component\Notifier\NotifierInterface;
use Symfony\Component\Notifier\Recipient\RecipientInterface;
use Symfony\Contracts\Service\ResetInterface;
#[Autoconfigure(tags: [Notification::class])]
abstract class Notification extends BaseNotification implements ResetInterface
{
protected ?Event $event = null;
public function __construct(
private NotifierInterface $notifier,
protected RecipientFactory $recipientFactory,
) {
parent::__construct();
$recipientFactory->setNotification($this);
}
public function __invoke(Event $event): void
{
$this->event = $event;
try {
foreach ($this->recipients() as $recipient) {
$this->notifier->send($this, $recipient);
}
} finally {
$this->reset();
}
}
/**
* @return string[]
*/
final public function getChannels(RecipientInterface $recipient): array
{
$channels = [];
if ($recipient instanceof EmailRecipient) {
$channels[] = 'email';
}
if ($recipient instanceof InternalMessageRecipient) {
$channels[] = 'internal-message';
}
return $channels;
}
public function reset(): void
{
$this->event = null;
}
/**
* Returns a list of recipients.
*
* Examples:
*
* yield new Recipient();
* yield from $this->recipientFactory->createFromPersons(...$ids);
*
* @see RecipientFactory
*
* @return RecipientInterface[]|iterable<RecipientInterface>
*/
abstract protected function recipients(): iterable;
protected function formatLocalizedDate(DateTime $dateTime, string $locale, string $format = 'LLLL'): string
{
/** @var Carbon $carbon */
$carbon = Carbon::instance($dateTime)->timezone(Timezone::getDefault())->locale($locale);
return $carbon->isoFormat($format);
}
}I hope sharing this may help you guys find some bottleneck in PHPStan (or a clue to fix some problematic typing in this class), so that we can shave off theses seconds from all our CI builds!
gnutix
PS: There are also two legacy files in our project that probably take longer than they should, but I can't share them publicly. One of them is a service class that takes ~5.41 sec for 273 lines of code, the other an old PHP file that processes some values to prepare a template's rendering that takes ~4.46 sec for 339 lines. Please let me know if you'd like to investigate those too, I could send them in private.
Code snippet that reproduces the problem
No response
Expected output
Faster analysis.
Did PHPStan help you today? Did it make you happy in any way?
I wanted to thank you for taking care of the planet by taking performance issues seriously. <3