Cache all attribute options for display in layered navigation #943

[rvelhote]

Related to issue #934.

This will improve performance by reducing the amount of SQL Queries when fetching all possible attributes to display in the layered navigation. The amount of queries you will save is related to the amount of attributes you each store has configured to be available for layered navigation.

When saving attributes Magento will clear the translate cache tag so that cache tag as well as the EAV cache tag are included with this cache object.

[colinmollenhour]

I may be missing something, but you do not need to add the translate cache tag because updating translations will not have a material affect on the attribute options.

You should add the individual id tag as well to be safe. So the final $tags = line can be:

            $tags = array_merge(['eav'], $this->getAttribute()->getCacheTags());

[rvelhote]

I included the translate cache tag because Magento also clears it after saving an attribute (with the comment Clear translation cache because attribute labels are stored in translation). If the user updates the attribute with a new translation then no one would see it until the translation cache was cleared.

The suggestion you made to use $this->getAttribute()->getCacheTags() is much better and it also includes Mage_Eav_Model_Entity_Attribute::CACHE_TAG plus an attribute specific tag. I will update the PR.

[rvelhote]

I updated the PR to match @colinmollenhour suggestions.

I've been using this in production for a couple of months and works well in reducing the number of SQL queries. Of course it will hit the cache backend instead so if it's the database it's not much of a difference but if it's Redis or Memcached it will be better.

[sreichel]

Already tested past days .... LGTM

[tmotyl]

Hi
I've took the patch for a little ride. Here are my findings:

1. The SQL query we will avoid looks like:

SELECT `main_table`.*, `tdv`.`value` AS `default_value`, `tsv`.`value` AS `store_default_value`, IF(tsv.value_id > 0, tsv.value, tdv.value) AS `value` FROM `eav_attribute_option` AS `main_table`
 INNER JOIN `eav_attribute_option_value` AS `tdv` ON tdv.option_id = main_table.option_id
 LEFT JOIN `eav_attribute_option_value` AS `tsv` ON tsv.option_id = main_table.option_id AND tsv.store_id = '7' WHERE (`attribute_id` = '184') AND (tdv.store_id = 0) ORDER BY main_table.sort_order ASC, value ASC

2. In my case on category page the $storeId = $this->getAttribute()->getStoreId(); was always null, which resulted in storing optionArray under empty string in $this->_options[$storeId].
   I recommend changing the line to:

$storeId = $this->getAttribute()->getStoreId();
if (is_null($storeId)) {
      $storeId = Mage::app()->getStore()->getId();
}

The same way it's done in setStoreFilter. This way it will be clear what is stored in $this->_options.
Anyway, to be honest I don't see how would one be able to change attribute store to have multiple store ids inside _options, but its another story.

3. In my case I saw multiple calls to getAllOptions with the same attribute, so we might want to consider using a runtime cache too. I would be interested in getting feedback from you whether its also a case for you, or it depends on project customizations.

[colinmollenhour]

@tmotyl A runtime cache usually makes sense because this will not be GCd in the lifetime of the request anyway and memory is cheap.

[rvelhote]

@tmotyl I will add a local runtime cache. I only see one call for each attribute on my store so it might be something specific to your store.