Bug #37363: PersistenceManager->validateObject() ==> to many calls/loops/recursions somewhere in a subcall - TYPO3.Flow - TYPO3 Forge

TYPO3 Flow Base Distribution TYPO3.Eel TYPO3.Flow TYPO3.Fluid TYPO3.Kickstart TYPO3.Welcome Packages Applications

Bug #37363

PersistenceManager->validateObject() ==> to many calls/loops/recursions somewhere in a subcall

Added by Sebastian Wojtowicz about 3 years ago. Updated about 3 years ago.

Status:

Resolved

Start date:

2012-05-21

Priority:

Must have

Due date:

Assigned To:

Karsten Dambekalns

% Done:

100%

Category:

Validation

Target version:

TYPO3 Flow Base Distribution - 1.1 beta 3

PHP Version:

5.3

Complexity:

Has patch:

Affected Flow version:

Git master

Description

In a nutshell¶

In my project I add objects with lots of sub sub objects. They are added in bigger numbers, like often 50, sometimes 100 even up to 1000 elements at once (it's vocabularies).
The problem is, right now it takes already 25 seconds on my local machine to add only 10 elements (with all their sub sub sub... objects of course, but still 25 seconds is way to much).
Currently I need to add 5000 vocabularies. This would take about 33 hours (if it doesn't fail at some point because of missing memory).

Testcase¶

Having no idea what causes it I created a small test-case at https://github.com/Allisone/Wojtowicz.AddALot

There you have a model hierarchy like this:
Elements A,B,C and D, so that As have Bs, Bs have Cs and Cs have Ds.

Via the commandline you can then call one of the following commands.
1. Create x Bs, each having a C, each C having a D
2. Create y As, each having x/y Bs, each having a C, each C having a D

I can see in the xdebug profiler output...
For 1. and x=20: 6480 calls are being made to TYPO3\FLOW3\Error\Result->mergeProperty()
For 2. and x=20 and y=4: 79656 calls are being made to TYPO3\FLOW3\Error\Result->mergeProperty()
(we get 20 Bs, each 5 Bs being added to an A)

Doing a small adjustment so that each B has 2 Cs (instead of one), I can see already 231576 mergeProperty() calls in the 2. command call.

So... I can't say exactly where the bug is, just give you the upper description, hoping that someone finds that bug, either between \TYPO3\FLOW3\Persistence\Doctrine\PersistenceManager\validateObject() and TYPO3\FLOW3\Error\Result->mergeProperty() or at some place where f.e. to many validators where added (or similar). But I think it should be related to validation...

More¶

For more details look at my provided testcase (+README):
https://github.com/Allisone/Wojtowicz.AddALot

Related issues

Associated revisions

Revision 26e4b3fa
Added by Karsten Dambekalns about 3 years ago

[BUGFIX] Avoid validating instances more often than needed

The GenericObjectValidator uses a static variable to store objects being
validated. Objects were still validated over and over because at the
end of isValid() they were detached again.

This change replaces that detach() call with another concept. Now
any validator implementing the new ObjectValidatorInterface can be
given an SplObjectStorage to keep track of instances already
validated.

If you reset that container when it makes sense (before you
actively validate objects) you can avoid validating single instances
more than once in one validation run but still validate at various
points on your logic flow.

If you have implemented your own object validator you should
implement the new ObjectValidatorInterface - the easiest way is
to extend the GenericObjectValidator, if you haven't done that
so far.

Change-Id: I7f437207e27593425b1fbdfc340a442e685a317f
Fixes: #37363
Related: #34154
Releases: 1.1, 1.2

Revision 632685da
Added by Karsten Dambekalns about 3 years ago

[BUGFIX] Avoid validating instances more often than needed

The GenericObjectValidator uses a static variable to store objects being
validated. Objects were still validated over and over because at the
end of isValid() they were detached again.

If you have implemented your own object validator you should
implement the new ObjectValidatorInterface - the easiest way is
to extend the GenericObjectValidator, if you haven't done that
so far.

Change-Id: I7f437207e27593425b1fbdfc340a442e685a317f
Fixes: #37363
Related: #34154
Releases: 1.1, 1.2

Revision fdc462b0
Added by Karsten Dambekalns about 3 years ago

[TASK] Make extending GenericObjectValidator easier

This change moves the check for already validated objects out
of the isValid() method and into validate() instead. Since in
custom validators isValid() is the method to implement, this
makes correct implementation less error-prone.

Change-Id: I16981ff9d2177cb4949d0fc3fcbd815f7c99cbc5
Related: #37363
Releases: 1.1, 1.2

Revision ed3788b3
Added by Karsten Dambekalns about 3 years ago

[TASK] Make extending GenericObjectValidator easier

Change-Id: I16981ff9d2177cb4949d0fc3fcbd815f7c99cbc5
Related: #37363
Releases: 1.1, 1.2

History

#1 Updated by Karsten Dambekalns about 3 years ago

Target version changed from 1.1 beta 2 to 1.1

#2 Updated by Sebastian Wojtowicz about 3 years ago

I really have problems going through all this recursive code, so I made some effort to visualize what is going on...

https://gist.github.com/942e34ef00da89b1f459

You can see the call tree for the first command (20 Bs with 1 Cs with 1 Ds) and for the second command (4 As with 5 Bs with 1 Cs with 1 Ds).

I've ignored calls where no further recursion occurred.

I'm still not able to tell where the problem is, but at least this will keep you from loosing your head while fighting these reoccurring calls.

#3 Updated by Karsten Dambekalns about 3 years ago

Status changed from New to Needs Feedback
Assigned To set to Karsten Dambekalns

Just to make sure: have you tried the changes listed at https://review.typo3.org/#/q/topic:34145-validation-optimisation,n,z - and if not, could you do that?

#4 Updated by Sebastian Wojtowicz about 3 years ago

just found that place in the code myself and wanted to report it here, to hear what you think about the fact that this seems to be called over and over again and that it should not be instancesCurrentlyUnderValidation but some kind of wasValidated...

and now that I see the patch, I realize that I tried it already (ChristianM pointed me to it as well)... but it didn't work before, made no difference. And I tried it twice, even though I was convinced it could not effect my case because I have no validators at all.

Well... obviously I messed up, don't know how, cause this patch is working. It's the right fix. Also checked the profiler output numbers. All good now.
On one hand I could bite my ass that somehow I messed up the first 2 times I tried that patch (how ?). On the other hand, I wouldn't have known now, how to use xdebug, debug with PhpStorm, what is and how to use kCachegrind, and how the validation is done :D
Just hope I didn't wasted your time :( and that I'm better equipped for the next bug I find ;)