Seems waste of memory to me, but I think Tim should comment.

The patch has some code style issues.

It's also missing an AutoLoader entry

Also, the "require_once 'ObjectCache.php';" shouldn't be needed

The patch is also not properly targetting a file

(In reply to comment #1)

Seems waste of memory to me, but I think Tim should comment.

Can you explain it a little?

Core devs don't see the use or find things wrong with the patch. This issue will not progress until the reported updates the patch so that it can be applied properly to trunk.

john wrote:

-needs-review

Per the above the diff isn't properly targetting a file and needs an Autoloader entry. Also if you could make it confirm to our Coding Conventions it would be appreciated. https://www.mediawiki.org/wiki/Manual:Coding_conventions

Let me know on MediaWiki-General if you'd like some help :)

+reviewed

Updated patch for r101353

Why it's not properly targetting a file?
Do you mean the path must be 'phase3/includes/LocalisationCache.php' instead of just 'includes/LocalisationCache.php'?
If so, see the updated patch. I've also corrected the code style, added an AutoLoader entry and removed require_once ObjectCache.php, although I remember it didn't work in 1.16 without it :)
Also, there was an issue with accel store - sometimes 'list:messages' expired in XCache while 'deps' didn't, which leaded to warnings on the top of page. I've fixed it - now it checks both keys to determine if the cache is missing.

Attached:

Minor thing, we don't capitalise null

r101492

re-applied at r101507 with way to make it work if there is no APC cache installed

Thanks! So, wfGetCache doesn't return FakeMemCachedClients anymore?

(In reply to comment #10)

Thanks! So, wfGetCache doesn't return FakeMemCachedClients anymore?

I don't know whether it ever did, I'm not familiar with wfGetCache(). But I can tell you what it did on my install :) . I do recall that Tim refactored wfGetCache() and the whole ecosystem around it a while ago, perhaps that's when this change in behavior was introduced.

See ObjectCache::newAccelerator() in includes/objectcache/ObjectCache.php

In the meantime I've found another bug with the implementation - it also relies on the presence of key '...:preload' and doesn't check it when checking for the expired cache, which can lead to "Invalid or missing localisation cache" exceptions.
I'll fix it soon.
About the comments to r101507:
Maybe LCStore_Accel should force disable manualRecache itself?

Check preload to recache

Attached:

back in the day I livehacked in APC-based cache for languages in mediawiki, it definitely makes sense to keep this data in memory ;)

New patch applied r102304