Bug tracker

This ticket has been moved to our new tracker. Open Ticket PHPBB3-7921 now.

get_username_string() performance (fix completed in vcs)

Have been playing with xdebug and Kcachegrind a bit and discovered to my horror that get_username_string() accounted for 27% of a simple viewforum.php pageview. Even worse results on an index page.

Have added some static luv and moved some return statements up. With those changes get_username_string() uses 2% instead of 27%

Currently fairly untested code, it's more like a proof of concept at the moment :)

Patch has been attached.

Ps. small bug in the patch, one of the last lines should be '$mode == 'profile', not '$mode != 'no_profile')

Comments / History

Edited ticket

Action performed by BartVB (Consultant) on Nov 23rd 2008, 17:34

Posted by naderman (Development Team Leader) on Nov 23rd 2008, 17:37

The problem here is that this makes the 3rd party integration problem append_sid for urls even worse. Because the u parameter is attached to the url after append_sid. We really need to decide whether we want to use append_sid for all urls.

Edited ticket

Action performed by BartVB (Consultant) on Nov 23rd 2008, 17:38

Posted by BartVB (Consultant) on Nov 23rd 2008, 17:41

The cache for the base URL is there mostly because append_sid() is relatively heavy. On viewtopic.php it uses 25% of total processing time.

If append_sid() is faster the base URL cache can probably be dropped. I really like append_sid(), also because it can be abused to munge URLs :D

An alternative solution is using {USER_ID} in the call to append_sid and doing a str_replace.

Edited post #124125

Action performed by BartVB (Consultant) on Nov 23rd 2008, 17:42

Edited ticket

Action performed by BartVB (Consultant) on Nov 23rd 2008, 17:54

Edited ticket

Action performed by BartVB (Consultant) on Nov 23rd 2008, 17:56

Edited post #124125

Action performed by BartVB (Consultant) on Nov 23rd 2008, 20:02

Edited ticket

Action performed by BartVB (Consultant) on Nov 24th 2008, 08:28

Edited ticket

Action performed by BartVB (Consultant) on Nov 24th 2008, 08:41

Posted by Acyd Burn (Server Manager) on Nov 30th 2008, 17:22

Let me update the function with your append_sid() suggestion...

Posted by Acyd Burn (Server Manager) on Nov 30th 2008, 17:23

Code: Select all
function get_username_string($mode, $user_id, $username, $username_colour = '', $guest_username = false, $custom_profile_url = false)
{
   global $phpbb_root_path, $phpEx, $user, $auth;
   static $_profile_cache;
   static $_base_profile_url;

   if (isset($_profile_cache[$user_id][$mode]))
   {
      return $_profile_cache[$user_id][$mode];
   }

   $username_colour = ($username_colour) ? '#' . $username_colour : '';

   if ($guest_username === false)
   {
      $username = ($username) ? $username : $user->lang['GUEST'];
   }
   else
   {
      $username = ($user_id && $user_id != ANONYMOUS) ? $username : ((!empty($guest_username)) ? $guest_username : $user->lang['GUEST']);
   }

   // Build cache for all modes
   $_profile_cache[$user_id]['colour'] = $username_colour;
   $_profile_cache[$user_id]['username'] = $username;

   // No profile url
   $tpl = (!$username_colour) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>';
   $_profile_cache[$user_id]['no_profile'] = str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($username_colour, $username), $tpl);

   // Profile url - only show if not anonymous and permission to view profile if registered user
   // For anonymous the link leads to a login page.
   if ($user_id && $user_id != ANONYMOUS && ($user->data['user_id'] == ANONYMOUS || $auth->acl_get('u_viewprofile')))
   {
      if (empty($_base_profile_url))
      {
         $_base_profile_url = append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile&amp;u={USER_ID}');
      }

      $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&amp;u=' . (int) $user_id : str_replace('={USER_ID}', '=' . (int) $user_id, $_base_profile_url);
      $tpl = (!$username_colour) ? '<a href="{PROFILE_URL}">{USERNAME}</a>' : '<a href="{PROFILE_URL}" style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</a>';
      $_profile_cache[$user_id]['full'] = str_replace(array('{PROFILE_URL}', '{USERNAME_COLOUR}', '{USERNAME}'), array($profile_url, $username_colour, $username), $tpl);
   }
   else
   {
      $_profile_cache[$user_id]['full'] = $_profile_cache[$user_id]['no_profile'];
      $profile_url = '';
   }

   // Use the profile url from above
   $_profile_cache[$user_id]['profile'] = $profile_url;

   return $_profile_cache[$user_id][$mode];
}

Assigned ticket to user "Acyd Burn"

Action performed by Acyd Burn (Server Manager) on Nov 30th 2008, 17:23

Posted by Acyd Burn (Server Manager) on Dec 2nd 2008, 13:20

Adjusted function:

