Please add file size infomation to image_get_info() [#27431]

Comment	File	Size	Author
#5	image.inc_0.diff	1.14 KB	ezheidtmann

	image.inc.diff	1.14 KB	Cvbge

Comment #1

Bèr Kessels CreditAttribution: Bèr Kessels commented 25 July 2005 at 13:20

+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.

Log in or register to post comments

Comment #2

Cvbge CreditAttribution: Cvbge commented 25 July 2005 at 15:11

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.

Log in or register to post comments

Comment #3

ezheidtmann CreditAttribution: ezheidtmann commented 25 July 2005 at 16:13

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.

Log in or register to post comments

Comment #4

ezheidtmann CreditAttribution: ezheidtmann commented 25 July 2005 at 16:32

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.

Log in or register to post comments

Comment #5

ezheidtmann CreditAttribution: ezheidtmann commented 25 July 2005 at 16:33

File	Size
image.inc_0.diff	1.14 KB

Here's an updated patch correcting a spelling error.

Log in or register to post comments

Comment #6

Bèr Kessels CreditAttribution: Bèr Kessels commented 26 July 2005 at 07:25

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?

Log in or register to post comments

Comment #7

ezheidtmann CreditAttribution: ezheidtmann commented 26 July 2005 at 16:22

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.