Performance improvement to DboSource::_mergeHasMany()

[David Gallagher]

I have been working on a performance improvement to
DboSource::_mergeHasMany() and I wanted to get some input from other
developers so that maybe this can be implemented soon in to CakePHP. I
had actually hoped to get this ready and tested before the next
release of 2.2 but I haven't had time.

I know in my own application, retrieving a large amount of data, that
this change has improved performance significantly. I am not sure yet
how affects performance for smaller data sets.

You can find my changes here:
https://github.com/thegallagher/cakephp/compare/master...faster-mergeHasMany

Does anyone have any concerns with these changes that I may be able to
resolve before I submit a pull request?

[José Lorenzo Rodríguez]

Sorry, but your changes broke nearly all tests related to models :(

[David Gallagher]

Sorry Jose I didn't test properly. It was failing testRealQueries test. I didn't notice any other failures though. Perhaps I did something wrong when testing?

Try these changes https://github.com/thegallagher/cakephp/compare/2.1...faster-mergeHasMany

Does this make sense what I am trying to do? When I get a chance I will post some performance results.

[Ceeram]

You can enable travis for your fork and it will run AllTests so you can easily how your changes affect the tests on different setup (phpversions, databases)

Op donderdag 24 mei 2012 05:53:56 UTC+2 schreef David Gallagher het volgende:

[mark_story]

I think the direction is right, but it looks like multiple associations with overlapping keys are going to cause issues as on line 1321 you make $key without any scoping to which association it is.  I think that might be the cause of the failing tests José was referring to.

-Mark

[David Gallagher]

Ok I have worked out what was wrong now all tests are passing. Thanks for the tip on Travis @Ceeram.

Now I just need to do some performance testing to make sure it actually is faster and doesn't use too memory.