image_load makes admin/content/media unusably slow

Grr this is due to the fact that we don't have access to the image height and width attributes from file_load() due to a stupid core decision - #1448124: Image dimensions should be available from file_load() for images, and not stored in field data. You get PHP notices if you do not provide height and width for theme_image_style(). What we will have to do is add a new table {image_dimensions} via file_entity with columns fid, height, and width that can store this data and then load these values in via hook_file_load().

Is it really necessary to create a new table? I suggest the following..

1. try to get the data saved with the image field using file_usage in a db query
2. if that fails, fall back to using image_get_info
3. if that also fails, null values will be passed as width and height, which should avoid the php notices

I have tried to modify the file_entity_file_formatter_file_image_view function. (from 7.x-1.0-rc3, where I have the same problem) Here it is:

function file_entity_file_formatter_file_image_view($file, $display, $langcode) {
  $scheme = file_uri_scheme($file->uri);
  $local_wrappers = file_get_stream_wrappers(STREAM_WRAPPERS_LOCAL);
  
  if (isset($local_wrappers[$scheme]) && strpos($file->filemime, 'image/') === 0 ){

  	$width = NULL;$height = NULL;
  	
  	//try to read the field name of this file in the db, using the file_usage table
  	$field_name_result = db_query(
  	 "SELECT field_config_instance.field_name ".
  	 "FROM file_usage ".
  	 "JOIN node ON node.nid = file_usage.id ".
  	 "JOIN field_config_instance ON node.type = field_config_instance.bundle ".
  	 "JOIN field_config ON field_config_instance.field_id = field_config.id ".
  	 "WHERE file_usage.module = 'file' && file_usage.fid = :fid && field_config.type = 'image' LIMIT 1",
  	array(':fid'=>$file->fid))->fetchCol(0);
  	
  	if(!empty($field_name_result)){
  		//with the field name, we can read the field data for the file
  		$field_name = $field_name_result[0];
  		$field_data = db_select('field_data_'.$field_name,'fd')
  		              ->fields('fd',array($field_name.'_width', $field_name.'_height'))
  		              ->condition($field_name.'_fid',$file->fid)
  		              ->execute()
  		              ->fetchAssoc();
  		              
  		if(!empty($field_data))
  		{
  		    $width = $field_data[$field_name.'_width'];
            $height = $field_data[$field_name.'_height'];
  		}
  	}
  	
  	//if the field data couldn't be found in the db fall back to the image_get_info function
  	if(empty($width) || empty($height))
  	{
  		$info = image_get_info($file->uri);
  		$width = $info['width'];
  		$height = $info['height'];
  	}
  	
    if (!empty($display['settings']['image_style'])) {
      $element = array(
        '#theme' => 'image_style',
        '#style_name' => $display['settings']['image_style'],
        '#path' => $file->uri,
        '#width' => $width,
        '#height' => $height,
      );
    }
    else {
      $element = array(
        '#theme' => 'image',
        '#path' => $file->uri,
        '#width' => $width,
        '#height' => $height,
      );
    }
    return $element;
  }
}

But as you say, I agree, it would be more relevant to save the width and height with the file data in stead of the field data.. Or it would be a good idea to add which field uses the file in the file_usage table, because, that would make it easier to get the field data.

image_theme() defines defaults for width and height, so you only get warnings when width and height are not provided if you haven't cleared the cache or you are calling theme_image_style() directly (illegal).

@prinds Your code would add two additional multi-table queries for every single image, so yes, an additional {image_dimensions} table is wise in this case.

@Dave Reid,
well, two multi-table queries is a lot faster than image_load in my setup.. :)

But I see what you mean. In the long run, an extra table is better performance (and probably also safer than complex queries)..

I would be happy to work on this during the Denver2012 sprint.

Here is a patch implementing the solution suggested in #2. Image dimensions are loaded from the {image_dimensions} table in file_entity_file_load as $file->image_dimensions. In file_entity_file_formatter_file_image_view, if $file->image_dimensions is not set, the dimensions are loaded using image_get_info and stored in {image_dimensions}.

Here is an updated patch with hook_file_insert, hook_file_update abd hook_file_delete support. There is no more code to load image dimensions file_entity_file_formatter_file_image_view since it is always done on file_save and file_load. I did write tests for the patch, but I'm issue running the test cases locally.

Here is an updated version of the patch in #11 with fixed tests.

