Poor performance of _forum_update_forum_index() when passed a non-forum node [#1715402]

Forked from #145353: Forums performance improvements.

_forum_update_forum_index() performs the following expensive query on every comment insert, update, publish, or delete, regardless of whether the comment is attached to a forum topic node. In my context, this has a serious impact on migration rollback of comments.

    $last_reply = db_query_range('SELECT cid, name, created, uid FROM {comment} WHERE nid = :nid AND status = :status ORDER BY cid DESC', 0, 1, array(
      ':nid' => $nid,
      ':status' => COMMENT_PUBLISHED,
    ))->fetchObject();

This is to setup a db_update on forum_index, which will do nothing anyway if the parent node wasn't a forum topic. So, we want to avoid this query unless we know the node is already in forum_index. I added an inner join to forum_index on the initial comment query in the function, and in a very rough informal test the average time of forum_comment_delete() goes from 54ms to 1ms (this is with 73k comments in the comment table).

I'll freshen my D8 checkout and submit a patch.

Comment	File	Size	Author
#15	forum-index_update_performance_d7-1715402-15.patch	648 bytes	mikeryan
#15
#12	forum-index_update_performance_d7-1715402-12.patch	645 bytes	mikeryan
#12
#1	forum-index_update_performance-1715402-1.patch	666 bytes	mikeryan
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan commented 6 August 2012 at 15:37

Status:

Active

» Needs review

File	Size
forum-index_update_performance-1715402-1.patch	666 bytes

Comment #2

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan commented 6 August 2012 at 16:13

Comment #3

moshe weitzman CreditAttribution: moshe weitzman commented 6 August 2012 at 18:13

Status:

Needs review

» Reviewed & tested by the community

Makes sense, and code looks good.

Comment #4

chx CreditAttribution: chx commented 6 August 2012 at 19:49

*blink* if that query is expensive , is it expensive because it's not indexed? If it is not, why doesn't forum add an index on comment table? If it's indexed how it can be expensive?

Comment #5

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan commented 6 August 2012 at 20:05

EXPLAIN shows it's using the comment_num_new index - the issue is the sort ("Using where; Using filesort").

At any rate, this query is always going to be more expensive than the join I added to the initial query, and it's an unnecessary query for any non-forum nodes, so this is a big win when dealing with 176k blog comments...

Comment #6

chx CreditAttribution: chx commented 7 August 2012 at 10:06

After chatting with catch about this it seems #5 is a mis-post and I misunderstood what's going on. While the count query is likely to be near-instant as I suspected the point is not the query in the patch but not running the other two controlled by this count query. So yes, good to go.

Comment #7

chx CreditAttribution: chx commented 7 August 2012 at 10:13

Also, further talking -- shouldnt we change the ORDER in the $last_reply query to be ORDER BY created, cid so the comment_num_new index (nid,status,created,cid,thread) can be used fully? Can be a followup or can be done here. Won't that speed up further?

Comment #8

effulgentsia CreditAttribution: effulgentsia commented 7 August 2012 at 20:01

Re #7: that might speed up comment updates on long forum threads, but seems like a separate issue. #1 looks good for this issue, which is to prevent an unnecessary db_query_range() when dealing with a non-forum node.