Code: Select all
function get_username_string($mode, $user_id, $username, $username_colour = '', $guest_username = false, $custom_profile_url = false)
{
   static $_profile_cache;
   static $_base_profile_url;

   $cache_key = $user_id . (string) $guest_username;
   $profile = (isset($_profile_cache[$cache_key][$mode])) ? &$_profile_cache[$cache_key][$mode] : false;

   if ($profile !== false)
   {
      // If the mode is 'no_profile', we simply construct the TPL code due to calls to this mode being very very rare
      if ($mode == 'no_profile')
      {
         $tpl = (!$profile['colour']) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>'
         return str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($profile['colour'], $profile['username']), $tpl);
      }

      return $profile[$mode];
   }

   global $phpbb_root_path, $phpEx, $user, $auth;

   $username_colour = ($username_colour) ? '#' . $username_colour : '';

   if ($guest_username === false)
   {
      $username = ($username) ? $username : $user->lang['GUEST'];
   }
   else
   {
      $username = ($user_id && $user_id != ANONYMOUS) ? $username : ((!empty($guest_username)) ? $guest_username : $user->lang['GUEST']);
   }

   // Build cache for all modes
   $_profile_cache[$cache_key]['colour'] = $username_colour;
   $_profile_cache[$cache_key]['username'] = $username;
   $_profile_cache[$cache_key]['no_profile'] = true;

   // Profile url - only show if not anonymous and permission to view profile if registered user
   // For anonymous the link leads to a login page.
   if ($user_id && $user_id != ANONYMOUS && ($user->data['user_id'] == ANONYMOUS || $auth->acl_get('u_viewprofile')))
   {
      if (empty($_base_profile_url))
      {
         $_base_profile_url = append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile&amp;u={USER_ID}');
      }

      $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&amp;u=' . (int) $user_id : str_replace('={USER_ID}', '=' . (int) $user_id, $_base_profile_url);
      $tpl = (!$username_colour) ? '<a href="{PROFILE_URL}">{USERNAME}</a>' : '<a href="{PROFILE_URL}" style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</a>';
      $_profile_cache[$cache_key]['full'] = str_replace(array('{PROFILE_URL}', '{USERNAME_COLOUR}', '{USERNAME}'), array($profile_url, $username_colour, $username), $tpl);
   }
   else
   {
      $tpl = (!$username_colour) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>';
      $_profile_cache[$cache_key]['full'] = str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($username_colour, $username), $tpl);
      $profile_url = '';
   }

   // Use the profile url from above
   $_profile_cache[$cache_key]['profile'] = $profile_url;

   // If - by any chance - no_profile is called before any other mode, we need to do the calculation here
   if ($mode == 'no_profile')
   {
      $tpl = (!$_profile_cache[$cache_key]['colour']) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>'
      return str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($_profile_cache[$cache_key]['colour'], $_profile_cache[$cache_key]['username']), $tpl);
   }

   return $_profile_cache[$cache_key][$mode];
}

Posted by Acyd Burn (Server Manager) on Dec 2nd 2008, 15:02

Code: Select all
function get_username_string($mode, $user_id, $username, $username_colour = '', $guest_username = false, $custom_profile_url = false)
{
   static $_profile_cache;
   static $_base_profile_url;

   $cache_key = $user_id . (string) $guest_username;
   if (isset($_profile_cache[$cache_key][$mode]))
   {
      // If the mode is 'no_profile', we simply construct the TPL code due to calls to this mode being very very rare
      if ($mode == 'no_profile')
      {
         $tpl = (!$_profile_cache[$cache_key][$mode]['colour']) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>';
         return str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($_profile_cache[$cache_key][$mode]['colour'], $_profile_cache[$cache_key][$mode]['username']), $tpl);
      }

      return $_profile_cache[$cache_key][$mode];
   }

   global $phpbb_root_path, $phpEx, $user, $auth;

   $username_colour = ($username_colour) ? '#' . $username_colour : '';

   if ($guest_username === false)
   {
      $username = ($username) ? $username : $user->lang['GUEST'];
   }
   else
   {
      $username = ($user_id && $user_id != ANONYMOUS) ? $username : ((!empty($guest_username)) ? $guest_username : $user->lang['GUEST']);
   }

   // Build cache for all modes
   $_profile_cache[$cache_key]['colour'] = $username_colour;
   $_profile_cache[$cache_key]['username'] = $username;
   $_profile_cache[$cache_key]['no_profile'] = true;

   // Profile url - only show if not anonymous and permission to view profile if registered user
   // For anonymous the link leads to a login page.
   if ($user_id && $user_id != ANONYMOUS && ($user->data['user_id'] == ANONYMOUS || $auth->acl_get('u_viewprofile')))
   {
      if (empty($_base_profile_url))
      {
         $_base_profile_url = append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile&amp;u={USER_ID}');
      }

      $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&amp;u=' . (int) $user_id : str_replace('={USER_ID}', '=' . (int) $user_id, $_base_profile_url);
      $tpl = (!$username_colour) ? '<a href="{PROFILE_URL}">{USERNAME}</a>' : '<a href="{PROFILE_URL}" style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</a>';
      $_profile_cache[$cache_key]['full'] = str_replace(array('{PROFILE_URL}', '{USERNAME_COLOUR}', '{USERNAME}'), array($profile_url, $username_colour, $username), $tpl);
   }
   else
   {
      $tpl = (!$username_colour) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>';
      $_profile_cache[$cache_key]['full'] = str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($username_colour, $username), $tpl);
      $profile_url = '';
   }

   // Use the profile url from above
   $_profile_cache[$cache_key]['profile'] = $profile_url;

   // If - by any chance - no_profile is called before any other mode, we need to do the calculation here
   if ($mode == 'no_profile')
   {
      $tpl = (!$_profile_cache[$cache_key]['colour']) ? '{USERNAME}' : '<span style="color: {USERNAME_COLOUR};" class="username-coloured">{USERNAME}</span>';
      return str_replace(array('{USERNAME_COLOUR}', '{USERNAME}'), array($_profile_cache[$cache_key]['colour'], $_profile_cache[$cache_key]['username']), $tpl);
   }

   return $_profile_cache[$cache_key][$mode];
}

Linked ticket with changeset: r9148

Action performed by Anonymous (I am too lazy to register) on Dec 2nd 2008, 16:19

Linked ticket with changeset: r9149

Action performed by Anonymous (I am too lazy to register) on Dec 2nd 2008, 16:22

Changed ticket status from "New" to "Fix completed in SVN"

Action performed by Acyd Burn (Server Manager) on Dec 2nd 2008, 16:22

Ticket details

Related SVN changesets