I'm wondering whether the views plugin data should be cached as they might produce quite some calls to t()

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fietserwin’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
895 bytes

I was playing a bit with the new Zend Server Z-Ray tool and found indeed that getting this info consistently takes over 10 ms on my local machine (all included data and programs on sdd, internal memory by far not exhausted). So a big yes to this feature.

To give it a kick, a first try.

fietserwin’s picture

FileSize
1.16 KB

OK, if I post a patch, I can as well post a working patch, not just "a hint of how it could be done" :)

2nd try:
- working code.
- cached per language as the data contains translated strings (that's why we want to cache it...).
- execution time of _views_fetch_plugin_data() reduced to around 3-4 ms.

das-peter’s picture

+1 to for caching.

Visual review looks good, only thing I found is this:

+++ b/includes/cache.inc
@@ -127,12 +127,26 @@ function _views_data_process_entity_types(&$data) {
+      $cache = cache_get($cache_key);
+      if (is_object($cache)) {

Throughout core following pattern is more common:
if ($cache = cache_get($cache_key)) {

fietserwin’s picture

Thanks for reviewing. New patch attached. I found a bogus invocation that might defer any gains (though it did not in my local test). Feel free to not include that part, or open a separate issue for that, if there's something really wrong at that point.

Kars-T’s picture

Status: Needs review » Reviewed & tested by the community

I ran the patch from #4 against my project and it doesn't seem like it broke anything but speed things up. The caching by language seems very reasonable. And the removing of the array from pager_plugin is wise as well or it will never cache.

From the code the patch seems clean. I think it is RTBC.

dawehner’s picture

Status: Reviewed & tested by the community » Fixed

10ms is not that bad, honestly.

Committed to 7.x-3.x and pushed

Status: Fixed » Closed (fixed)

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