We use this module to build a mega menu for a website, because of the design the menu needs to be expanded up to 4 levels, and the page takes nearly 8 seconds for the first load after a cache clear, the second refresh serves from cache takes less time, but on a new page, the first load still takes nearly 8 seconds.

I managed to track this down to `getMenuLinkItemContent` method of `src/service/MenuLinkTreeHandler.php` file, it lists `url.path` and `url.query_args` as cache contexts, which means the link item content needs to be rebuilt every time for a new page, also this will populate the cache-store with considerable amount of cache for the same menu. The question is, is this data really necessary to vary by URL?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

weynhamz created an issue. See original summary.

weynhamz’s picture

For the moment, remove these two 'url.path' and 'url.query_args' contexts. I guess the better approach is to check how core `MenuLInkTree.php` handle the menu item caches.

weynhamz’s picture

Status: Active » Needs review
weynhamz’s picture

Issue summary: View changes
ozin’s picture

Hi @weynhamz,
We will check the patch!
So what is the performance now?

weynhamz’s picture

After clearing the cache, the first-page build still takes nearly 8 seconds, but after that, all the othe pages now takes 1~3s for TTFB without Varnish depending on the complexity of the page.

weynhamz’s picture

After removing the above mentioned two cache contexts, the performance improved a lot, but it also brings another problem, the menu active link is cached as well. Attached the patch introduced a new cache context specifically for link item content, makes the cache varying by link active trail state.

Status: Needs review » Needs work

The last submitted patch, 7: 3082978-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

weynhamz’s picture

Fix tests.

weynhamz’s picture

Status: Needs work » Needs review
b_sharpe’s picture

Status: Needs review » Reviewed & tested by the community

Definitely needed! TTFB was getting killed on a site with a larger menu without this patch.

  • ozin committed 5c3b678 on 8.x-2.x authored by weynhamz
    Issue #3082978 by weynhamz, ozin: Performance issue on large menu
    
ozin’s picture

Status: Reviewed & tested by the community » Fixed

Booom, cache improved! @weynhamz thanks for the patch

Heine’s picture

Thanks, I've tracked down massive cache bloat to this issue. The change has been pushed to 8.x-2.x, but constitutes a BC break on the changed signature of MenuLinkTreeHandlerInterface::processMenuLinkTree .

Shouldn't this be pushed to 8.x-3.x instead?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.