When calling taxonomy_get_term_by_name(), the resulting term will be cached again and again on database, even though it is already in the cache table.

The broader issue is that entityCacheLoad(), when called with $conditions instead of $ids (e.g. in this case, $conditions = array(vid=xx and name=xx)), will not find the cached entry (since indexed via the indexKey), and will cache the results every time.

So each time an entity is loaded via $conditions, a cache_set will be done. Not helping performance, especially if your cache tables are still in database.

I know $conditions are apparently deprecated, but some core functions still use that, e.g. taxonomy_get_term_by_name.

A patch is incoming.

#8 cache_entities_by_id-1670848.8.patch969 bytestorgosPizza
PASSED: [[SimpleTest]]: [MySQL] 5,828 pass(es).
[ View ]


ndeschildre’s picture

Status:Active» Needs review
new1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 5,720 pass(es).
[ View ]

The attached patch will not cache entities loaded in entityCacheLoad with $conditions, since the cache entries cannot be used by calls of entityCacheLoad with $conditions.

johnv’s picture

Status:Needs review» Reviewed & tested by the community

This patch reduces the amount of Queries on my pages a lot.
Tested it by :
- show page: show number of queries with devel_querylog
- enable patch
- refresh page: show number of queries with devel_querylog
- rename a term
- refresh page: change of term is reflected.

Dave Reid’s picture

Status:Reviewed & tested by the community» Needs review

The patch in #1 needs to be adjusted for the coding standards. Comments need a space between the // and the comment text, and if statements need a space in-between the if and the next parenthesis.

colan’s picture

new1.31 KB
PASSED: [[SimpleTest]]: [MySQL] 5,811 pass(es).
[ View ]

Cleaned it as per the coding standards.

johnv’s picture

Status:Needs review» Reviewed & tested by the community

The new patch applies cleanly and works as expected.

marcelovani’s picture

Issue summary:View changes

Patch #4 works well. Thanks.

damiankloip’s picture

This generally makes sense.

+++ b/entitycache.module
@@ -112,7 +112,15 @@ class EntityCacheControllerHelper extends DrupalDefaultEntityController {
+          if (empty($queried_entities_by_id) == FALSE) {

I think it would be more readable to just do !empty() here.

torgosPizza’s picture

new969 bytes
PASSED: [[SimpleTest]]: [MySQL] 5,828 pass(es).
[ View ]

Here is a re-roll of the patch from #4 with the change suggested in #7.

DamienMcKenna’s picture

We just had another project that was incorrectly running cache_set() because of this problem, and the patch worked great.

skwashd’s picture

Status:Reviewed & tested by the community» Fixed

Thanks for the patch. I have committed #8 and it will be included in the next release.

skwashd’s picture

  • skwashd committed dbf1820 on 7.x-1.x authored by ndeschildre
    Issue #1670848 by torgosPizza, colan, ndeschildre: Fixed Unnecessary...

Status:Fixed» Closed (fixed)

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