Comments

tim.plunkett’s picture

Status: Active » Closed (duplicate)

That's a really great idea, I agree.

Field Collection is using the Entity API, and there is an issue for that. Once that's in, I don't believe there is anything else that would need to be done after that issue is in.

So, marking as duplicate of #1111120: Support for EntityCache.

jhedstrom’s picture

Status: Closed (duplicate) » Active

I'm re-opening this, because the way this was implemented in Entity API is that it defaults to false, which, unfortunately, pushes the responsibility back out to this module.

jhedstrom’s picture

Status: Active » Closed (duplicate)

Re-closing, after a chat with tim in irc, I'm going to try and push this back up the stack to Entity API itself.

jhedstrom’s picture

Fidelix’s picture

Status: Closed (duplicate) » Active

The outcome of #1336924: Default to use EntityCache if available, and create necessary cache bin tables was not positive, so we still have this feature missing, and this probably won't change unless we do it in the field collection side.

I'm setting this back to active, but feel free to set the issue to whichever status you find more appropriate.

Fidelix’s picture

Sorry to bump this, but can we move this forward?

kevinquillen’s picture

This should be a priority, in my opinion.

If you have a field collection with a large amount of data, embedded in a host entity, it starts hitting performance pretty hard. Especially if there is a lot of other things going on, formatters, theming, image loaders etc. If you have more than one FC on an entity, ouch.

Even if you get EntityCache installed for Field Collection, what you really want to cache is the rendered output (in my opinion), not the item_id- or at least everything you need to render with. I keep getting this fatal error too, with Field Collection Table module:

Call to undefined method stdClass::view() in /sites/all/modules/contrib/field_collection_table/field_collection_table.module on line 56

kevinquillen’s picture

Priority: Normal » Major

If I change the display formatter to the default:

Fatal error: Call to undefined method stdClass::view() in /sites/all/modules/contrib/field_collection/field_collection.module on line 887

kevinquillen’s picture

Would seem like the class is being lost when EntityCache is enabled, throwing that error. The class on the item from get_class is stdClass, not FieldCollectionItemEntity.

ezeedub’s picture

From the Entity Cache project page:

There is no automated support for contrib entities, to use entitycache for a contrib or custom entity:

  • Create a cache_entity_$myentity table/bin.
  • Specify EntityCacheDefaultEntityController as your controller class in hook_entity_info(), or extend it.
  • Never update your entity tables outside of save/delete API calls.

What work needs to be done in addition to this for this module? I need this!

I got here after profiling a site that makes heavy use of field collections. After adding several thousand nodes, a few hundred of which had up to 100 field collection items (total FCs approx 8000), I noticed that node/add was taking forever to load.

Turns out there's an EntityFieldQuery for each field collection item. Why is that even the case on node/add?

kevinquillen’s picture

You have to create the table like it says (just duplicate the cache table and rename it), and then in Field Collection .module, alter the entity info hook for the caches to TRUE and change the controller class as it says.

The problem it seems is that what gets cached is losing its class definition, instead of FieldCollectionEntityItem, its cached as stdClass, and when its unserialized you get the fatal PHP errors above (call to undefined method).

I did a quick and dirty patch to FieldCollection Table module to cache the rendered HTML for 30 minutes and use that to improve page load performance to get by this for now.

saltednut’s picture

I did a quick and dirty patch to FieldCollection Table module to cache the rendered HTML for 30 minutes and use that to improve page load performance to get by this for now.

@kevinquillen is your patch sandboxed? We've inherited some problematic architecture with similar problems regarding field collections.

kevinquillen’s picture

Note that this is for Field Collection Table module. Here is the basic gist of what I did to get around this:
~Line 110 of field_collection_table.module:


      $element[0] = array(
        '#theme' => 'table',
        '#header' => $header,
        '#rows' => $rows,
      );

      $element[0]['#hostEntityID'] = $entity->identifier();
      $element[0]['#fieldCollectionBundle'] = $instance['field_name'];
      $element[0]['#view_mode'] = $settings['view_mode'];

