Seems like $users (and $links as reported in #449364: Latest dev removes homepage for registered users from [realname-link], instead it links to the profile) has been split between realname_make_name() and _realname_make_name().

This has resulted in that realname_make_name() only checks for a name in $users[$account->uid] but never actually assigns any value to it.

It's the opposite case in _realname_make_name() - it only assigns $users[$account->uid] - but never actually uses it.

The assigning of $users[$account->uid] should be moved from _realname_make_name() to realname_make_name() I think.

CommentFileSizeAuthor
#10 realname_static_cache.patch2.72 KBbcn
#8 realname_static.patch2.72 KBmsonnabaum
#6 realname.patch2.17 KBmaximpodorov
#1 realname.patch2.04 KBmaximpodorov

Comments

maximpodorov’s picture

maximpodorov commented
StatusFileSize
new realname.patch2.04 KB

Can it be a possible solution?

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented
Status: Active » Needs review

I'd like to see some testers.

maximpodorov’s picture

maximpodorov commented

Works for me :)
Really, it reduces query count significantly.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

There will be a new release on Wednesday; are you willing to bet that this can be included without breaking stuff?

The next tester can mark this RTBC.

bcn’s picture

bcn commented
Status: Needs review » Needs work

this patch no longer applies.

maximpodorov’s picture

maximpodorov commented
StatusFileSize
new realname.patch2.17 KB

This patch applies to the current dev version. It includes commenting out the line which addresses the static array $links which is defined in other function. I hope someone will have enough time to return the functionality of homepage calculation when the data is taken from the static cache. After that, this fix will be completed.

maximpodorov’s picture

maximpodorov commented

Any news for this problem fixing?

msonnabaum’s picture

msonnabaum commented
StatusFileSize
new realname_static.patch2.72 KB

Here's a different approach that I think works a bit better. It also should take care of the homepage issue.

It adds a D7-style central static cache so that statically cached variables can be shared across functions. This approach simply allows realname_make_name and _realname_make_name to use the same cache, so the existing logic works as expected.

This patch only addresses the one issue, but I imagine the central static cache, once committed, could be used in other places as well.

gausarts’s picture

gausarts commented

Subscribing. Thanks

bcn’s picture

bcn commented
Status: Needs work » Needs review
StatusFileSize
new realname_static_cache.patch2.72 KB 
+++ realname.module	11 Mar 2010 18:44:10 -0000
@@ -698,7 +699,7 @@ function realname_make_name(&$account) {
-
+  ¶

Too much whitespace here, but otherwise I think this patch looks good. Attached patch fixes this space issue, but is otherwise identical.

Thanks!

Powered by Dreditor.

gaellafond’s picture

gaellafond commented

Excellent!

After enabling devel query module, I found that RealName was generated a lot of queries. Most of them are due to a simple typo:
http://drupal.org/node/805526

And the rest are due to the broken caching system that you solve here.

Your cache function is a copy of &drupal_static() for D7:
http://api.drupal.org/api/function/drupal_static/7

May I suggest you to call the function drupal_static and define it like this:

if (!function_exists('drupal_static')) {
  function &drupal_static($name, $default_value = NULL, $reset = FALSE) {
    [...]
  }
}

That way, if we forget about this function when the module will be port to D7, this part of the module will still works in harmony with Drupal core.

Thanks for that Patch!

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented
Status: Needs review » Fixed

Committed to 6.x-1.x-dev.

@gaellafond: good suggestion.

voxpelli’s picture

voxpelli commented

@gaellafond: For future reference - conditional functions in php are not very good from a performance perspective - so what you suggest should be avoided - instead just note that the function should be replaced with drupal_static() once the module gets updated to D7.

Status: Fixed » Closed (fixed)

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