Problem/Motivation

This is a follow-up issue to #1578832: [already commited but followup for CR?] <front> menu links are missing active trail classes.

Fixing the other issue introduced a performance penalty for the front page because an extra query had to be run in the menu system to check for link items matching <front>. This could be avoided if we had the ability to load menu links for multiple route names instead of just one at a time. Per @larowlan:

I don't think we can avoid the extra queries because we only have \Drupal\Core\Menu\MenuLinkManager::loadLinksByRoute and \Drupal\Core\Menu\MenuTreeStorageInterface::loadByRoute but I think it would be worth adding loadLinksByRoutes and loadByRoutes methods to both those services to support doing this in a single query.

@catch agreed that this task could be done in a follow-up issue.

Proposed resolution

  • Add \Drupal\Core\Menu\MenuLinkManager::loadLinksByRoutes.
  • Add \Drupal\Core\Menu\MenuTreeStorageInterface::loadByRoutes.
  • Update \Drupal\Core\Menu\MenuActiveTrail::getActiveLink() to use loadLinksByRoutes() for better performance.

Remaining tasks

User interface changes

Introduced terminology

API changes

Two new functions, one in MenuLinkManager and another in MenuTreeStorageInterface.

Data model changes

Release notes snippet

Issue fork drupal-3523497

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

dcam created an issue. See original summary.

larowlan’s picture

Issue tags: +Performance

,

dcam’s picture

Status: Active » Needs review
dcam’s picture

Status: Needs review » Active

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

vasike made their first commit to this issue’s fork.

vasike’s picture

Status: Active » Needs review

I confirm this issue.
Another example of using this improvement it's for contrib modules as Menu Breadcrumb.

MR updates
- I merged the main, instead of rebase as there were too many commits
- Lost some changes in the solving conflicts
- Small PHPStan fix
- Update the tests with previous updates + new values for query counts.
- The tests failure not related with this.

Needs review , for now ... till we'll get the right status from community & maintainers.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record

Very very small comments. But new API will need a CR. This may need submaintainer sign off who is pwolanin

vasike’s picture

Status: Needs work » Needs review

MR review addressed - all Green.

Also created an change record ... maybe also a reiew/update there would help.