node edit page performance and scalability with thousands of menu links

Created on 17 May 2021, over 3 years ago
Updated 8 July 2024, 3 months ago

Problem/Motivation

Some of my clients either have already over 3000 nodes and 3000 menu links or plan to have this many and more.

Client relying heavily on the menu system.

  • using menu_breadcrumbs contrib module for breadcrumbs generated by menu links
  • using pathauto pattern built by menu link parents [node:menu-link:parents:join-path]/[node:menu-link:title]
  • using two content types, child/parent, parent has enabled menu link, landing page can be a child of another landing page but basic page cannot have children, basic page child gets a disabled menu link
  • custom code disables menu links for newly created 'child' items

about 400 parents (enabled links), 2100 children menu links (disabled menu links)

without patch, 29 seconds from an empty cache to load the node edit page

with patch, 3 seconds from an empty cache to load the node edit page.

Steps to reproduce

See problem/motivation description.

Proposed resolution

see patch

Remaining tasks

Come up with an acceptable way to improve performance and scalability, perhaps inspire yourselves by the attached patch.

User interface changes

Vastly improved performance

API changes

See patch, fairly straight forward to understand. Tests do not exist for disabled menu item usage with the node form. Existing tests pass against the proposed patch.

Data model changes

None

Release notes snippet

none (yet)

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
Menu systemΒ  β†’

Last updated about 21 hours ago

Created by

πŸ‡¨πŸ‡¦Canada joseph.olstad

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡Έ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 12 months ago
    30,438 pass
  • last update 12 months ago
    29,676 pass
  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    retriggering for 10.1.x, 11.x

  • πŸ‡¨πŸ‡¦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 9 months ago
    Build Successful
  • last update 9 months 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);
        }
        }
    
Production build 0.71.5 2024