Description
Preconditions and environment
- Tested in Magento 2.4.4-p2, but I have seen nothing to suggest the issue has still been fixed (I can see the issue when looking at the master branch code on GitHub)
- Performance monitored through a mixture of NewRelic and https://github.com/Smile-SA/magento2-module-debug-toolbar.
Steps to reproduce
- Install https://github.com/Smile-SA/magento2-module-debug-toolbar to allow the issue to be observed. Alternatively the issue can be observed with use of a step debugger, or by adding some debug logging within
Magento\Framework\App\FrontController->dispatchPreDispatchEvents(). - Go to a page which has a "forward" action. In other words, any page where an admin-defined URL is mapped to a route. For example, a product, category or CMS page.
Expected result
The controller_action_predispatch event should be fired once.
(plus one each of the dynamic events controller_action_predispatch_* events)
Actual result
The controller_action_predispatch event is fired twice - once for the Forward action, and one for the resulting page (i.e. the product, category etc). You can view this in the observers tab of the debug toolbar extension (if you are using it)
For our site the controller_action_predispatch event is one of the most expensive events, and so the fact it is called twice is having a noticeable performance impact. For example, it means expensive observers like Magento\Persistent\Observer\SynchronizePersistentInfoObserver are unnecessarily running twice.
Additional information
We can't see any reason why you would want controller_action_predispatch to run more than once, especially for a forward action.
A quick-and-dirty solution would be to replace this line:
$this->dispatchPreDispatchEvents($actionInstance, $request);
with:
if (!is_a($actionInstance, '\Magento\Framework\App\Action\Forward')) {
$this->dispatchPreDispatchEvents($actionInstance, $request);
}
I have tested this on my installation, and cannot see any negative side-effects.
A potentially better solution would be to allow actions to mark themselves as not requiring dispatchPreDispatchEvents to be sent.
A final solution would be to add handling within the observers themselves, to ensure each observer is only ran once per request. I don't like this approach, as it would put an expectation on extension authors to update their extensions in the same way.
Release note
No response
Triage and priority
- Severity: S0 - Affects critical data or functionality and leaves users without workaround.
- Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
- Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
- Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
- Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.