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);
??
| Comment | File | Size | Author |
|---|---|---|---|
| fivestar-noload.patch | 2.46 KB | turadg |
Comments
Comment #1
turadg commentedThis is a simple and perfectly safe patch that decreases load time. I hope someone can commit it.
Comment #2
ericduran commentedI'm moving this to a task.
Also this is a step in the right direction but is still an extra database query thats not needed 90% of the time. By the time we get o the fivestar_form function, we've already touch the node object about 3 times, This should be better.
Comment #3
whiteph commentedWe can no longer support the Drupal 6 version of Fivestar. It is in security maintenance mode only. When the Drupal 8 version of Fivestar is released, the Drupal 6 version will be officially deprecated. However, if a volunteer can be found who is willing to test patches for the Drupal 6 version, we could revert it to normal maintenance. See Help testing Drupal 6 patches.