#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 romanb):
Replying to [comment:10 colnector]:
> I truly apologize if what I wrote seemed like an insult. The word
'imagine' was wrongly chosen and I guess it should have been more around
'consider it to be'. My intentions were good.
I did not take it as an insult. I'm in no way married to my/our code. I
always just want the best for the software i'm developing, hence when I
say i would be happy if someone has a better hydration mechanism with the
same functionality, i mean it.
>
> 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.
>
> * when to calculate expressions and when to save their results so
they're not calculated in the loop
> * when to do a function call and when not to
> * which function calls to use (static/dynamic)
> * whether or not to pass more variables to a function and whether to
pass by reference
Yes, we are always (trying to) consider all of these points.
> * 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.
>
> I've currently looked at the result set parsing loop and it seems to me
that many things should have been calculated outside the loop and are
calculated inside (thus for every row).
>
> Line 131:
> {{{
> $listeners[$componentName]->preHydrate($event);
> }}}
> 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).
> 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.
>
> 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;
> 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.
>
> These are all very minor things when programming in general, but should
be considered in the hydration loop specifically.
>
> Thanks
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.
--
Ticket URL: <http://trac.phpdoctrine.org/ticket/1510#comment:11>
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
-~----------~----~----~----~------~----~------~--~---
