[#4925] - batch exec queries instead as each as single query
- Closed
- 19 Dec 2015
- Medium
- Build: staging
- # 4925
- Diff
- rdeutz:DbExecuteBatch
- Success continuous-integration/travis-ci The Travis CI build passed Details
User tests: Successful: Unsuccessful:
The reodering in the JTable executes any reordering statement as a single query. If you have a lot of articles in one category it can easily add hunderts of query this results in a large response time and can be result in timeouts.
This is hard to find out, because of the redirect after a save the debug shows only the executed queries from the last request and not from the save request.
This patch implements for the mysqli DB driver a executeBatch command that can handle an array of queries, concats the queries and execute it with the mysqli_multi_query function. This is significant faster, I had on my local env. 1:15 before and 0:02 after the patch. (just watch the videos)
I also did some refactoring because if I did not I had to duplicate code. I don't think there are BC issues because I implemented a new function instead of extending the execute function. I think that is more save because in this area the test coverage is not so good.
How to test this:
I have made a database backup, because you need a DB with 3000 Articles in one single category to see an effect in a local env. If you are on a shared hosting env. you can run into troubles with much less articles. I have seen that in a database von 300 articles in one category. So you can use my DB backup and them simply try to save an article in the category "Overload 1", it might be take some time, so if you timeouts on a low lover you will probably not seeing the result page, try to set "max_execution_time = 240" or zero and make sure you memory_limit is high enough. If you have done that just wait, grab a coffee or watch a star trek episode.
Then add the patch and do it again you should see that there is a significant difference.
Here are the link to the files you might need to test or view what happened.
https://dl.dropboxusercontent.com/u/959364/dbExecuteBatch.sql.zip
https://dl.dropboxusercontent.com/u/959364/beforepatch.mp4
https://dl.dropboxusercontent.com/u/959364/afterpatch.mp4
jissues-bot
- | Labels |
Added:
?
|
||
| Category | ⇒ | SQL |
@brianteeman what env you are using? I have a MacBook Pro with 16GB and the fastest processor one can buy. I used home-brew to install mysql. Did you do time measurement and can tell how long it runs on your system?
It is my Mac book air. Nothing special. I will retest again and record it
On 25 Oct 2014 10:13, "Robert Deutz" notifications@github.com wrote:
@brianteeman https://github.com/brianteeman what env you are using? I
have a MacBook Pro with 16GB and the fastest processor one can buy. I used
home-brew to install mysql. Did you do time measurement and can tell how
long it runs on your system?—
Reply to this email directly or view it on GitHub
#4925 (comment).
I have retested it and recorded it for you
https://dl.dropboxusercontent.com/u/2940199/robertbeforepatch.mp4
https://dl.dropboxusercontent.com/u/2940199/robertafterpatch.mp4
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4925.
Tested it again on my local machine with a different setup, it is much more faster now so the performance boost is on a low level. Anyway it makes sense at least for a crappy host setup and it brings us back the queryBatch method we lost someday on the way from 1.5 to 3.x
Can you please merge in the latest changes. Right now this can not be merged. Also, can you split the debugQuery method into its own PR? I think it is rather difficult to see the right code when those 2 changes are done together.
I did not look into this more deeply, but can't we merge the queries anyway into something more intelligent? Like having one query that simply takes the current field and adds 1 to it? That would make that one query instead of dozens, even if its done in a batch. I can't give a perfect answer here, since I would have to look into my SQL book, too, to get this right, but that sounds better than having 100 queries for 100 articles...
Last but not least, I'm not sure that it will work the way you wrote. You have the $query object, clear that object, insert the new data, hand that object over to an array and then clear it again in the next itteration. Are you sure that the array stores this by value and not by reference? Because in the later case, all queries would be the same.
after applying the patch, i cant create a new article. it chrashes with Fatal error: Call to undefined method JTableContent::setColumnAlias() in C:\xampp\htdocs\bugtesting\libraries\legacy\table\content.php
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4925.
| Status | Pending | ⇒ | Information Required |
@rdeutz can you have a look into the unsuccessful testes here? Maybe this can by fixed with a resync to staging?
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4925.
@rdeutz is this still valid or shall we close it
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4925.
still valid but it seems only an issue for some people, closing it.
| Status | Information Required | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-12-19 13:23:11 |
| Closed_By | ⇒ | rdeutz |
@test
Installed the db and created an article in the category overload1
It was slow but nowhere near as slow as in the video
Applied the patch
It was quicker but nowhere near as fast as in the video
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4925.