Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Right now, every time we save an entity, we delete all rows in the data and revision data tables and insert them again.
However, many fields never change and especially when working with translations, in the 90% (at least) use case, only one translation actually changes.
The only exception is saving a new revision in the revision data table, then we just insert.
Proposed resolution
Instead, do the following:
1. If this is a new entity or a new revision for the revision data table, just insert.
2. If there are any removed languages, delete those rows.
3. Then loop over the still-existing languages
3a) The language is new, just insert
3b) The language existed before. Map the loaded entity, get the differences and do an update query just for those, if there are changes.
I don't know too much about DB internals, but I'd expect this to be *considerably* faster in many scenarios. Will run some tests soon.
Example with adding a new translation:
HEAD: DELETE, N insert queries (N = total amount of languages)
Patch: 1 insert query (existing translations did not change)
Similar with e.g. updating one translation out of 5, then on HEAD, we do a DELETE and then 5 inserts, with this, we only need to do a single update query for the fields that really changed.
There's a certain amount of overhead, due to loading the revision, due to having to map it, but I would assume this is still faster in almost all cases.
This also explicitly uses the new loaded revision ID insted of $entity->original. I'm pretty sure we have some very nasty bugs related to that and conditional field saving. Need to open an issue.
Remaining tasks
* Apply the same to base and revision tables?
* Avoid loading the loaded revision multiple times. Store it or pass it around somehow.
* Profile/Benchmark this.
* Fix tests/code.
User interface changes
API changes
Data model changes
Comments
Comment #2
BerdirComment #3
BerdirI did some profiling on this, but I'm not quite seeing the results I was hoping for. One reason is that mapToStorageRecord() is expensive.
With a single language, blackfire.io causes enough overhead to make it considerably slower. With just some manual microtime() comparisons, it seems to be slightly faster with the attached optimzation (aka, when saving the default revision).
With multiple translations, it gets a bit faster, but still not as much as I was hoping.
Comment #7
JacobSanford@Berdir, your patch in #3 no longer applied to 8.6.x. A reroll with no further modifications is attached.
Interdiff empty due to reroll.
Regards!
Comment #8
matsbla CreditAttribution: matsbla commentedComment #9
hchonovI guess we'll not have to use
mapToStorageRecord()once #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) is ready and then we'll see the results you were hoping for :).Comment #10
BerdirYes, that might help. We still have to use it, but only once and that one call can be optimized to only look at changed fields from the start :)