~Line 180 to hook_theme, same file:


'table__field_collection_table' => $base + array(
      'variables' => array(
        'header' => NULL,
        'rows' => NULL,
        'attributes' => array(),
        'caption' => NULL,
        'colgroups' => array(),
        'sticky' => TRUE,
        'empty' => '',
        'settings' => array(),
        'hostEntityID' => NULL,
        'fieldCollectionBundle' => NULL,
        'view_mode' => NULL,
      ),
    ),

End of the .module file, add a hook_flush_caches:


/**
 * Implements hook_flush_caches().
 * @return array
 */
function field_collection_table_flush_caches() {
  return array('cache_field_collection_table');
}

Then I made this change in its theme.inc on the function theme_table__field_collection_table:


/**
 * Derivative of theme_table() solely for the HOOK_preprocess_table__PATTERN().
 */
function theme_table__field_collection_table($variables) {
  if ($cache = cache_get($variables['fieldCollectionBundle'] . ':' . $variables['view_mode']  . ':' . $variables['hostEntityID'], 'cache_field_collection_table')) {
    return $cache->data['html'];
  }

  $content = theme_table($variables);
  cache_set($variables['fieldCollectionBundle'] . ':' . $variables['view_mode']  . ':' . $variables['hostEntityID'], array('html' => $content), 'cache_field_collection_table', strtotime('+3 hours', REQUEST_TIME));
  return $content;
}

It may not be the 'right' way to do it but I needed a fast solution. This keeps the rendered HTML cached for 3 hours or until Drupal cache is cleared.I could not afford 80+ SQL calls per entity_load (and dozens of entity_loads for each field collection item) of a host entity when its page view callback was called, it is so heavy. If your field collection tables in the database have hundreds of thousands of rows, it becomes a huge bottleneck response time for the user.

After this change and the cache became primed for many of these entities, my NewRelic reports got MUCH better.

ian-moran’s picture

Hello @tim.plunkett

We spoke last night at the meetup, and I've done a little more digging since.

So if you change the controller class to:

'controller class' => 'EntityCacheDefaultEntityController',

You get different errors for node creation and node view:

Node Creation - EntityMalformedException: Missing bundle property on entity of type field_collection_item. in entity_extract_ids() (line 7633 of /home/communit/public_html/test/includes/common.inc).

Node View - Fatal error: Call to undefined method stdClass::view() in field_collection/field_collection.module on line 1213

You can go to the node edit page without difficulty but save gives the following error - Recoverable fatal error: Argument 1 passed to field_collection_item_is_empty() must be an instance of FieldCollectionItemEntity, instance of stdClass given, called in ield_collection/field_collection.module on line 1655 and defined in field_collection_item_is_empty() (line 1029 of field_collection/field_collection.module).

I noticed that the EntityAPIController uses entityCacheGet and EntityCacheDefaultEntityController uses entityCacheLoad and this could explain the problem with obtaining the correct id's, but it feels like a bit of a stab in the dark.

If you can point me in the right direction I'm happy to continue investigating, testing, and patching.

Thanks a lot

kevinquillen’s picture

I actually just removed the HTML caching I did in favor of a Views based cache. Neither seemed to have much impact, and it was interfering with memcache.

If you have a FC entity that has 30-60 rows, it can take up to 8-14 seconds to get and load and render all that data which will destroy the page you requested in the process. It's a killer wait.

Is there any sane way to implement caching so the user doesn't pay the price?

steven jones’s picture

Status: Active » Needs review
StatusFileSize
new1.71 KB

Let's get this back on track eh?

Here's a patch that adds a 'Field collection - Entity cache' module that is incredibly simple, it does the following:

  • Has dependencies on Field collection, and Entity cache.
  • Adds the cache tables for Entity cache to use.
  • Adds entity cache support to the field collection item entity definition.

