On a site with a large number of nodes, the query to generate the counts for each forum is quite expensive. For example from a 6.x install, devel indicates the following on /forum on my local instance of a site with X nodes:
Executed 244 queries in 1726.65 milliseconds. Queries taking longer than 1000 ms and queries executed more than once, are highlighted. Page execution time was 2009.72 ms.
1321.66
1
advanced_forum_get_forums
SELECT DISTINCT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count FROM node n INNER JOIN node_comment_statistics l ON n.nid = l.nid INNER JOIN term_node r ON n.vid = r.vid WHERE n.status = 1 GROUP BY r.tid
That's 77% of the total time spent in the DB for one query. For reference, the EXPLAIN:
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE n ref PRIMARY,vid,node_status_type node_status_type 4 const 55652 Using temporary; Using filesort
1 SIMPLE l eq_ref PRIMARY PRIMARY 4 rupal_blizzard.n.nid 1
1 SIMPLE r ref vid vid 4 drupal_blizzard.n.vid 2 Using index
Adding n.type = 'forum'
to the query gives us the same forum count results, but a dramatically faster query by reducing the number of rows from {node} - only 30ms on my local.
Patches forthcoming.
Comment | File | Size | Author |
---|---|---|---|
#6 | 1668740.6-query-node-type-6.x-2.x.patch | 1.85 KB | mcdruid |
#5 | 1668740.5-query-node-type-6.x-2.x.patch | 1.88 KB | mcdruid |
#4 | 1668740.4-query-node-type-6.x-2.x.patch | 1.82 KB | mcdruid |
#2 | af7_1668740_1.patch | 1.3 KB | troky |
#1 | 1668740.1-query-node-type-6.x-1.x.patch | 2.41 KB | deviantintegral |
Comments
Comment #1
deviantintegral CreditAttribution: deviantintegral commentedHere they are, for all three branches. I have not tested the 6.x-2.x patch.
Comment #2
troky CreditAttribution: troky commentedInteresting.
7.x doesn't need array_walk(ing)... everything else looks good.
Comment #3
troky CreditAttribution: troky commentedCommitted to 7.x.
Comment #4
mcdruidThanks for the patches deviantintegral, and sorry it's taken me a long time to look at them in detail.
I've concentrated on the 6.x-2.x patch, and will look at the 1.x branch later; I think the code is going to be more-or-less the same.
The advanced_forum_get_forums function is "is copied from the forum module and adapted". The original code was probably a good candidate for a couple of comments to explain what's going on :)
So a couple of problems with the patch (which you did say you hadn't tested); you added the AND n.type IN condition to one of the multiple versions of $sql, but it looks like in many cases this version gets thrown away. If db_rewrite_sql doesn't change the first $sql, it gets replaced by a slightly different query. In some basic tests with permissions set a couple of different ways, and accessing the forum list as different users, I found db_rewrite_sql was not changing $sql, so your addition was always being discarded. There must be circumstances under which the query gets rewritten, but I didn't find one.
There was also a stray 'AND', and I was keen to avoid the addition of the array_walk callback if possible; if we were being really paranoid we could ask whether we can trust the $node_types enough to put them straight into a query.
So here's my rewrite of the patch - which seems to work okay. I've tested it with 0, 1, and 2 $node_types.
Would appreciate it if people could test this - particularly anyone using forum_access or other types of access control. It would also be good to hear whether it does improve performance.
Comment #5
mcdruidTweak to avoid an undefined variable - this had been removed in a tidy-up.
Comment #6
mcdruidAnother tweak which really should avoid the undefined variable this time.
Comment #7
mcdruidCommitted to 6.x-2.x
http://drupalcode.org/project/advanced_forum.git/commit/7425a85
Thanks!