That function can possibly be called very often (currently ~92 times on our front page, lots of images) and every time it is, it calls ctools_export_load_object() which executes the same query over and over again.

Maybe it should be ctools_export_load_object() that adds a static cache, not sure, but it was easier to add it here.

CommentFileSizeAuthor
#10 1258286-file-displays-static-cache.patch2.26 KBDave Reid
#8 static_cache.patch2.43 KBBerdir
#5 file_entity_file_displays_static_cache.patch2.44 KBBerdir
static_cache_for_file_displays.patch2.49 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Project: D7 Media » File Entity (fieldable files)
Component: File entity » Code

Moving over to File entity, as requested by Dave Reid. Patch will need to be updated, though.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Needs work
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Seems like a good change.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
file_entity_file_displays_static_cache.patch2.44 KB

Same code as before.

Status: Needs review » Needs work

The last submitted patch, file_entity_file_displays_static_cache.patch, failed testing.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 7.x-1.x-dev » 7.x-2.x-dev

File entity has switched to a 7.x-2.x branch and the 7.x-1.x branch is no longer used. Please make sure to update your Git clones.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
static_cache.patch2.43 KB

Re-rolled the patch against 7.x-2.x

mfer’s picture

mfer CreditAttribution: mfer commented
Status: Needs review » Reviewed & tested by the community

This still applies with a small offset.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Reviewed & tested by the community » Needs review
FileSize
1258286-file-displays-static-cache.patch2.26 KB

This static cache doesn't really do a whole lot of good performance-wise with fallback condition.

If I run the following calls:

file_displays('video', 'default'); 
file_displays('video', 'full'); // View mode uses default
file_displays('video', 'media_original'); // Valid customized view mode
file_displays('video', 'default');

The patch in #8 results in:
3 cache misses
3 calls to field_view_mode_settings()
3 calls to file_displays_load()

This patch results in:
2 cache misses
2 calls to file_displays_load()
2 calls to field_view_mode_settings()

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Needs review » Fixed

Actually this is easy to reproduce the bug in #8 by just calling file_displays('video', 'non-existant-view-mode'); multiple times over and over. Each time will result in a cache miss and call to file_displays_load(). Tested and committed #10 to Git.

http://drupalcode.org/project/file_entity.git/commit/20eb1bf (file_entity 7.x-2.x)
http://drupalcode.org/project/media.git/commit/cf71b86 (media 7.x-1.x)

Status: Fixed » Closed (fixed)

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