This seems to work really well, I've had a quick glance over the field collection code, and it looks like we use the API, so we should be good to go, though someone more familiar with the field collection code will need to check this.

The reason that I've added a complete new module is because working out when entity cache is on, and then only creating the new database table is actually quite hard, so doing it this way makes it super simple and obvious what it going on.

In some simple tests, on our site that heavily uses field collection, some pages executed in half the time they were before, and used half the number of SQL queries.

saltednut’s picture

@Steven Jones Haven't had time to apply the patch and try it out but I've reviewed the code here and its exactly what we'd been doing - using a 'helper' module to maintain that field_collection has its cache tables in place when entitycache is on. +1 for that.

steven jones’s picture

Here's a quick follow up to my patch in #16 apparently we should be declaring the cache bin as one that should get flushed.

steven jones’s picture

Another quick follow up, we should also disable the field cache, if we have entity cache, so that is done in this patch.

john bickar’s picture

Status: Needs review » Needs work

I tested this patch on a node with 11 field collections with three fields each (image, text, text area). Results were inconclusive. It sped up the page load for anonymous users (~300ms vs. ~800ms), but loading the node/edit page got significantly slower (1m30s vs. 6.3s).

I am not sure why this patch would negatively impact loading the node/edit page. No warnings were generated.

nicksanta’s picture

Status: Needs work » Needs review
dasjo’s picture

we have a generic solution to support entitycache for entity api entities now: #2017685-4: Automatically create cache tables for entities that want to support entitycache.module
what remains is, i guess, to add support in field_collection_entity_info()

something like

  // Support entity cache module.
  if (module_exists('entitycache')) {
    $return['field_collection_item']['field cache'] = FALSE;
    $return['field_collection_item']['entity cache'] = TRUE;
  }
lpeabody’s picture

Issue summary: View changes
StatusFileSize
new421 bytes

Added patch with code suggestion from #22. Reinstalling after applying patch #4 from https://drupal.org/node/2017685, and applying the patch I added here, the cache_entity_field_collection_item table gets created and after adding a data to a field collection, and then viewing the node, the cache table is successfully updated. I think it might be working :)

Status: Needs review » Needs work

The last submitted patch, 23: field_collection-1268620-entity-cache-23.patch, failed testing.

dwillems’s picture

Status: Needs work » Needs review
StatusFileSize
new423 bytes

Tried to re-roll #23 over the latest HEAD. I can re-confirm this works with the Entity API patch applied and with a project making wide use of field collections, having them in memcache is quite useful.

nicksanta’s picture

I've been using this patch in production for 6 months now.

dasjo’s picture

as the entitycache support for entity api entities has been committed, you should switch to the latest patches, see #25

saltednut’s picture

Title: Entity Cache » Use Entity Cache in Field Collection
mariacha1’s picture

Status: Needs review » Reviewed & tested by the community

Can't see any issues with RTBCing this.

Andre-B’s picture

please get this in :)

fago’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

mariacha1’s picture

Just leaving a note here, that this update does require the latest dev version of Entity API to work. (Checkout at git hash d9baed7.) This will no longer be the case once Entity API v. 1.4 containing https://drupal.org/node/2017685 is released.

ianthomas_uk’s picture

Status: Fixed » Needs work

That's actually quite a big problem - without that patch you'll get fatal errors when saving nodes. I'd suggest that beta7 should be unpublished until a new version of Entity API is released (unless we can just do a new release of Entity API now, I notice both are maintained by @fago).

Once that has been released, the field_collection.info should be updated to require entity_api 1.4 or above.

fago’s picture

Entity 1.4 is already out - so in conjunction with that one is fine. I'd not add a general dependency as it's only problematic with entity cache *if* you use the db backend + the dependency system does not play nice with dev versions. I added a not on the project page instead.

fago’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

heddn’s picture