- πΊπΈUnited States smustgrave
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request β as a guide.
This will need a valid test case to show the issue. For the related issue π When adding a new menu link, restrict the available parents to the current menu Fixed just posted in slack to the #contribute channel to see about getting a product manager review.
- last update
about 1 year ago 30,438 pass - last update
about 1 year ago 29,676 pass - π¨π¦Canada joseph.olstad
For the test scenario I ran with XHProf it was about 400 landing page menu links (parents, enabled links), 2100 children (basic page/children that are not allowed to have children/ disabled menu links)
Performance prior to patching was the best with the administrator role
Performance prior to patching was the worst with lesser roles that did not have the administer menus /administer content permission. These roles incur vastly more hits to the db for queries relating to permissions to access the entity linked by the menu link and for queries to build the link entity it'self.Performance benchmarks were based on a cold cache node edit/node add page load.
Performance after patching for this scenario was improved by about 900%
a full XHProf performance profile was done for this, in our use case with 2500 (or so, maybe it was more) menu links it is over 50,000 db queries on menu link permissions and loading disabled menu data that have no bearing on anything in the node edit form/node add form. The vast majority of those in our test scenario were for disabled menu links that are actually unwanted. The end result without the patch is just noise for the node form and very poor performance (especially on a cold cache).
It's especially poor performance for non administrator roles.
The administrator role gets better performance because there's no query to check menu link access
however lesser roles do not have permissions for everything so the whole permissions stack runs for those. The menu link permissions checks are expensive and unneeded in this case.
There's a legitimate justification for this patch to go into a core release , it passes core automated testing btw.
Check the test results on the core patch, all passing (still 100% passing).
If there's no core coverage for this scenario it's probably because disabled menu links are not essential to the node forms functionality. Basically noise and wasted db queries.
I repeat, 900% performance improvement, and this number only rises as your site grows.
- π¨π¦Canada SKAUGHT
needs work:
a. patch is using \Drupal::routeMatch(). please use injection.
b. for our example use case, Indeed, from Drupal Core there are many use cases where various different editor roles would need to have access all menu items.. This process does not actually aid in 'building the list better'.in General, I might assume we are missing related issues already open/in-progress that we should look into as well.
- last update
about 1 year ago Build Successful - last update
about 1 year ago CI aborted Thanks for the patch @joseph.olstad
I also encountered this problem and solved the issue in this way, since for me the output of more than 10,000 menu items was not of great importance, I limited the depth to level 2 (setMaxDepth), thereby I achieved normal operation of editing and creating a page, and I edit the menu through the bigmenu module .Sample:
$route = \Drupal::routeMatch()->getRouteName(); if ($route == 'entity.node.edit_form' || $route == 'node.add' ) { // my big menu (10000 links) if ($menu_name='menu-menu-category' ) { $parameters->onlyEnabledLinks(); $parameters->setMaxDepth(2); } }
Thanks for the patch @joseph.olstad
I also encountered this problem and solved the issue in this way, since for me the output of more than 10,000 menu items was not of great importance, I limited the depth to level 2 (setMaxDepth), thereby I achieved normal operation of editing and creating a page, and I edit the menu through the bigmenu module .Sample:
$route = \Drupal::routeMatch()->getRouteName(); if ($route == 'entity.node.edit_form' || $route == 'node.add' ) { // my big menu (10000 links) if ($menu_name='menu-menu-category' ) { $parameters->onlyEnabledLinks(); $parameters->setMaxDepth(2); } }