Problem/Motivation

While debugging an Open Social installation with about 100-200 modules I was surprised to find the install required 124.297 database queries to install. I was even more surprised when I found that ~85.000 of those came from the admin_toolbar_tools module. (Re-installing with admin_toolbar instead of admin_toolbar_tools left only 38.417 database queries)

Profiling showed admin_toolbar_tools_entity_insert, admin_toolbar_tools_entity_update, and admin_toolbar_tools_entity_delete were called a total of about 1100 times in total (901, 177 and 8 respectively). This caused the menu tree to be rebuild a lot of times.

Steps to reproduce

Proposed resolution

Ensure that none of the mentioned hooks have any effect during installation, but only rebuild once post-installation.

Remaining tasks

User interface changes

API changes

Data model changes

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

Kingdutch created an issue. See original summary.

rob230’s picture

We have the same problem when any module is enabled on the site - our deployments failed due to running out of memory. Disabling the admin_toolbar_tools module resolves the problem.

Our profiling found that having the admin_toolbar_tools module enabled means when a module is enabled, an extra 1.3 GB of memory is used and an extra 21 seconds of load time. Most of the memory is used by admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks::getDerivativeDefinitions(), called by admin_toolbar_tools_entity_insert() hook which causes all the menu to be rebuilt.

jonmcl’s picture

I am also noticing a memory issue introduced by admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks::getDerivativeDefinitions()

I end up with over 400 items in $links. If I hack the code to exclude some entity types, and get the total count of $links down to around 300, all seems to work well. It doesn't appear to matter which entity types I exclude.

Strangely, we are most likely to trigger the issue one some node bundles' Manage Field page, but the issue doesn't trigger on other entity types' bundles.

Sorry, this is probably not very useful information.

Disabling the module, or returning an empty array from ::getDerivativeDefinitions() does seem to not cause the issue to happen.

peterjlord’s picture

Thanks for the great module. This is a goto module for me as it increases usability.
I'm to having performance issues. I think it's since upgrading to 10.3

I've had to remove the module to restore my sites performance :(

daniel.pernold’s picture

StatusFileSize
new720.16 KB
new717.07 KB

My profiling has shown that it is mainly the access checks on the menu items that cause the problem in our projects. If we reduce the depth of the toolbar from 4 to 3, the performance increases significantly.

/admin/config/user-interface/admin-toolbar -> set 3 here

The time is spent here: https://git.drupalcode.org/project/admin_toolbar/-/blob/3.x/src/Render/E...

The same applies, of course, to the analogous implementation in the Gin Toolbar (and any other):
https://git.drupalcode.org/project/gin_toolbar/-/blob/8.x-1.x/src/Render...

Generally, if you uninstall the module, there is a huge performance jump.

Module INSTALLED:
ON

Module UNINSTALLED:
OFF

dydave’s picture

Version: 3.4.1 » 3.x-dev
Category: Bug report » Task
Priority: Major » Normal

Really great work here, with the profiling and all the comments, it's greatly appreciated!
Thanks a lot!

Could you please try testing this again on the latest version of the module and a recent core stable supported site?

We would like to understand what could be concretely done with the code to improve the issue?
Perhaps considering some sort of refactoring, caching strategy, changing the default depth value (in the installation profile), etc... ?

Would anybody have any advice or suggestions?

Thanks in advance!

mglaman’s picture

I know Drupal core doesn't have a "needs rebuild" flag for the menu links like it does the router. But this module could implement its own version of that. So on kernel terminate it rebuilds the menu (ideally once per request then) not multiple times per request

ressa’s picture

Perhaps a caching strategy could be worth considering, since menu items rarely change, mostly after a new module is installed, or a new content type is added. Perhaps the menu items could get cached, and only rebuild after events such as these?

  • Module install or uninstall
  • Content type creation or deletion
  • Paragraph creation or deletion
  • More?

Or maybe there are too many different scenarios to track ...

