Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
The \Drupal\diff\Controller\PluginRevisionController::compareEntityRevisions() unnecessarily loads every revision of an entity just to work out the list of revisions for a language. On a site where a node has 4000 revisions this takes over 20 seconds locally and breaks on production. It does over 48 million function calls...
Steps to reproduce
Create a node with lots of revisions...
Proposed resolution
Use an entity query do this in one go... we have an example to copy from core - see \Drupal\Core\Entity\ContentEntityStorageBase::getLatestTranslationAffectedRevisionId()
On my production site this makes the comparison possible whereas before it was a WSOD. And on a local dev site:
- HEAD: locally takes 23 seconds with 48 million+ function calls
- MR: locally takes 1 seconds with 3 million function calls
Remaining tasks
User interface changes
None
API changes
\Drupal\diff\Controller\PluginRevisionController::getRevisionIds() is no longer called. Will need to file an issue in Workspaces Extra because \Drupal\wse\Controller\WseDiffNodeRevisionController extends this. But the module is only alpha. - tweaked approach makes the change in a BC compatible manor.
Data model changes
None.
Issue fork diff-3568465
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:
changes
Comments
Comment #3
alexpottThis is a duplicate of #3418442: Revisions should be filtered by language in the database query but I think that that issue has got sidetracked and started doing more than just fixing the gaping performance issue. I think we could deprecate \Drupal\diff\Controller\PluginRevisionController::getRevisionIds() and add a tag to the query being added here just in case workspaces needs to do something to it and be done.
Comment #4
alexpottI would land this in 8.x-1.x and 2.x and then do then do the deprecation of \Drupal\diff\Controller\PluginRevisionController::getRevisionIds() in 2.x only - perhaps using the existing issue.
Comment #5
alexpottComment #6
alexpottComment #7
alexpottComment #8
dwwSweet, thanks! The description from the summary here sounds great.
The latest weekly 8.x-1.x pipeline has the warnings about cspell, eslint and stylelint: https://git.drupalcode.org/project/diff/-/pipelines/717740
The failures in the pipeline in here seem new: phpcs + phpunit.
NW for that. Will look more closely at the diff once the pipeline is happy.
Thanks again!
-Derek
Comment #9
acbramley commentedYou're right, but there's so much overlap between the performance issue and the handful of other duplicates around language/revisions not showing that I figured it best to copy the now accepted fix from Core in #2722307: Move translation based conditions into database query on revisions overview page and just use that here.
(quoting from slack)
I'm not sure what you mean by stuck? The issue is in needs review and it (was) passing tests - I will look into why these are failing now.
8.x-1.9 was slated as the final 8.x-1.x release, I don't see a reason to still be on that branch unless you can tell me otherwise I'd prefer this just go into 2.x so we can mark 8.x-1.x as unsupported.
Comment #11
acbramley commentedhttps://www.drupal.org/project/diff/releases/8.x-1.10