Problem/Motivation

When you have lots of jsonapi routes the \Drupal\views\Plugin\views\display\PathPluginBase::alterRoutes() method does lots and lots of unnecessary work.

Steps to reproduce

Install JsonAPI and views and rebuild the routes.

Using the standard profile this change results in 2220 less calls to \Drupal\views\Plugin\views\display\PathPluginBase::overrideAppliesPathAndMethod(). On a client site which makes heavy use of JsonAPI and has a *lot* of custom entity types and bundles of said entity types this results in 290556 less calls!

Blackfire runs:

Proposed resolution

Change the order of the if condition.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#2 3248649-2.patch917 bytesalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Issue summary: View changes
Status: Active » Needs review
FileSize
3248649-2.patch917 bytes
vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer and at Solathat, Acro Commerce commented

Wow, that's a massive gain (esp. on the client site) for a change of order of conditions checked.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer and at Solathat, Acro Commerce commented
Status: Needs review » Reviewed & tested by the community
bbrala’s picture

bbrala
Location Netherlands
CreditAttribution: bbrala at SWIS commented

Agree, was also reviewing lol. Rtbc agreed.

  • larowlan committed 1e05f80 on 9.4.x 
    Issue #3248649 by alexpott: \Drupal\views\Plugin\views\display\...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Title: \Drupal\views\Plugin\views\display\PathPluginBase::alterRoutes() can become very slow on a site with lots of entities and JsonAPI » [backport] \Drupal\views\Plugin\views\display\PathPluginBase::alterRoutes() can become very slow on a site with lots of entities and JsonAPI
Version: 9.4.x-dev » 9.3.x-dev

Committed 1e05f80 and pushed to 9.4.x. Thanks!

Holding off backport until I ascertain whether we're in beta freeze.

  • larowlan committed 68f2ce3 on 9.3.x 
    Issue #3248649 by alexpott: \Drupal\views\Plugin\views\display\...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Title: [backport] \Drupal\views\Plugin\views\display\PathPluginBase::alterRoutes() can become very slow on a site with lots of entities and JsonAPI » \Drupal\views\Plugin\views\display\PathPluginBase::alterRoutes() can become very slow on a site with lots of entities and JsonAPI
Status: Reviewed & tested by the community » Fixed

Freeze hasn't started yet

Backported to 9.3.x

Status: Fixed » Closed (fixed)

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

donquixote’s picture

donquixote CreditAttribution: donquixote as a volunteer commented

With this optimization we still have O(n*m) cost.
n = number of routes
m = number of views displays with path plugins, or other route subscribers that need to scan all routes.

So there is still a problem on some sites.

Follow-up: #3372094: Optimize route subscribers using indexed route collection