Optimize _menu_navigation_links_rebuild()

Thanks for this. I believe earlier in #512962: Optimize menu_router_build() / _menu_router_save(), you said that it would be possible to create a version of menu_link_save, delete, etc. that was optimized to do the INSERT all in one query, or at least in batches (if it would be too much to load in memory all at once). Any more thoughts along those lines?

Also, there is an issue that discusses the number of no-op SELECT queries that menu_rebuild generates: #302638: No-op and slow queries during menu rebuild. That issue had a few patches, one of which even made it in to an earlier version of 6.x, but was rolled back. Now that issue is stalled. Perhaps that would be relevant too.

chx, sun, pwolanin, moshe_weitzman, Damien Tournoud, et al.: do you have any suggestions?

subscribe... EDIT: 6.x issue below
http://drupal.org/node/512962#comment-2522418

looking at http://api.drupal.org/api/function/_menu_navigation_links_rebuild/6 seems like the quickest way to speed this up is to use db_multi_select for the second loop of this function. All the other queries seem to grab multiple rows. This is the code block I'm thinking of optimizing; if some one could verify my guess that would be nice.

  if ($menu_links) {
    // Make sure no child comes before its parent.
    array_multisort($sort, SORT_NUMERIC, $menu_links);

    foreach ($menu_links as $item) {
      $existing_item = db_fetch_array(db_query("SELECT mlid, menu_name, plid, customized, has_children, updated FROM {menu_links} WHERE link_path = '%s' AND module = '%s'", $item['link_path'], 'system'));
      if ($existing_item) {
        $item['mlid'] = $existing_item['mlid'];
        // A change in hook_menu may move the link to a different menu
        if (empty($item['menu_name']) || ($item['menu_name'] == $existing_item['menu_name'])) {
          $item['menu_name'] = $existing_item['menu_name'];
          $item['plid'] = $existing_item['plid'];
        }
        $item['has_children'] = $existing_item['has_children'];
        $item['updated'] = $existing_item['updated'];
      }
      if (!$existing_item || !$existing_item['customized']) {
        menu_link_save($item);
      }
    }
  }

This doesn't address menu_link_save() ATM. Main issue I see with doing a multiple database operation is http://api.drupal.org/api/function/db_last_insert_id/6

The other thing I see is to do a db_multi_update on this code

  $placeholders = db_placeholders($menu, 'varchar');
  $paths = array_keys($menu);
  // Updated and customized items whose router paths are gone need new ones.
  $result = db_query("SELECT ml.link_path, ml.mlid, ml.router_path, ml.updated FROM {menu_links} ml WHERE ml.updated = 1 OR (router_path NOT IN ($placeholders) AND external = 0 AND customized = 1)", $paths);
  while ($item = db_fetch_array($result)) {
    $router_path = _menu_find_router_path($item['link_path']);
    if (!empty($router_path) && ($router_path != $item['router_path'] || $item['updated'])) {
      // If the router path and the link path matches, it's surely a working
      // item, so we clear the updated flag.
      $updated = $item['updated'] && $router_path != $item['link_path'];
      db_query("UPDATE {menu_links} SET router_path = '%s', updated = %d WHERE mlid = %d", $router_path, $updated, $item['mlid']);
    }
  }

Knowing which one is slower would be nice. Currently too busy to benchmark ATM.

Subscribe

The following is a patch for D6, as that's what I'm currently needing this to work on, not sure how simple it would be to port to D7.

This patch would still need work, as it could probably use some memory optimization. I don't have any benchmarks, but regular rebuilds seem to be much much quicker. There's also room for improvement by doing something similar for the deleting of orphaned links, and adjusting changed router paths.

Tracking.

Managed to completely miss this issue, however there's an RTBC Drupal 7 patch here that removes a lot of the queries done here #1010480: Optimize _menu_navigation_links_rebuild(). Didn't compare it to the Drupal 6 patch.

#7 patch seems not speed up my system. I use Pressflow and Ubercart together (installed, but all modules disabled) and use devel for testing speed. It's around 70 sec and 1400 queries, not depends from patch. I test it with enabling/disabling Aggregator module.

Devel module shows me two functions which need to be optimized:
1. _menu_update_parental_status
2. menu_link_save
Both have "UPDATE menu_links" queries.
(1) have only "UPDATE menu_links SET has_children = 1 WHERE mlid = [different numbers]"
(2) have more complex UPDATE like "UPDATE menu_links SET menu_name = 'admin_menu', plid = 457, link_path = 'admin/settings/site-information', router_path = 'admin/settings/site-information', hidden = 0, external = 0, has_children = 0, expanded = 0, weight = 0, depth = 2, p1 = 457, p2 = 511, p3 = 0, p4 = 0, p5 = 0, p6 = 0, p7 = 0, p8 = 0, p9 = 0, module = 'admin_menu', link_title = 'Site information', options = 'a:1:{s:5:\"alter\";b:1;}', customized = 0 WHERE mlid = 511" Everywhere I see "SET menu_name = 'admin_menu'" i.e. admin menu rebuilt.

While this is the older issue, I'm marking it as duplicate of the issue linked in #9 since that already has an rtbc patch against Drupal 7.