Optimization for fivestar_form(), don't node_load

turadg - July 31, 2009 - 20:47
Project:Fivestar
Version:6.x-1.18
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review
Description

I was trying to cut down the load time for a page I have listing hundreds of nodes with ratings. I saw in the log that each node was being loaded unnecessarily, which was bad because some of their load hooks are pretty slow.

I tracked the unnecessary load down to fivestar_form() in this block:

if ($content_type == 'node') {
if (is_numeric($content_id)) {
$node = node_load($content_id);
}
else {
return array();
}
}

Attached is a patch that cuts out node_load() and instead does a single db query to look up the type, which is the only part of the node that is used in the function. It yield the following improvement:

# loading whole node
Executed 1866 queries in 1066 milliseconds. Page execution time 11134 milliseconds.
# loading just type
Executed 867 queries in 594 milliseconds. Page execution time was 8074.83 ms.

Incidentally, what happens if $content_type != 'node'? The node (or type) value doesn't get set then, but the subsequent code relies on it. Perhaps the logic should be like this:

if (! ($content_type == 'node' AND is_numeric($content_id)) {
return array();
}
$node = node_load($content_id);

??

AttachmentSize
fivestar-noload.patch2.46 KB

#1

turadg - August 7, 2009 - 23:22
Status:active» needs review

This is a simple and perfectly safe patch that decreases load time. I hope someone can commit it.

 
 

Drupal is a registered trademark of Dries Buytaert.