#1510: Time is wasted in Hydration
------------------------------+---------------------------------------------
Reporter: colnector | Owner: romanb
Type: defect | Status: assigned
Priority: critical | Milestone: 1.0.3
Component: Query/Hydration | Version: 1.0.2
Resolution: | Keywords:
Has_test: 0 | Mystatus: Pending Core Response
Has_patch: 0 |
------------------------------+---------------------------------------------
Comment (by colnector):
Replying to [comment:11 romanb]:
> > What I'm trying to say it that since the hydration algo is so critical
in Doctrine, every bit of it has to be meticulously scrutinized. By every
bit I mean even things like (and as I'm not proficient with the code
itself these are general):
>
> I agree but the best way to optimize things is by proper profiling.
Chaning or obfuscating code just of out of the assumption it improves
performance somehow doesnt make sense.
Profiling is a very important tool but code inspection for itself should
not be neglected as sometimes you cover only some of the scenarios with
the profiler.
> > * using PHP string with single comma ' instead of double "
>
> This is a bogus optimization imho. From all i know there really is no
difference between these, at least not since some late php4 versions.
AFAIK $s = "This is number $number."; is still slower than $s = 'This is
number '.$number.'.'; but perhaps it's been optimized.
> > What if there are no listeners? Are we doing an extra function call?
>
> Yes, listeners add a bit of overhead. How much that is can be seen with
a profiler (i.e. xdebug). Since listeners within hydration are bit of a
hacky thing anyway *and* add an impact to hydration even though the
majority does not use them, they have been removed in trunk (2.0) (apart
from that the event listener system for 2.0 will be more performant by
design).
It'll be helpful if for 1.1 the check will be outside the loop. Currently
1.1 does have performance issues.
> > Line 176:
> > {{{
> > // It would be nice if this could be moved to the query parser but I
could not find a good place to implement it
> > if ( ! isset($map['parent'])) {
> > }}}
> > This is in the loop...
>
> I will look into this.
Thanks, let me know if I should open another ticket for it.
> > Line 196:
> > {{{
> > if ( ! $relation->isOneToOne() && $driver->initRelated($prev[$parent],
$relationAlias)) {
> > }}}
> > Can't the relation type be determined outside this inner loop?
>
> I will look into this, too but i doubt it will make any difference.
isOneToOne() is just a return true/false;
But it's a dynamic class function call and not a simple variable which
makes a difference in a loop.
> > Also, perhaps consider not using _gatherRowData() and
_setLastElement() as functions but place their code in the loop. It's a
shame PHP doesn't have inline functions but perhaps sacrifying a bit of
readability would help performance.
>
> This does bring no noticeable difference in performance and is not worth
the code obfuscation. The main hydration method is still too long and too
deeply nested. These 2 parts have been extracted on purpose. It is more
important to have code that is easier to maintain and debug, especially in
this case.
>
As I've seen _gatherRowData() is called only once so it wouldn't really
obfuscate the code including it inside the function.
> As i said, the best way to look for optimizations is with a profiler. If
profiling reveals any other things that can be improved let us know. We
(I) do profiling sessions on a regular basis. Your help is appreciated. I
guess the reason noone spotted this is_array issue yet is that noone dealt
with really large result sets yet. Thanks for that! I will try to fix this
ticket soon. Jons patch looks pretty decent, pulling this into the driver
code is a good idea.
>
Indeed a profiler is very useful but it has its limitations as the one you
write about previously:
> What is strange though is that xdebug doesnt seem to be able to figure
out where exactly the time is spent in _setLastElement.
Thanks
--
Ticket URL: <http://trac.phpdoctrine.org/ticket/1510#comment:12>
Doctrine <http://www.phpdoctrine.org>
PHP Doctrine Object Relational Mapper
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"doctrine-svn" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.co.uk/group/doctrine-svn?hl=en-GB
-~----------~----~----~----~------~----~------~--~---