There's only a couple of variables in drupal_lookup_path() which really need to use drupal_static(). This saves about 3% of page execution time and 6,000 function calls on a node with 300 comments.

CommentFileSizeAuthor
#11 path_lookup.patch7.69 KBcatch
#10 path_lookup_2.patch7.4 KBcatch
#8 head.png386.25 KBcatch
#8 patch.png415.79 KBcatch
#7 path_lookup.patch7.4 KBcatch
#5 path_lookup.patch7.03 KBcatch
#3 path_lookup.patch1.38 KBcatch
patch.png181.6 KBcatch
head.png187.18 KBcatch
path_lookup.patch1.19 KBcatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Active » Needs review
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Reviewed & tested by the community

nice. very simple patch, RTBC in my opinion.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
path_lookup.patch1.38 KB

Fixed up code comments a bit after feedback from pwolanin in irc, no functional change.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

Why not having a single $cache variable:

$cache = drupal_static(__FUNCTION__, array(
  'map' => array(),
  'no_src' => array(),
  'system_paths' => array(),
  'no_alias' => array(),
  'first_call' => TRUE
);
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs review
FileSize
path_lookup.patch7.03 KB

Untested version of Damien's suggestion, which I like a lot better.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review
FileSize
path_lookup.patch7.4 KB

Fixed the test fail.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
patch.png415.79 KB
head.png386.25 KB

Halves time spent in self, takes about 1/3rd off total time too for drupal_function_exists() on the default front page.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented 
+          $cache['map'][$path_language] = db_query("SELECT src, dst FROM {url_alias} WHERE src IN(:system) AND language IN(:language, '') ORDER BY language ASC, pid ASC", array(

Missing spaces after IN.

Aside from that, this looks good to go.

This review is powered by Dreditor

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
path_lookup_2.patch7.4 KB

Fixed the whitespace.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
path_lookup.patch7.69 KB

Fixed some more.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs documentation

Hm! Not only does this speed up the page execution time, the resulting code seems much easier to read, as well.

Committed to HEAD.

Seems like we also change the documentation at http://drupal.org/update/modules/6/7#static_variable_api to removes the recommendation for using the colon-suffix in favour of this array-based approach.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Fixed

Updated, thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Performance, -Needs documentation

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

Issue tags: +Performance, +Needs documentation

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.