Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Contextual links for nodes consist of a link to node/%node/edit, and another link to node/%node/delete.
We can easily check access to those links with node_access(). So there is no reason why we need to be calling menu_get_item(), which does a relatively expensive query, for every single node in a list of teasers, to get the same information in such a roundabout way.
Here's devel query output on the front page with ten nodes, out of the 90 queries, in the region to 15-20 of the database queries are from contextual links, and ten of those are due to this. Each of those queries takes 0.6ms on a very small database, which probably equates to at least 10ms per request added together along with the additional processing menu_get_item() has to do. Since that's a good 5-10% of the request, I'm marking this as critical - it's only going to get worse as more modules add their own contextual links, and we need to be setting a good example in core.
Comments
Comment #1
sunComment #2
good_man CreditAttribution: good_man commentedIn my fresh installation, I got only 0.08 for every menu_contextual_links() and about the same time for menu_get_item() in the front page.
From what you've explained I understand that you want to call node_access() instead of menu_get_item(), or before it to check access before getting full menu item info?
Comment #3
catchWell we need a mechanism to allow modules to define their own contextual links manually - which in this case would be node module defining an edit and delete link itself and calling node_access() on them, rather than passing of to contextual links item to do menu_get_item() - this indirectly calls node_access() in the end anyway, just through a much, much longer code path. I haven't looked at the contextual links code recently but in general this ought to be easy enough to do, even if it requires bypassing the normal mechanism completely.
Comment #4
good_man CreditAttribution: good_man commentedaha now I see it, but if we make a simplified and optimized version of menu_get_item() for nodes view in node.module, don't we make a duplication here?
Comment #5
sunWe could hack in a special case for node access, true, but we have separate permissions for editing and deleting nodes.
Comment #6
catchmenu_get_item() is cached now. It's probably too late to change how this all works now, and the caching will get rid of the worst of it for sites who care, so downgrading to normal.