field_info_fields() should be avoided. See https://drupal.org/node/1915646

The more memory-efficient field_info_field_map() function was introduced in Drupal 7.22. I think modules can now reasonably depend on Drupal >=7.22, because there have been security releases since then.

The attached patch implements the recommended changes.

It results in a large memory saving (c. 10MB) on a real site I manage when loading a View for the first time. That is, because I have patched all the rest of the site's contrib modules in the same way. (I'm not sure why Name tokens are loading for each View but perhaps that's a different matter)

CommentFileSizeAuthor
#8 name-2204883-8-use-field_info_field_map-for-less-memory-overhead.patch1.33 KBAlan D.
#5 name-memory-2204883-4.patch1.58 KBpjcdawkins
name-save-memory.patch861 bytespjcdawkins
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs review » Needs work

Like, but from the change log

if (VERSION >= '7.22') {
  // snippet B.
}
else {
  // snippet A.
}

i.e. the info file change will not be enforced (from pers. comms., I haven't tested this myself on early Drupal 7 versions)

Maybe

+      $fields = VERSION >= '7.22' ? field_info_field_map() : field_info_fields();
+      foreach ($fields as $field_name => $info) {
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

And from the looks of it, are you sure it was this location that called the function?

@see name_views_data_alter()

The token one is actually cached, so would not be called that often, but it is still a valid performance improvement.

i.e.

    // A cache hit will bypass the call to the field_info_fields()
    if (!$reset && $cache = cache_get("name_token_types_chained:$langcode", 'cache')) {
      $types = $cache->data;
    }
...
pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented

Re #2: it was revealed by a backtrace from field_info_fields() which was only getting called on loading a View from a cold views cache. Not that often but often enough in a site with lots of pages that contain different views. Re #1: fair point, I'll submit another patch when back in front of a computer

pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented
Status: Needs work » Needs review

New patch attached.

BTW while the change log recommends VERSION >= '7.22', as mentioned here, we should use version_compare() instead.

pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented
FileSize
name-memory-2204883-4.patch1.58 KB

Oops. Here's the patch.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

nice, now I just need a couple of Drupal versions to test before committing...

webflo’s picture

webflo CreditAttribution: webflo commented
Status: Needs review » Needs work

I think we should get this in but without the version_compare and just add a dependency to system module (>= 7.22).

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs work » Needs review
FileSize
name-2204883-8-use-field_info_field_map-for-less-memory-overhead.patch1.33 KB

As per webflo's old suggestion. Bumped to 7.40 for no real reason :P

  • Alan D. committed 0a4408c on 7.x-1.x 
    Issue #2204883 by pjcdawkins, Alan D., webflo: Bring Field API Memory...
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs review » Fixed

3 years grace was enough time to update core, bumped again to 7.50 ;)

Status: Fixed » Closed (fixed)

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