Download & Extend

Static cache in realname_make_name() broken in dev

Project:RealName
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

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.

Comments

#1

Can it be a possible solution?

AttachmentSizeStatusTest resultOperations
realname.patch2.04 KBIgnored: Check issue status.NoneNone

#2

Status:active» needs review

I'd like to see some testers.

#3

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

#4

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.

#5

Status:needs review» needs work

this patch no longer applies.

#6

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.

AttachmentSizeStatusTest resultOperations
realname.patch2.17 KBIgnored: Check issue status.NoneNone

#7

Any news for this problem fixing?

#8

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.

AttachmentSizeStatusTest resultOperations
realname_static.patch2.72 KBIgnored: Check issue status.NoneNone

#9

Subscribing. Thanks

#10

Status:needs work» needs review

+++ 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.

AttachmentSizeStatusTest resultOperations
realname_static_cache.patch2.72 KBIgnored: Check issue status.NoneNone

#11

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!

#12

Status:needs review» fixed

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

@gaellafond: good suggestion.

#13

@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.

#14

Status:fixed» closed (fixed)

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

nobody click here