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.
On every page in HEAD with shortcut module enabled we have the following queries:
SELECT s.set_name AS set_name FROM shortcut_set s INNER JOIN shortcut_set_users u ON s.set_name = u.set_name WHERE (u.uid = :db_condition_placeholder_0)
SELECT ss.* FROM shortcut_set ss WHERE (set_name = :db_condition_placeholder_0)
These could be consolidated into a single query with a join. Here's the relevant snippet from shortcut_current_displayed_set().
Additionally, there's absolutely no reason why this should be using db_select().
// If none was found, try to find a shortcut set that is explicitly assigned
// to this user.
$query = db_select('shortcut_set', 's');
$query->addField('s', 'set_name');
$query->join('shortcut_set_users', 'u', 's.set_name = u.set_name');
$query->condition('u.uid', $account->uid);
$shortcut_set_name = $query->execute()->fetchField();
if ($shortcut_set_name) {
$shortcut_set = shortcut_set_load($shortcut_set_name);
}
Comments
Comment #1
catchThis is a bit worse than I originally realised:
shortcut_current_displayed_set() calls menu_load_links()
However shortcut_renderable_links() calls menu_tree($menu_name) - which then calls http://api.drupal.org/api/function/menu_tree_page_data/7 - which gets the links from cache anyway, and also has language support - as far as I can see links from menu_load_links() are queries then thrown away on most requests.
So the menu_load_links() call needs to go too.
Comment #2
David_Rothstein CreditAttribution: David_Rothstein commentedIs this really critical? I'm pretty sure these queries only run for users who have appropriate permissions. And also, only on pages where the shortcuts will actually be displayed (except perhaps for http://api.drupal.org/api/function/shortcut_page_build/7 which has some sketchy stuff going on in it that could probably be improved).
I think having two versions of shortcut_set_load() (one of which would be a lightweight version that did not load the menu links) was discussed in the original issue and decided against in the name of a cleaner API..... We could obviously revisit that if necessary, but is it really a big performance hit? Seems like we are talking about a single database query that should be relatively fast.
Comment #3
catchOriginally I'd thought the menu_load_links() was being used to render the shortcuts, which would've meant they bypassed the translatable query tag, which would've been critical, but forgot to change the status back as I followed the code flow, back to normal.
However it's still three queries run on every page for no good reason, whereas it could be one. And executing a page callback now takes a baseline of one database query if using a non-database caching backend, so a couple of queries can easily be 10/20% of those required to execute a request. I also don't see why a separate round trip to the database to load something into memory when there's a 99% chance it won't be used should be considered 'clean'.
Comment #4
catchComment #5
webchickSubscribing.
Comment #6
sun.core CreditAttribution: sun.core commentedNot critical. Please read and understand Priority levels of Issues, thanks.