Closed (outdated)
Project:
Tour
Version:
2.0.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
2 Jul 2013 at 19:55 UTC
Updated:
5 Sep 2024 at 19:54 UTC
Jump to comment: Most recent
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.
Add back the per-page caching of tours and profile the performance improvement.
Write the patch
Add tests
Profile it
Review
None
None
#1744302: [meta] Resolve known performance regressions in Drupal 8
Comments
Comment #1
tim.plunkettThis would be partially mitigated by #1918768: Refactor tour module to use routes instead of paths, but still worth doing.
Comment #2
moshe weitzman commentedI 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.
Comment #3
larowlanThe 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.
Comment #4
tim.plunkettAgreed 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.
Comment #17
quietone commentedThis extension is being deprecated, see #3336033: [Meta] Tasks to deprecate Tour module. It will be removed from core and moved to a contrib project, #3376099: [11.x] [Meta] Tasks to remove Tour.
This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
Comment #19
smustgrave commentedComment #20
smustgrave commentedComment #21
smustgrave commented@larowlan would you mind looking at the tour_toolbar() for the 2.0.x branch of contrib to see if the issue you saw still exists?
Comment #22
smustgrave commentedWe've added a bit of caching in 2.0.x version so believe this may be outdated.
If I'm wrong please reopen.