+++ b/file_entity.file.incundefined
@@ -109,3 +118,80 @@ function file_entity_file_operation_info() {
+    // Delete image dimensions from the {image_dimensions} table if it's already there.
+    db_query('DELETE FROM {image_dimensions} WHERE fid = :fid', array(':fid' => $file->fid));
+    // Insert the image dimensions into the {image_dimensions} table.
+    db_query('INSERT INTO {image_dimensions} (fid, width, height) VALUES (:fid, :width, :height);', array(
+      ':fid' => $file->fid,
+      ':width' => $file->image_dimensions['width'],
+      ':height' => $file->image_dimensions['height'],

We should use db_merge() here. Otherwise this is a great patch and you've put really good work into it, especially with the documentation. Once we've got that

+++ b/file_entity.installundefined
@@ -531,22 +531,22 @@ function file_entity_file_formatter_file_image_view($file, $display, $langcode)
 
   $scheme = file_uri_scheme($file->uri);
   $local_wrappers = file_get_stream_wrappers(STREAM_WRAPPERS_LOCAL);
-  if (isset($local_wrappers[$scheme]) && $image = image_load($file->uri)) {
+  if (isset($local_wrappers[$scheme]) && isset($file->image_dimensions)) {
     if (!empty($display['settings']['image_style'])) {

Just a note that #1142630: media_fid_illegal_value error when adding media to an entity is going to conflict with this hunk which is about to land. I can probably fix this when applying. Just a note to myself.

Here is an updated patch suing db_merge().

Awesome. Putting on commit list.

Committed to 7.x-2.x: http://drupalcode.org/project/file_entity.git/commit/d897533

This probably needs to be backported to Media for 7.x-1.x since it's a severe performance issue.

FYI.. I'm not sure, but I think this commit might be causing an issue with the @becw patch on #1426730: Automatically open the file edit modal after uploading a file. I'm not sure if her patch needs to be reworked or if I need to open a new issue for File Entity?

My comment http://drupal.org/node/1426730#comment-5817694 shows how the warning appears.

Here is a possible patch that would backport to Media 7.x-1.x. Had little chance to fully test.

Ouch, this is huge. Took me a while to track this down as it only happened on production. We have the files on a relatively slow NFS storage and this is absolutely killing the site, as we have tons of images displayed everywhere. I'm talking about 60s+ response times.

I have commented it out for now, this site has it's own handing for that anyway as it was built before this existed. Will have a closer look when I find the time.

I read through this thread and I wonder why I am having this problem. See I'm not using images. I have a folder with a few hundred MP3 files. These obviously do not have a height or width. Yet when I load a piece of media from the library, the full list does not load. And I have to scroll down the list and wait for Ajax to load it bit by bit.

It used to take over a minute to load the full list. I got it down a little. I visited admin/config/media/browser
and switched the allowed field extensions from:
jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp mp3 mov m4v mp4 mpeg avi ogg oga ogv wmv ico

to mp3

This has obvious drawbacks if I ever want to use the Media module to manage any other media besides MP3.

It now loads in about 20 seconds or so. But I would ask, why is it not possible for me to load all the songs instantly?

Reroll of #15 for Media 7.x-1.x with the File Entity test omitted.

Tested it locally and it worked fine.

#24: no_image_load_in_formatter-1447790-24.patch queued for re-testing.

The code looks good, though I haven't tested it yet =)

This would also require removing existing update function in file_entity..care to submit a patch there, so i commit them both in same time?

Just tested this. and it worked perfectly. I don't know what update function is being talked about here, but this should be in 7.x-1.x-dev ASAP. After adding a couple of large files and in a server with 128Mb PHP max mem, this prevented the media browser from working.

@rootatwc: what update function are you referring to? Is it in 7.x-1.x or 2.x? If in the first version, don't forget that file entity is included with the media module of that version.

this would be not needed
http://drupalcode.org/project/file_entity.git/blob/0a29735:/file_entity....
If i commit this one, then file_entity_update_7200 will make other updates fail, right?

We should ensure any update adding the table first checks db_table_exists(), file_entity_update_7200() included.

Back to needs work then, to add this conditional.
This issue will take care of it in file_entity #1890486: Check if image_dimensions exists before creating it

I added the table check to the update function. Applies cleanly to latest 7.x-1.x.

setting back to needs review

After reading more I've noticed that in File Entity it uses !$file->filesize instead of !filesize($file->uri) for determining if the file is empty. Since filesize() doesn't prevent the warning mentioned in the code comments (http://drupal.org/node/681042) I've updated the patch to use $file->filesize.

I haven't thoroughly tested this patch but it applied without errors, and I am not experiencing any performance issues related to the media module.

#35: no_image_load_in_formatter-1447790-35.patch queued for re-testing.

Committed #35 to 7.x-1.x.

+++ b/file_entity/file_entity.install
@@ -211,3 +243,44 @@ function file_entity_update_7002() {
+function file_entity_update_7101() {
+  if (!db_table_exists('image_dimensions')){
+    $schema['image_dimensions'] = array(
+      'description' => 'Cache images dimensions.',

How is this going to get populated for existing images?

@joachim: In file_entity_file_load($files), file_entity_image_dimensions($file, FALSE); is called for all loaded files. This function takes care of populating the table.