Contrib.social

Use yield/generator to provide active trail URL candidates and other performance improvements

Open on Drupal.org →
Created on 25 July 2024, 4 months ago
Updated 26 July 2024, 4 months ago

Problem/Motivation

menu_trail_by_path frequently shows up as a performance issue in our monitoring and profiling. 2.x improved things a lot, but the current implementation is still less than optimal.

Right now, we prepare the whole trail hierarchy, do route lookups to verify if each part is a valid path. As soon as we have a match, all the extra work is wasted.

Steps to reproduce

Proposed resolution

Replace \Drupal\menu_trail_by_path\MenuTrailByPathActiveTrail::getTrailUrls() with a generator function that prepares the trail URLs one-by-one as we check them. Merge getCurrentPathUrls() and getCurrentRequestUrl() into that.

2.x dropped a separate service and moved them back into the main active trail service. I'm considering to revert that so that providing the trail is a separate, public API that is easier to override.We do that in a project.

Remaining tasks

Figure out if this is a BC break and requires a new major version. Possibly, because while we could keep the old methods and deprecate them, we can't account for subclasses that have customized the existing methods, this will break if we stop calling them.

I also really don't like the the current language prefix behavior as well as the current URL one that prefers the URL and looking it up over using the already available route match information.

User interface changes

API changes

Data model changes

📌 Task
Status

Needs review

Version

2.0

Component

Code

Created by

🇨🇭Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @berdir
  • Merge request !13initial version of trail url provider using a generator → (Open) created by berdir
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇨🇭Switzerland berdir Switzerland

    This seems quite beneficial in early testing on our project.

    I have some more ideas on improvements, including caching the generator output when it's enabled for multiple menus*.

    We'll test and monitor this on production, but more testing is welcome too, the impact of this should be better the deeper your path hierarchy is, for example with multiple hierarchical categories that have articles below.

    * This should be avoided whenever possible to limit the overhead of this, typically only some sort of main menu needs to support trail by path, while many other menus (e.g. footer and header menus) only need none or even no active trail, which can greatly improve performance.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024