Hello,

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.

Files: 
CommentFileSizeAuthor
#4 only_cache_entities_loaded_by_id-1670848-4.patch1.31 KBcolan
PASSED: [[SimpleTest]]: [MySQL] 5,811 pass(es).
[ View ]
#1 entitycache-remove-unnecessary-setcaches-1670848.patch1.32 KBndeschildre
PASSED: [[SimpleTest]]: [MySQL] 5,720 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
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.

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.

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.

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

Cleaned it as per the coding standards.

Status:Needs review» Reviewed & tested by the community

The new patch applies cleanly and works as expected.

Issue summary:View changes

Patch #4 works well. Thanks.

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.