[drupal-devel] [feature] Please add file size infomation to image_get_info()

[Bèr Kessels]

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

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?




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.




------------------------------------------------------------------------

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.