This is a quite simple fix, though it makes Drupal pages appear much faster displaying nodes not having different teasers and bodies. check_output() is expensive, and there is no point in executing all the filters on the same text since the output will be the same... This makes pages displaying nodes not having different bodies and teasers display significantly faster (have not measured, but I think this is not a question).

If this is committed, other modules need to be fixed too. Forum.module has a very similar code part for example, which I have complained about before...

CommentFileSizeAuthor
#6 Drupal-fix.check.output.speeds.patch3.79 KBgábor hojtsy
Drupal-increase.node.view.speed.patch719 bytesgábor hojtsy

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Ups, this should have the "patch" status...

Kjartan’s picture

Kjartan commented

PS: One minor coding standard thing, } else { needs a line break after the }.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

As far as I see, this problem does not warrant a new patch, since Dries nearly always edits the diffs... So the patch is still up :) (BTW I'll take care of coding standards better next time).

dries’s picture

dries commented

Committed this patch and modified the code to conform the coding standards. Thanks.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

As I have written, there are more like this (at least one in forum.module), so I'll provide more patches here...

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
StatusFileSize
new Drupal-fix.check.output.speeds.patch3.79 KB

Since this approach was accepted, here is a more general fix, which includes lots of other modules having the teaser and body check_output() calls. I have introduced a new node_check() function, which prepares the teaser and the body.

I don't like the naming of check_output(), so I am not generally friendly with the naming of node_check(), but I have used this name to be consistent with the current naming of check_output()...

This patch will increase the speed of forum pages as well as many other parts of Drupal, where nodes are displayed having only a body (=teaser).

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
dries’s picture

dries commented

How about node_prepare() or node_prepare_content? The name node_check() is not really explanatory IMO.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Well, node_prepare() is fine, you can search&replace in the contents of the patch.

But then it might be a good idea to rename check_output() to prepare_text() or something more appropriate after applying this patch ;) check_output() does not make much sense either, since it mostly runs the filters on the text, and does not do much checking...

dries’s picture

dries commented

Committed to HEAD using node_prepare(). Thanks Goba.

I agree that check_output() is a bad name though it is probably worth noting that it used to be a good name. Renaming check_output() should probably be covered by a seperate issue.

Anonymous’s picture

(not verified) commented