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.

Files: 
CommentFileSizeAuthor
#10 1258286-file-displays-static-cache.patch2.26 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 17 pass(es).
[ View ]
#8 static_cache.patch2.43 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 11 pass(es).
[ View ]
#5 file_entity_file_displays_static_cache.patch2.44 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity_file_displays_static_cache.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
static_cache_for_file_displays.patch2.49 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

Berdir’s picture

Status:Active» Needs review
Berdir’s picture

Project: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

Status:Needs review» Needs work
Dave Reid’s picture

Seems like a good change.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new2.44 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity_file_displays_static_cache.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

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

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

Status:Needs work» Needs review
StatusFileSize
new2.43 KB
PASSED: [[SimpleTest]]: [MySQL] 11 pass(es).
[ View ]

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

mfer’s picture

Status:Needs review» Reviewed & tested by the community

This still applies with a small offset.

Dave Reid’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new2.26 KB
PASSED: [[SimpleTest]]: [MySQL] 17 pass(es).
[ View ]

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

If I run the following calls:

<?php
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

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.