#2256521-42: [META] New plan, Phase 2: Implement menu links as plugins, including static admin links and views, and custom links with menu_link_content entity, all managed via menu_ui module identifies that the menutree refactor as it stands now moves route-to-path generation from MenuLink::preSave() to theme_menu_link(), causing a performance regression. While I agree with the comment there that HEAD is wrong for saving it at the MenuLink level, this patch fixes that regression by moving that generation to MenuTreeStorage::treeDataRecursive() where it can end up getting cached within loadTreeData().

In the long run, we may end up not wanting this, if some other issue ends up implementing a UrlGenerator cache or if #1805054: Cache localized, access filtered, URL resolved, and rendered menu trees ends up getting good hit ratios on cached rendered menus.

However, in the meantime this patch is here in case we decide that it shouldn't be #2256521: [META] New plan, Phase 2: Implement menu links as plugins, including static admin links and views, and custom links with menu_link_content entity, all managed via menu_ui module's scope to introduce an interim regression.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

Since this method is deprecated, it doesn't seem like the right way forward. That's why it's removed in the other patch.

effulgentsia’s picture

FileSize
2.91 KB

Since this method is deprecated, it doesn't seem like the right way forward. That's why it's removed in the other patch.

There's lots of things in HEAD that aren't right. The question is when does improving them justify performance regressions. If someone is willing to commit that issue's giant refactor despite the performance regression, I won't object. Just in case though, keeping this patch updated.

pwolanin’s picture

@effulgentsia - we can cache it much more effectively at the render layer, so given that that's the final goal, I don't want to go backwards on this.

Wim Leers’s picture

Issue tags: +Performance