Hi,
We were having performance issues with the Menu Trail By Path module. We have a high amount of menu items (1800+) and this was causing issues with the getActiveTrailLink function located in /src/MenuTrailByPathActiveTrail.php.
public function getActiveTrailLink($menu_name) {
$menu_links = $this->menuHelper->getMenuLinks($menu_name);
$trail_urls = $this->pathHelper->getUrls();
foreach (array_reverse($trail_urls) as $trail_url) {
foreach (array_reverse($menu_links) as $menu_link) {
/* @var $menu_link \Drupal\Core\Menu\MenuLinkInterface */
/* @var $trail_url \Drupal\Core\Url */
if ($menu_link->getUrlObject()->toString() == $trail_url->toString()) {
return $menu_link;
}
}
}
return NULL;
}
As you can see in the function, there is a foreach in a foreach. In this there is a getUrlObject call. This would mean if you are in a trail url 3 levels deep it does 1800 queries x 3.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | 2910494-25.patch | 18.79 KB | berdir |
| #23 | interdiff-22_23.txt | 755 bytes | Oscaner |
| #23 | 2910494-23.patch | 9.42 KB | Oscaner |
| #5 | performance-issues-high-amount-menu-items-2910494-5.patch | 2.75 KB | falco010 |
Comments
Comment #2
falco010Created a patch that first gathers all menu url strings and trail urls string in separate arrays. Then looping over them to only do the comparison check (without any queries).
Comment #3
falco010Comment #5
falco010After some testing, I found out the first patch gave some problems with the active trail.
Fixed that and some coding standards in this new patch.
Comment #6
falco010Comment #7
idebr commentedComment #8
R.Muilwijk commentedThe proposed patch still uses the Menu Helper which retrieves the full menu tree, load the MenuLink entities and the url objects in single entity gets.
This patch proposes a different solution where the path validator looks up the corresponding menu link items.
Comment #10
R.Muilwijk commentedFixed patch.
Comment #11
R.Muilwijk commentedThere is still a fail in the testUrlHome() I need to look into.
Comment #12
R.Muilwijk commentedComment #13
davy-r commentedComment #15
weynhamzHave a huge menu, performance is terrible due to this module, webprofiler show at least 1.5s on getActiveTrailIds, after applying patch from #12, webprofiler shows nothing and the page load is way faster.
Though the method in #12 heavily relies on the menu_tree table, and I have some menu items created by menu_item_fields, but not listed in the menu_tree table, thus it fails to find the active trail ids. After resaving the menu item, the item shows in the menu_tree table, and everything works.
Comment #16
adrian.conte commentedI've rerolled the patch for the latest dev version & v1.3
Comment #17
pameeela commentedComment #19
smustgrave commentedGoing to try this patch out. Did this succeed for anybody else? Our site has experienced performance issues and reports have been pointing back to this module. I'm only using it for 1 menu and checking 2 parts of the url but that menu is close to 2,000 links. If this works it be great to be included in the next release!
Comment #20
smustgrave commentedSo after pushing out this patch page loads have gone from 45+ seconds in some cases to under 4.
Also unrelated but one other thing we did was uninstall menu items extras as Acquia was flagging that on us. Not sure if it was uninstalling that module or this patch or the combo but something worked.
Would love to see this patch get rolled into the next release!
Comment #21
marco.aresu commentedThe patch number #16 works very well on Drupal 8.9 and Menu Trail By Path 1.3.
Comment #22
Oscaner commentedComment #23
Oscaner commentedComment #24
berdirCan confirm that this is a massive performance improvement. The amount of removed services and API's is a little bit problematic though, makes me wonder if this should be released as a new branch.
Also, the test fails will need to be invested and that case fixed.
Comment #25
berdirOk, I've been working on this quite some time.
The frontpage requires special handling to find links that actually point to , as the path validator is too intelligent about that and finds the configured frontpage route instead.
However, the whole thing still felt strange. trail urls were basically already the same thing and we've been converting them to string and back to a URL object. (we did Url::fromUserInput(), which actually runs the path validator too) And the extra loop isn't really needed because we only loop over these URL's once with the new approach. So several iterations of improvements later, I ended up removing the path helper service and simplifying and merging its code directly into the active trail service.
Also fixed and extended the tests.
This seems to work well for our use cases, I'd appreciate some feedback from others that were using previous iterations of the patch and then I plan to release this as a new 2.x major version.
No interdiff as a lot has changed anyway.
Comment #26
berdirCommit won't show up, but I've committed and released this as a new 2.x release: https://www.drupal.org/project/menu_trail_by_path/releases/2.0.0-alpha1.
Comment #27
berdir