The SELECT src FROM path lookup queries weren't getting statically cached leading to 8 extra (duplicated) queries on a typical page. This applies to 4.7.* as well (and maybe 4.6... don't know).

To test this, turn on the devel module, path module, and make sure you have at least one aliased URL. Look in your query log for a section like this:

0.17	0	drupal_lookup_path	SELECT src FROM url_alias WHERE dst = 'node'
0.21	0	drupal_lookup_path	SELECT src FROM url_alias WHERE dst = 'node'
0.19	0	drupal_lookup_path	SELECT src FROM url_alias WHERE dst = 'node'
0.17	0	drupal_lookup_path	SELECT src FROM url_alias WHERE dst = 'node'
0.23	0	drupal_lookup_path	SELECT src FROM url_alias WHERE dst = 'node'

Apply the patch and that query should only get executed once. Next issue.... why doesn't the devel module report dupe queries anymore?

CommentFileSizeAuthor
#7 path_5.patch496 bytesrobertdouglass
#3 path_4.patch1.57 KBrobertdouglass
path_3.patch500 bytesrobertdouglass

Comments

dries’s picture

dries commented

Good catch. Might be worth a code comment.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks.

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new path_4.patch1.57 KB

Added comments to all the lookup logic and removed an if block from the second part. This is a bigger change, closer review needed.

robertdouglass’s picture

robertdouglass commented
Status: Fixed » Needs review
robertdouglass’s picture

robertdouglass commented
Version: x.y.z » 4.7.3

changing version so that this can get backported.

killes@www.drop.org’s picture

killes@www.drop.org commented
Version: 4.7.3 » x.y.z

backported the original patch, moving back to HEAD

robertdouglass’s picture

robertdouglass commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new path_5.patch496 bytes

We introduced a bug. The thing that should be cached is not the $path, but rather FALSE. If you're looking for the source for an alias and you don't find one, then FALSE is the correct return value. So yes, there is an optimization that can be made in the caching of results, but the first solution wasn't the right way.

This will need to be backported again.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

killes@www.drop.org’s picture

killes@www.drop.org commented

backported

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
RayZ’s picture

RayZ commented

I believe this fix caused another bug http://drupal.org/node/89947.

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Rob - I'm not sure I follow your logic on the FALSE thing... Surely if you find yout path in that array you'd return true or the path... Maybe I'm mis-understanding it...