-
-
Notifications
You must be signed in to change notification settings - Fork 439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add runtime cache to Zend_Locale_Data #918
Conversation
Minor improvement: might as well save the result ( |
Haha, I just realized that this PR is based on my patch from almost 8 years ago! Thanks for digging it up! |
yep, oldi but goldi ;) |
cd5d7e9
to
a1fcb81
Compare
} | ||
static::$_localCache['id'] = $data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmotyl
why this key used 'id'
?
should it be the $id
variable ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn, it should. Can you make a PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and in line 1526 too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one reason, why its always good to wait for a second Review 👀
Fixed in 8b89754. Thanks @ilnytskyi! |
We tried this patch with M2 and we got an error :)
|
@@ -985,7 +1001,9 @@ public static function getContent($locale, $path, $value = false) | |||
$val = urlencode($val); | |||
$id = self::_filterCacheId('Zend_LocaleC_' . $locale . '_' . $path . '_' . $val); | |||
if (!self::$_cacheDisabled && ($result = self::$_cache->load($id))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw if I understand correctly lack of
// add runtime cache to avoid callng cache backend multiple times during one request
if (isset(self::$_localCache[$id])) {
return self::$_localCache[$id];
}
before the if
will trigger $_cache->load($id)
and data is not taken from runtime
Noticed that after the patch we anyway have many Zend_LocaleC_
coming from cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you propose a patch/pull request? thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1018
I've pushed the change, and tested it.
We ran into seemingly random warnings in both frontend and backend:
Code adjacent to line 853: // erase day string
$daylist = Zend_Locale_Data::getList($options['locale'], 'day');
foreach($daylist as $key => $name) {
if (iconv_strpos($number, $name) !== false) {
$number = str_replace($name, "EEEE", $number);
break;
}
Not sure if #1018 would fix it. [edit] |
Fixes: #917
This imporves performance,as Zend is not calling cache backend multiple times for locale information.