Updated: Comment #0


Tour module loads all tour config entities on every page and then filters them down to those required for that page. As these are relatively static objects they should be cached per page so subsequent views of the page don't require the full round trip again. This code existed in the original patch but was identified as 'premature optimisation' and subsequently removed.

Proposed resolution

Add back the per-page caching of tours and profile the performance improvement.

Remaining tasks

Write the patch
Add tests
Profile it

User interface changes


API changes


#1744302: [meta] Resolve known performance regressions in Drupal 8


tim.plunkett’s picture

This would be partially mitigated by #1918768: Refactor tour module to use routes instead of paths, but still worth doing.

moshe weitzman’s picture

I don't think we want to casually add more caching to app. To do so, we need benchmarks to show that Drupal is slow without the patch, and fast after it. And then we need to talk about why tour entities are special and so on.

larowlan’s picture

Title:Add per-page caching of loaded tours.» Add per-page caching of filtered and rendered tours.

The loading isn't the concern, its loading them all, filtering to those relevant to the page and then rendering those. But yeah agreed profile first. Will need to simulate more tours though as we've still only got a few in core.

tim.plunkett’s picture

Title:Add per-page caching of filtered and rendered tours.» Add per-page caching of filtered and rendered tours
Issue tags:+Needs profiling

Agreed we need profiling either way.

Tour is special because unlike other cases, we cannot use entity.query to do the filtering before loading (see the issue I liked before for why).

In most other cases, we try to pass a set of IDs to loadMultiple() first.

And yeah, also rendering.