Ticket #412 (new enhancement)

Opened 4 years ago

Last modified 4 years ago

ORM Performance Issues

Reported by: tronics Owned by: somebody
Priority: must Milestone: 1.2
Component: Other Version: 1.0.0 Stable
Keywords: Cc:

Description

After seeing performance issues in real life as well as profiling, we should address to this.


Simple Testcase (akrohn):
Generated around 4000 rows for the projects table of the sample database and was curious what the profiler would say. (used QSqlServer2005Database)
For a Project::LoadAll?() it got me this for a total time of 21.569 ms.


Qcubed ORM is generating a lot of objects when doing eg. a LoadAll?()
This has also proven to hamper performance on other ORMs like Doctrine.

In this thread the problem is described in detail:
 http://qcu.be/content/qlistbox-holds-all-objects-making-it-slow-certain-situations-possible-performance-improvemen

We should enhance the performance.

There are several ideas:

* Using Arrays instead of Objects will definitly increase the perfromance by a very siginificant factor (with doctrine it is like 1:4.5) (tronics)

* Reduce calls to GetColumn?() and use InstantiateDbRow?() instead (akrohn)

Off topic: * Make ORM agnostic (see different ticket: http://trac.qcu.be/projects/qcubed/ticket/402) (tronics)

Change History

Changed 4 years ago by akrohn

Hey, I've got to add, that it needed 21 seconds only because the xdebug-profiler was enabled, without it needs only around 5 seconds.

Avoiding GetColumn?() calls in InstantiateDbRow() did get me to 3.2 seconds for a 6 column table. So this is a lot for such a little change.

Related to this is avoiding of "Select *" discussion, that should enhance performance too.

Changed 4 years ago by tronics

Thanks for that comment.

Changed 4 years ago by alex94040

Folks, do you want to prototype the ideas that you described? A patch would be awesome. Separate patches for each of the separate ideas would be best (=easiest to review).

Changed 4 years ago by VexedPanda

Note that GetColumn? itself probably isn't the problem. All it really is is a wrapper for QType::Cast. I imagine Cast is the performance hog.

Changed 4 years ago by akrohn

Thought that myself and avoided QType::Cast in GetColumn?(), but this resulted in not so much performance gains as avoiding GetColumn?(). For many types like string or integer GetColumn?() and QType:Cast calls are not really necessary. Instead accessing directly theColumnArray? could be a first step for better performance.

Changed 4 years ago by tronics

Please check out this:

Fernando Lord writes on Qcodo forum to the same matter:
"

No, of course it doesn't solve cluttering the client with HTML options if a ListBox? is filled with a large query result.

But the suggestion in the third message of issue #42 (the ability to query concrete field values instead of entire rows) does solve the performance problem of InstantiateDbResult?() necessarily creating an active record object for every row retrieved from the database. And in fact a good example of it as a good improvement is using that feature to fill ListBoxes?, as they may only need to retrieve the record identifier and a name, not the full row.
"
this is the link
 http://www.qcodo.com/issues/view.php/42

Do you think this is also an improvement?

Changed 4 years ago by VexedPanda

Yes, this is definitely an improvement as well, and we have ticket #79 for that. Though I'm unsure specifically what level of improvement to expect here.

Changed 4 years ago by VexedPanda

From IRC, rts_ suggests we try switching away from a pre-load scenario, and instead use an iterator, so that the load is distributed, and the user begins seeing the results sooner.

This would make apps appear more responsive, but would not improve overall performance.

Given the huge amount of changes this would entail, I'm against something that's only benefit is responsiveness.

Changed 4 years ago by VexedPanda

There's some really good discussion around how to treat this in  http://www.qcodo.com/issues/view.php/42/

Note: See TracTickets for help on using tickets.