Steps to reproduce, install toolbar.module, stick a debug_backtrace() in system_admin_menu_block(), watch the entirety of /admin being built on every page.

I think the proper fix for this is just to use a custom menu for the main toolbar links - this will be needed by the latest proposal at #508458: Config and modules page anyway.

#12 menu.png344.63 KBcatch
#6 remove_access.patch3.92 KBcatch
Failed: 11686 passes, 5 fails, 872 exceptions
[ View ]
#3 _menu_link_translate.png317.91 KBcatch



So there has to be around this, but in the meantime I found some bugs in system_admin_menu_block() which were causing duplicate rendering and queries: #521798: system_admin_menu_block() static caching is broken .

new317.91 KB

This is taking up approximately 30% of page execution time at the moment.

Title:toolbar_get_menu_tree() causes system_admin_menu_block() to be called on every pagesystem_admin_menu_block_access() is very, very, slow

admin_menu, with client-side caching being disabled, suffers from this, too.

Status:Active» Needs review
new3.92 KB
Failed: 11686 passes, 5 fails, 872 exceptions
[ View ]

We should just roll this back. You might get a link which doesn't go anywhere, but you'll get there 35% faster.

Status:Needs review» Needs work

The last submitted patch failed testing.

Status:Needs work» Needs review

This is now a straight rollback of #296693: Hide empty admin categories I may re-open that one at least temporarily since while the original bug is annoying, the fix is worse.

Status:Needs review» Active

uhm, no. Re-introducing MENU_DYNAMIC_ITEM would be an option, but reverting the code that ensures that admin categories are hidden if there are not any items below them does not do any good.

Title:system_admin_menu_block_access() is very, very, slowsystem_admin_menu_block_access() makes no sense
Component:toolbar.module» system.module
Assigned:Unassigned» Damien Tournoud

... and I believe I know the guy that introduced that.

On it.

new344.63 KB

webgrind screenshot, looks like Moshe's issue found some paths that rightly shouldn't be affected by this, but fixing this one would narrow that down a lot.

Assigned:Damien Tournoud» Unassigned

I'm not giving up hope of finding time to work on this, but I leave the floor open.

I've not been able to think of a good solution to this, but posted a patch over at #519046: Clean up toolbar menu code which fixes most of the symptoms without hopefully adding too much extra cruft.

Another reason system_admin_menu_block_access() does not make total sense is that there might be underlying tabs in the IA on D7. #545952: D7UX IA: move modules to config/modules is not viable as it is because ATM there are no default enabled modules exposing configuration to the configuration tab (admin/config path), so system_admin_menu_block_access() decides people should not have access, even though there is a tab underneath, which has content (but that is not rendered by system_admin_menu_block()).

Of course once we have core config items moved there, this will not be visible anymore, but it is still an issue for cases when the default tab on the item does not have accessible content but there are subtabs otherwise which have content.

Status:Active» Closed (duplicate)

#296693: Hide empty admin categories was rolled back, and is still open for fixes, marking this as duplicate.