Problem/Motivation

Found in #3512762-10: Optimize placeholder retrieval from cache. Placeholder strategies operate in a response event, not at every level of a render array. This means that if a placeholder is cached and has placeholders nested inside it, these aren't caught by CachedStrategy and will fall back to individual cache gets when the renderer reaches them.

However, we should be able to recursively retrieve the placeholders from cache within CachedStrategy.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3513928

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 !11537

catch’s picture

catch
he/him
Primary language English
commented
Title: Recursively replace placeholders in CachedStrategy » [PP-1] Recursively replace placeholders in CachedStrategy
Status: Active » Needs work

Doesn't show an improvement in isolation, but will after #3512762: Optimize placeholder retrieval from cache lands.

We can mutliple cache get the placeholders from all cached placeholders I think, and also implement recursion in case those placeholders also return placeholders.

catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] Recursively replace placeholders in CachedStrategy » Recursively replace placeholders in CachedStrategy
Status: Needs work » Needs review
Parent issue: » #2888838: Optimize render caching
Related issues: +#3512762: Optimize placeholder retrieval from cache

Implemented the recursion. We don't have double-nested, cached, placeholders in core or at least not that I can think of, or that are covered by performance tests, but this does show the improvement for the single-nested cached placeholders in the navigation bar - e.g. shortcut and user menu placeholders are now one multiple get instead of two single
gets.

catch’s picture

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

catch
he/him
Primary language English
commented
Issue tags: +11.2.0 release highlights

Rebased on #3512766: Optimize redirect chain retrieval in VariationCache and #3512762: Optimize placeholder retrieval from cache.

Still removes one cache get in navigation performance tests, which is the shortcut and user menu placeholders being fetched from cache together.

catch’s picture

catch
he/him
Primary language English
commented

Back to green after #3512766: Optimize redirect chain retrieval in VariationCache

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Going to rely on the test performance for this one and fact it went down. Don't see anything glaring so going to go on a limb.

longwave credited kristiaanvandeneynde.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

  • longwave committed b3238209 on 11.x 
    Issue #3513928 by catch, kristiaanvandeneynde: Recursively replace...

longwave closed merge request !11537

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Although there aren't any specific tests I don't see why this would cause any problems, let's ship it and hopefully improve performance.

Committed b323820 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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