Performance: Speed up list_forum and list_topic by > 90% (Patch included)
|Priority:||Must have||Due date:|
|Target version:||0.1.8||Estimated time:||4.00 hours|
The 0.1.7 (and current trunk) version of tx_mmforum_pi1->getunreadposts() is highly inefficient both in its implementation and its use. A very simple rewrite and two new index values on tx_mmforum_posts and tx_mmforum_postsread give a performance gain of more than 90% for showing the forum list. A patch which performs these changes based on a mm_forum 0.1.7 version is attached to this report.
Requesting the Mittwald Typo3 Forum at http://www.typo3.net/forum/ without being logged in on that side currently takes me takes ~3 seconds.
Visiting the same page after I've logged in took me > 30 seconds this morning after a couple of weeks of not visiting the forum.
The reason behind this is the way the forums and threads are marked unread which is tremendously inefficient.
The current implementation of tx_mmforum_pi1->getunreadposts() performs the following steps:
1) Retrieve all topic_ids (read) from tx_mmforum_postreads which the user has ever read in his entire life.
2) Retrieve all topic_ids (new) which have been created since the user has last pressed the "mark all read" link (that value might also be set when the user logs out)
3) for all (new) topics, check if the ID is in the array of (read) ids. If not, add it to the result of unread post ids.
I am using a development server which has close to 60,000 posts in 8,500 threads devided over 55 forums (which are filtered by their language version on the same pid).
After having found XHProf (a nice PHP profiler developed for facebook), the result shocked me.
Displaying the forum list of 20 Forums for the english version had this result:
Total Incl. Wall Time (microsec): 12,347,336 microsecs Total Incl. CPU (microsecs): 11,904,744 microsecs
12.4 seconds render time and 11.9s of that only CPU...
The top two scoring functions were:
Function Name Calls Calls% Incl. Wall Time (microsec) IWall% Excl. Wall Time (microsec) EWall% tx_mmforum_pi1::getunreadposts 1 0.0% 11,194,209 90.7% 8,921,498 72.3% count 2,077,275 97.4% 2,227,976 18.0% 2,227,976 18.0%
This means, that 1 Call to getunreadposts() was responsible for 90.7% of the scripts runtime and spend 72.3% of the time in the method itself (exclusively without calling other methods). And more than that, someone (who turned out to be getunreadposts() called count() more than 2 million(!!) times...
I don't want to got too much into the details of this function but I've redesiged it this way:
The getunreadposts() method has been effectivley relocated to MySQL selecting the unread posts with a fairly simple query on tx_mmforum_posts which uses a subquery on tx_mmforum_postsread. Both tables need a new index to make use of the benefit:
In both cases the indices can omit the columns on the right hand side, but when they are kept like this, performance is, of course, best.
Furthermore, I didn't like that, regardless of what you are displaying on a page (the forums, a certain forum, posts which are solved, whatever...), the pi1 selects all unread topic ids... So I've added a filter parameter and restructured list_forum and list_topic to only ask for those unread values they are really interested in.
The forum page does not need the ids of the topics which are unread, it needs to know if there is any unread topic in a forum. And more than that, it is only interested in those forums it wants to display on that page.
Likewise, a list of 20 posts does not need to know the ids of all unread posts to render the topic icon, it needs to know if those 20 particular posts have been read or not.
The method now supports this functionality and here is the result:
Total Incl. Wall Time (microsec): 1,600,213 microsecs
Total Incl. CPU (microsecs): 1,084,068 microsecs
The rendering time for the forum list dropped from 12.3 seconds to 1.6 seconds.
The time spend in getunreadposts() (and its child functions) dropped from 11.2 seconds to 440ms.
This is a speed increase of 96%.
In 0.1.7, the different list_* functions all use this method. The result of getunreadposts() is actually buffered in getForumIcon and getTopicIcon but nevertheless the method is always called twice when those functions are used. In my patch, I've streamlined this a bit for list_forum and list_topic but not for the other views since I do not use them.
If there are any questions, feel free to contact me. (I am German.)
The changes are really fairly simple once you found the bottleneck and I think the effect justifies that I've specified target version 0.1.8.
Updated by Martin Helmich over 3 years ago
- Status changed from New to Accepted
first of all, thanks a lot for all the work you have commited to this issue (concerning issues #4693 and #4694 as well). After your devastating verdict, I decided to take a closer look at the
getunreadposts function myself and I have to admit that the current implementation is suboptimal at best (The enormous amount of
count() calls by the way is a result of an adventurous loop construction, with
count being called in every iteration of a for-loop over all read posts nested inside a loop over all total posts. So, the total amount of
count-calls is in fact the product of the total amount of posts and the amount of posts viewed by the current user. Really ugly.)
Although the performance gain is most likely dependent of the amount of data you are working with, getting bigger the more data you need to process, this is a huge step regarding the performance of the mm_forum extension. I have applied your patch to the current trunk (I had some difficulties since your patch seemed to be against the 0.1.7 version) and everythings tests allright.
Since all these improvements are easy to implement (regarding your other bug reports as well), while bringing great improvements at the same time, I have decided that these patches should indeed make it into the upcoming 0.1.8 version. Again, thanks again for your work.