While investigating performances issues on one of our site, I noticed that enabling the XML sitemap menu module was responsible for a time increase of about 40% for a single drush cc menu
command.
Tracing the issue, this is caused by xmlsitemap_menu_xmlsitemap_process_menu_links
be called to process (called from xmlsitemap_menu_menu_link_alter
) each menu item individually during menu rebuild.
I may be wrong, but there is little gain in immediate process of a menu link from a menu no used in the site map.
Would it make sense to only process menu items form menu actually in the sitemap. On our site, and I guess many other, it would greatly reduce the load during menu rebuild.
Comment | File | Size | Author |
---|---|---|---|
#10 | xmlsitemap-skip-rebuilds-2646152-10.patch | 2.76 KB | nplowman |
| |||
#5 | xmlsitemap-skip-rebuilds-2646152.patch | 2.55 KB | quicksketch |
Comments
Comment #2
pbuyle CreditAttribution: pbuyle at Floe design + technologies commentedThe attached patch implement the proposed solution.
Comment #3
pbuyle CreditAttribution: pbuyle at Floe design + technologies commentedComment #4
quicksketchThis looks great @pbuyle! I ran into the same issue with XMLSitemap having a rather devastating effect on menu rebuilds. I applied this patch and it had a tremendous benefit:
Before: 6307 queries in 2053.25 ms Page execution time was 11350.89 ms
After: 2091 queries in 942.86 ms Page execution time was 6986.62 ms
It seems like this could also be applied to
xmlsitemap_menu_menu_delete()
andxmlsitemap_menu_menu_link_insert()
, but those are very minor in comparison to the change made here inxmlsitemap_menu_menu_link_alter()
. Hooks for insert/update/delete are not called during rebuilds, buthook_menu_link_alter()
is.So until #1013856: hook_menu_link_update() not invoked for menu links that have other elements form_alter()'d in is fixed, this work-around helps menu rebuilds tremendously. In our case, the inclusion of the "management" menu was about 90% of the links being loaded and saved into the xmlsitemap table, none of which you'd ever want to be displayed in the sitemap.
Comment #5
quicksketchI think #2 is a fine patch and could be committed as-is. However if we want to avoid the unnecessary entries in the xmlsitemap table entirely for unused menus then this patch will skip the inserting of these items entirely. They will be added during the menu rebuild if the menu is enable later. For consistency with new sites, I added an update hook that removes all the unnecessary entries from the xmlsitemap table.
Comment #6
quicksketchRTBC #2. I'll leave it to the maintainers whether they think #2 or #5 is a better approach. Either solves the critical issue that we unnecessarily recreate xmlsitemap entries for disabled menus. #5 just builds on #2 to entirely avoid/remove the entries in the xmlsitemap table.
Comment #7
candelas CreditAttribution: candelas as a volunteer commentedPatch #5 works great. I was having timeouts and after this patch all works perfect. My xmlsitemap table went from 1234 entries to 325. Thanks a lot!
Comment #8
pifagorComment #10
nplowman CreditAttribution: nplowman at FFW commentedAttaching an updated patch that works with 7.x-2.4.
Comment #11
candelas CreditAttribution: candelas as a volunteer commented