Persistence: object tree validation performance is really slow

neos-bot commented 10 years ago

Jira issue originally created by user jpaardekooper:

Our application has some serious problems with the performance of persisting data of large data sets. I'll try to explain it as clear as possible with a basic scenario.

Say our application has users that can be linked to one or more companies. A company is fairly large object with alot of OneToMany relations, for example Cars, Houses, Pets etc. Alot of stuff. Most of these objects have their own repository as well.

We have a UI that lets us create a Car and link it to a company. The performance of this is fine when the current user isn't link to alot of Companies that don't have alot of Cars, Houses, Pets etc. However, as soon as the data set becomes larger (say, 50 companies each with 25 Cars, Houses and Pets), it takes about 30 seconds to insert one simple Car.

The basic object model:

class Company {

    /****
     * @var string
     */
    protected $name;

    /****
     * @var \Doctrine\Common\Collections\ArrayCollection<Car>
     * @ORM\OneToMany(mappedBy="company")
     * @Flow\Lazy
     */
    protected $cars;

    // alot more fields
}

class Car {

    /****
     * @var string
     */
    $licensePlate

    /****
     * @var Company
     * @ORM\ManyToOne(inversedBy="cars")
     */
    protected $company;

    // some more fields
}

I've tracked the performance down to the TYPO3.Flow/Classes/TYPO3/Flow/Persistence/Doctrine/PersistenceManager.php file, where the Car object is triggered for validation (line 82). It then goes into the validateObject method and validates the Car. But when it validates the Car, it will also validate the Company and the Company will also validate all the Houses, Pets, etc.

A temporary solution is to add the @Flow\IgnoreValidation annotation to the $company property in Car. I don't think that's a very flexible and nice solution and wonder if it could be done better.

Jira-URL: https://jira.neos.io/browse/FLOW-17

neos-bot commented 10 years ago

Comment created by @albe:

I'd suggest solving that by providing @Flow\ValidationGroups annotation for entities. That way, one could annotate the company as @Flow\ValidationGroups("{Controller}") so it is only validated inside controller actions, but not during persistence any more. Of course that completely bypasses persistence validation for that entity, and possibility of invalid data inside the persistence arises when properties are changed inside the controller. That is a business decision though.

A second step could be to do peristence validation only within the bounded contexts, ie only up to aggregate roots. This would match default persistence cascading behaviour, but would also be harder to implement, as we'd have to copy some of the doctrine logic into the object validation (possibly including cascade annotations).

neos-bot commented 10 years ago

Comment created by @albe:

For record keeping, here is a post from the mailing list which relates to this: {quote}First of all, let's take the example of submitting a form to create a new instance of a model. In my case, I have models with complex relationships (an Asset is associated with a Version of a Software, which is associated with a Software, which has Modules, etc.). When I submit a form to create a new instance of an Asset, the validation framework will validate ALL members of my model recursively. So it will validate my Asset fields (name, date, author, etc.) as well as the associations it has to other models, even though those models were not created by the form (Version, Software, Modules). This creates unnecessary queries to the database (to load the other models to validate) as well as wasted CPU cycles and thus the request takes more time to complete. [..]

A smart validation stack would only validate the model's members which were submitted with the form, and not the other associated models (which are only referenced by their unique identifier).

The same goes when the PersistenceManager validates all scheduled insertions/updates to the database: it will blindly validate all members of the models, regardless of the changesets that will be applied in the database. There is no need to re-validate the model all over again when only certain fields have changed. {quote}

The important parts are bolded and suggest the perfect validation logic: Controller validation only validates submitted properties and persistence validation only changeset records. Both cases are hard to detect at validation time though, so this is not trivial to implement.

neos-bot commented 10 years ago

Comment created by @albe:

Some ideas on how to implement the optimized validation:

For property mapping:

real overhead is only induced when validating related entities, which were not submitted or only submitted by **identity in which case the validation of those (persisted) entites can be skipped. Checking this for each property is much more work and does not really improve performance a lot unless you work with entities with thousands of properties.
hence one could just mark all reconstituted objects on which additional properties get set in PersistentObjectConverter to have to be validated. All other reconstituted objects are marked to be skipped for validation. The GenericObjectValidator would then only execute when the skip flag is not set for the validated entity.
the only problem is how to store this information per object in a way that does not interfere with the object itself and is transient for manual validation invocations (check current validation group to be Controller?)

For persistence:

an easy and generic optimization would be to only validate hierarchy up to the next aggregate root. Syncing this behaviour to ORM Cascade annotations would be possible too, but more complex.
the best thing would be to only validate entities that have changes, hence everything that has a changeset in doctrine's UnitOfWork. This would mean that for each entity that is about to be validated one would have to check UnitOfWork::getEntityChangeSet($entity) and check if that is empty. If so, skip validation for that entity, but only in Persistence validation group of course.