Problem/Motivation

The active trail cache collector stores the calculated active trail per route + parameters for all menus on a page.

Even though it's a per route + params cache, this is used for the active trail cache context which is used to determine render cache IDs, so we probably need to keep the cache. #3529464: Make menu trail behaviour in SystemMenuBlock optional would make it possible to skip this logic entirely.

We can skip cache collector locking here because the cache isn't additive - i.e. it's not accumulating across multiple pages, it's just collecting the same handful of things per page. Also the chances of multiple requests all trying to build the same item are quite low anyway, and even if that happens occasionally, it would be outweighed by all the lock acquires and releases when it doesn't happen.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3560670

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch opened merge request !13996

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

With that logic, do we need to use the CacheCollector at all, can't we just use a regular cache lookup then? Is it just for the static cache in case of multiple requests? That could use a backend chain then?

catch’s picture

catch
he/him
Primary language English
commented

With that logic, do we need to use the CacheCollector at all, can't we just use a regular cache lookup then?

The cache collector still handles collecting (sorry) the various active trails that are requested via menus during the request, which would be harder to do without it.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

That makes sense. Perfomance tests conflict, but happy to RTBC after this.

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#3529464: Make menu trail behaviour in SystemMenuBlock optional

#3529464: Make menu trail behaviour in SystemMenuBlock optional affects the same lock queries in the same performance tests, hoping to land that one first because it has much wider impact (if you configure it).

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

Rebased, taking @berdir's #7 as a license to RTBC this. Only standard performance tests need updating now because Umami doesn't use the active trail at all after #3529464: Make menu trail behaviour in SystemMenuBlock optional.

godotislate made their first commit to this issue’s fork.

  • alexpott committed e316198f on 11.x 
    perf: #3560670 Don't lock on the active trail cache collector cache...

  • alexpott committed 19ea2283 on main 
    perf: #3560670 Don't lock on the active trail cache collector cache...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 19ea228 and pushed to main. Thanks!
Committed e316198 and pushed to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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