The path_cache patch includes the following change:

-    $count = db_result(db_query('SELECT COUNT(pid) FROM {url_alias}'));
+    $count = db_result(db_query('SELECT COUNT(*) FROM {url_alias}'));

This appears to be a MyISAM-specific optimization. On websites running with InnoDB or PostgreSQL this query can take a long time during a critical code path.

The attached version of the patch changes the above to solve this performance hit on these other database engines:

-    $count = db_result(db_query('SELECT COUNT(*) FROM {url_alias}'));
+    $count = (int)db_result(db_query('SELECT pid FROM {url_alias} LIMIT 1'));

This works as pids are non-zero, so it really doesn't matter which one is returned. If none exists, $count gets set to 0.

CommentFileSizeAuthor
path_cache.patch6.92 KBjeremy

Comments

firebus’s picture

firebus commented
Assigned: Unassigned » firebus

absolutely! thanks for the catch!

m3avrck’s picture

m3avrck commented

Indeed, this sped up the query for MothersClick around 900%, great catch!

firebus’s picture

firebus commented
Status: Needs review » Fixed

this is a pretty minor change with big impact for INNODB/PSQL users, and no one has made any complaints, so i've committed it.

since path cache is currently broken, i've updated the files in the DRUPAL-5-6 directory

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

ramuchidambaram’s picture

ramuchidambaram commented

Thanks for the patch, it gives a good performance benefit.
if (!isset($count)) {
//$count = db_result(db_query('SELECT COUNT(pid) FROM {url_alias}'));
$count = (int)db_result(db_query('SELECT pid FROM {url_alias} LIMIT 1'));
}
Here the $count variable is used to indicate that a url alias has been created. Can't we use $count =1 or session variable like $_SESSION['count'] instead of running the query "(int)db_result(db_query('SELECT pid FROM {url_alias} LIMIT 1'));" ?.
In a site with a large and growing node table where we are in any way sure that there are alias, do we really require to have this query on every page load? This is one of the more time consuming queries as it uses the node table.

ramuchidambaram’s picture

ramuchidambaram commented
Status: Closed (fixed) » Needs review