Issue status update for http://drupal.org/node/27431 Post a follow up: http://drupal.org/project/comments/add/27431 Project: Drupal Version: cvs Component: other Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Cvbge Updated by: Bèr Kessels Status: patch +1 for this feature. But we should be carefull with it, though. This function is called an awful lot. Cvbe, have you got experience with benchmarking? I am interested to see if this has an impact on speed and performance. If so, we could consider a lower level (inside the function) "caching": just see if the file exists in the file table, if so return those functions. But let's not keep that from including this filesize. Bèr Kessels Previous comments: ------------------------------------------------------------------------ Mon, 25 Jul 2005 11:40:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/image.inc.diff (1.14 KB) image_get_info() return information about image - width, height, extension, mime type. It's quite common to get file size (physical size) when getting dimension. This simple patch adds file_size to the returned array that contains file's size in bytes.

Issue status update for http://drupal.org/node/27431 Post a follow up: http://drupal.org/project/comments/add/27431 Project: Drupal Version: cvs Component: other Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Cvbge Updated by: clydefrog Status: patch I have not done a benchmark, but I would guess stat-ing a file is much less expensive then opening it up, parsing the file structure, and figuring out the dimensions. clydefrog Previous comments: ------------------------------------------------------------------------ Mon, 25 Jul 2005 11:40:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/image.inc.diff (1.14 KB) image_get_info() return information about image - width, height, extension, mime type. It's quite common to get file size (physical size) when getting dimension. This simple patch adds file_size to the returned array that contains file's size in bytes. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:20:33 +0000 : Bèr Kessels +1 for this feature. But we should be carefull with it, though. This function is called an awful lot. Cvbe, have you got experience with benchmarking? I am interested to see if this has an impact on speed and performance. If so, we could consider a lower level (inside the function) "caching": just see if the file exists in the file table, if so return those functions. But let's not keep that from including this filesize. ------------------------------------------------------------------------ Mon, 25 Jul 2005 15:11:44 +0000 : Cvbge I have no experience in benchmarking, I also have no real site that I could benchmark. If someone provides me with instructions and means for benchmarking I could do some tests. The PHP documentation for filesize says: "Note: The results of this function are cached. See clearstatcache() for more details. " grep shows that clearcache() is not used in drupal, so the cache is there. But the cache can only help in case you check the same file multiple times. The real question is how often you do image_get_info() but are not interested in file size and is filesize() expensive or not.

Issue status update for http://drupal.org/node/27431 Post a follow up: http://drupal.org/project/comments/add/27431 Project: Drupal Version: cvs Component: other Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Cvbge Updated by: clydefrog Status: patch Attachment: http://drupal.org/files/issues/image.inc_0.diff (1.14 KB) Here's an updated patch correcting a spelling error. clydefrog Previous comments: ------------------------------------------------------------------------ Mon, 25 Jul 2005 11:40:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/image.inc.diff (1.14 KB) image_get_info() return information about image - width, height, extension, mime type. It's quite common to get file size (physical size) when getting dimension. This simple patch adds file_size to the returned array that contains file's size in bytes. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:20:33 +0000 : Bèr Kessels +1 for this feature. But we should be carefull with it, though. This function is called an awful lot. Cvbe, have you got experience with benchmarking? I am interested to see if this has an impact on speed and performance. If so, we could consider a lower level (inside the function) "caching": just see if the file exists in the file table, if so return those functions. But let's not keep that from including this filesize. ------------------------------------------------------------------------ Mon, 25 Jul 2005 15:11:44 +0000 : Cvbge I have no experience in benchmarking, I also have no real site that I could benchmark. If someone provides me with instructions and means for benchmarking I could do some tests. The PHP documentation for filesize says: "Note: The results of this function are cached. See clearstatcache() for more details. " grep shows that clearcache() is not used in drupal, so the cache is there. But the cache can only help in case you check the same file multiple times. The real question is how often you do image_get_info() but are not interested in file size and is filesize() expensive or not. ------------------------------------------------------------------------ Mon, 25 Jul 2005 16:13:42 +0000 : clydefrog I have not done a benchmark, but I would guess stat-ing a file is much less expensive then opening it up, parsing the file structure, and figuring out the dimensions. ------------------------------------------------------------------------ Mon, 25 Jul 2005 16:32:03 +0000 : clydefrog image_get_info() already calls is_file(), which does a stat() and caches the result. If my understanding is correct, that means the call to filesize() will hit the cache and thus have the minimum possible performance impact.

Issue status update for http://drupal.org/node/27431 Post a follow up: http://drupal.org/project/comments/add/27431 Project: Drupal Version: cvs Component: other Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Cvbge Updated by: clydefrog Status: patch I'm sorry if my comments seemed caustic. I was merely being frank. The clearstatcache() doc page seems to imply that the PHP cache only lives for one page. But adding filesize() to image_get_info() would not change performance, as a stat() is already run for is_file(). Now, would removing is_file() help performance? I can't say. clydefrog Previous comments: ------------------------------------------------------------------------ Mon, 25 Jul 2005 11:40:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/image.inc.diff (1.14 KB) image_get_info() return information about image - width, height, extension, mime type. It's quite common to get file size (physical size) when getting dimension. This simple patch adds file_size to the returned array that contains file's size in bytes. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:20:33 +0000 : Bèr Kessels +1 for this feature. But we should be carefull with it, though. This function is called an awful lot. Cvbe, have you got experience with benchmarking? I am interested to see if this has an impact on speed and performance. If so, we could consider a lower level (inside the function) "caching": just see if the file exists in the file table, if so return those functions. But let's not keep that from including this filesize. ------------------------------------------------------------------------ Mon, 25 Jul 2005 15:11:44 +0000 : Cvbge I have no experience in benchmarking, I also have no real site that I could benchmark. If someone provides me with instructions and means for benchmarking I could do some tests. The PHP documentation for filesize says: "Note: The results of this function are cached. See clearstatcache() for more details. " grep shows that clearcache() is not used in drupal, so the cache is there. But the cache can only help in case you check the same file multiple times. The real question is how often you do image_get_info() but are not interested in file size and is filesize() expensive or not. ------------------------------------------------------------------------ Mon, 25 Jul 2005 16:13:42 +0000 : clydefrog I have not done a benchmark, but I would guess stat-ing a file is much less expensive then opening it up, parsing the file structure, and figuring out the dimensions. ------------------------------------------------------------------------ Mon, 25 Jul 2005 16:32:03 +0000 : clydefrog image_get_info() already calls is_file(), which does a stat() and caches the result. If my understanding is correct, that means the call to filesize() will hit the cache and thus have the minimum possible performance impact. ------------------------------------------------------------------------ Mon, 25 Jul 2005 16:33:36 +0000 : clydefrog Attachment: http://drupal.org/files/issues/image.inc_0.diff (1.14 KB) Here's an updated patch correcting a spelling error. ------------------------------------------------------------------------ Tue, 26 Jul 2005 07:25:42 +0000 : Bèr Kessels please do not get my comment as a "I do not lke it" :) I was merely wondering if this was going to have an impact on operformance. But from what I see now, the PHP cache will save the day. But... does that cache live on forever? Or only for one PHP run?