Problem/Motivation

At the moment, when altering the language switcher links with the translated URL alias, all the facets are always loaded and a nested loop occurs for each facet and language. This can become quite slow on sites with many facets and many languages as all the facets are loaded on each authenticated page request since the language switcher links are not cached.

Steps to reproduce

Proposed resolution

Cache the data the alter needs to replace the filter URL aliases.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 3198510-3.patch9.88 KBUpchuk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Upchuk created an issue. See original summary.

Upchuk’s picture

Upchuk CreditAttribution: Upchuk at European Commission and European Union Institutions, Agencies and Bodies, WEBOMELETTE commented
Related issues: +#2893374: Translate url alias
Upchuk’s picture

Upchuk CreditAttribution: Upchuk at European Commission and European Union Institutions, Agencies and Bodies, WEBOMELETTE commented
Status: Needs work » Needs review
FileSize
3198510-3.patch9.88 KB

Here is an attempt at improving this.

I made a service where we do a first iteration and cache the info needed for the facet URL aliases (translated alias, segment, filter key) and we cache this. Any subsequent calls to facets_language_switch_links_alter will deal only with this cached array of data rather than always having to load facets and reconstruct stuff.

Initially I thought that tests were missing for this but i saw they are there (created in #2893374).

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
Status: Needs review » Reviewed & tested by the community

This is hard to review because there is not a big overlap with the current files. @Upchuk explained what they were going to do before this patch arrived. It looks like a good idea.

  • borisson_ committed bcb8f92 on 8.x-1.x authored by Upchuk 
    Issue #3198510 by Upchuk: Language switcher links alter performance
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Calibrate commented
Status: Reviewed & tested by the community » Fixed
Chewie’s picture

Chewie CreditAttribution: Chewie for European Commission and European Union Institutions, Agencies and Bodies commented 
+++ b/facets.module
@@ -358,61 +358,12 @@ function facets_system_breadcrumb_alter(Breadcrumb &$breadcrumb, RouteMatchInter
+  /** @var \Drupal\facets\LanguageSwitcherLinksAlterer $alterer */
+  $alterer = \Drupal::service('facets.language_switcher_links_alterer');
+  $alterer->alter($links, $type, $url);
 }

It would be better to check that this service exists especially if we introducing it in this change.

+++ b/facets.module
@@ -358,61 +358,12 @@ function facets_system_breadcrumb_alter(Breadcrumb &$breadcrumb, RouteMatchInter
 }
 
+
 /**
  * Implements hook_form_FORM_ID_alter().
  */
diff --git a/facets.services.yml b/facets.services.yml

Unnecessary line.

I can confirm that changes look good for me and really improving performance.

Status: Fixed » Closed (fixed)

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

droddis’s picture

droddis CreditAttribution: droddis commented

Hi there, Does anyone know if this has been rolled into 1.8, or should we be using the dev version? If not, have there been enough changes that we should not implement on the production version of 1.8?

I've been a bit worried about the security update from 1.6 to 1.8.

thanks in advance,

David