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

Comments

berdir’s picture

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

berdir
Primary language German
Location Switzerland
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
commented
Status: Needs review » Needs work
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Seems like a good change.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new 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
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
commented
Status: Needs work » Needs review
StatusFileSize
new static_cache.patch2.43 KB

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

mfer’s picture

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
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 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
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.