Problem/Motivation

On cold caches we query all of the entity last installed schema definitions and then immediately query again to get the field definitions for individual entity types.

Steps to reproduce

Proposed resolution

Add the field storage definitions to the static and persistent cache when we look up all definitions, this reduces the number of both database queries and cache sets with a completely cold cache.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3587415

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.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

This is reminding me of #3131585: Performance regression caused by using the last installed entity definitions

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Here's a @longwave thought - how storing them in state but caching them on disk so they can be opcached?

catch opened merge request !15597

catch’s picture

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

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

catch
he/him
Primary language English
commented

caching them on disk

We need to invalidate them when schema definitions are updated, which means they need to be consistent across multiple web heads. Discovery cache is already chained fast so gets are from APCu.

amateescu’s picture

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

Looks great to me!

catch’s picture

catch
he/him
Primary language English
commented

Looking back at #3131585: Performance regression caused by using the last installed entity definitions I think this is more or less a direct follow-up of that issue .

The way it's implemented now, to get the entity definitions we also load the field definitions, but then throw them away, then they're separately got and cached when field definitions for an individual entity type are requested.

The individual cache items for field storage definitions make sense to save loading everything in the 'cool cache' state when only some might be needed.

This is also covered by performance tests - when I wasn't writing back to the persistent cache in earlier iterations, it was just moving those writes from the completely cold to less cold requests. When we write back with ::setMultiple() it's neutral for the 'cool' cases but still an improvement for the cold case.

alexpott’s picture

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

Committed e385751 and pushed to main. Thanks!

We need a new MR for 11.x

  • alexpott committed e3857514 on main 
    perf: #3587415 Avoid querying for last installed schema definitions...

catch opened merge request !15599

catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Reviewed & tested by the community

Added a backport MR - only performance test numbers changed so back to RTBC.

  • alexpott committed afdb3e69 on 11.x 
    perf: #3587415 Avoid querying for last installed schema definitions...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

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.