Maybe a checksum, checking if the number of menu items have changed, could be used, and only rebuild if it has changed?

I note this: "... Open Social installation with about 100-200 modules ...". Just out of curiosity, it could be interesting if those with slow performance could share the number of modules installed ...

Remember, as Ted Bowman once said :)

There's a module for that! Don't use it!

https://events.drupal.org/dublin2016/sessions/theres-module-dont-use-it....

prudloff’s picture

I feel like this issue is mixing two different performance problems:

  • Issue summary: this module slows down the installation process because it generates a lot of SQL queries.
  • Some comments: rendering the toolbar is slow (mostly because of access checks)

I think it should focus on the initial problem and a separate issue could be opened for the slow rendering.

prudloff’s picture

Some comments: rendering the toolbar is slow (mostly because of access checks)

I created a separate issue for this: #3547970: Rendering a toolbar with a lot of links is slow

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

svicer’s picture

Status: Active » Needs review

Profiling results and fix in MR !209

I profiled drush en on a YUSAOpenY site (100+ modules) using Xdebug. The cachegrind file was 7GB (846M lines). Key findings:

  • MenuLinkManager->rebuild702,325s inclusive (91% of total time)
  • php::usleep209,181s (lock waits from concurrent rebuilds)
  • admin_toolbar_tools ExtraLinks->routeExists635,189s (triggers route rebuild on every call)

Root causes identified

1. Entity hooks (hook_entity_insert/update/delete) call \Drupal::service('plugin.manager.menu.link')->rebuild() synchronously for every entity change (menus, roles, views, bundle types). During module installation, hundreds of config entities are created in sequence — each triggers a full menu link rebuild.

2. ExtraLinks::routeExists() calls routeProvider->getRoutesByNames() individually for each route name (dozens of times). Each call goes through RouteProviderLazyBuilder::getRouteProvider() which triggers routeBuilder->rebuild() on every invocation outside of site install context.

Fix (MR !209) — two changes

1. Deferred rebuild via DestructableInterface: New MenuLinkRebuildManager service collects rebuild requests from entity hooks and executes a single MenuLinkManager->rebuild() at request termination. Uses the needs_destruction service tag — same pattern as core's RouteBuilder. Works in both HTTP and CLI (drush).

2. Batch route preloading in ExtraLinks::routeExists(): Instead of querying routes one-by-one (each triggering a route rebuild), preloads all routes once via getAllRoutes() and caches the result in a property.

Results after fix

Command Before After
drush en dblog hung indefinitely 7.5s
drush en (24 modules) hung indefinitely 63s
dydave’s picture

Thanks a lot Vladyslav (@svicer)! 🙏
This looks super impressive! 🤩

Thank you so much for everything: The diagnostic, the analysis, the MR with suggested code changes, the results after patch, etc...
Plus the MR with tests all seem to still pass, so congratulations!! 🥳

I'll make sure to look into this in priority and see if this could get merged before dropping support for D9 versions.

More feedback coming soon.
Thanks again! 😊

dydave’s picture

Raaaa ... After doing some testing of this patch, it looks like it introduces a genuine behavioral regression versus the original code: previously rebuild() ....

Basically :

  • Create a new role, for exemple roleTest1, submit the form.
  • Check whether the link appears in the admin menu ==> KO🔴: The link does not appear immediately in the menu and requires an additional page refresh.

Note this regression appeared even without the additional changes I made to the MR.

We need to do a bit more thinking on this before being able to merge these changes....

dydave’s picture

OK: I'm now confident this solution is solid across all three contexts:

  • interactive UI,
  • bulk/config-sync,
  • CLI

and ready for review once again:

Vladyslav (@svicer): Any chance you could help us test again this updated merge request?
If possible with the same protocol you used to measure the performance improvements?

Otherwise any reviews or testing feedback in general, on the merge request, would be greatly appreciated.

Thanks in advance!