Closed
Bug 349200
Opened 18 years ago
Closed 18 years ago
[FIX]Imagelib does not usefully cache 404s
Categories
(Core :: Graphics: ImageLib, defect, P2)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(1 file)
937 bytes,
patch
|
pavlov
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE: 1) Profile the "with <img> tags" testcase for bug 64858 2) Notice that there's a lot of image-loading-related stuff going on, in particular a lot of reframing to replace with alt text. 3) Start debugging to find out what's up. ACTUAL RESULTS: Discover that in imgRequest::OnDataAvailable if we don't find a decoder we cancel with NS_IMAGELIB_ERROR_NO_DECODER, and imgRequest::Cancel calls RemoveFromCache(). As a result, every few hundred images on that page we start a new request (because the old one got removed from cache), and when it comes in we do alt text. If we had the 404 request cached, we could do the alt text up front without need for a reframe. This would speed up the testcase by about 10% or more. Perhaps we shouldn't RemoveFromCache() if we're canceling with NS_IMAGELIB_ERROR_NO_DECODER? Or would that break something else?
Assignee | ||
Comment 1•18 years ago
|
||
So I tried my suggestion. The testcase got 25% faster; the image version is now almost as fast as the non-image one. I think we should do this.
Assignee | ||
Comment 2•18 years ago
|
||
Attachment #234451 -
Flags: superreview?
Attachment #234451 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Attachment #234451 -
Flags: superreview?(darin)
Attachment #234451 -
Flags: superreview?
Attachment #234451 -
Flags: review?(pavlov)
Attachment #234451 -
Flags: review?
Comment 3•18 years ago
|
||
Comment on attachment 234451 [details] [diff] [review] I think we should do this. When does the entry get evicted from the cache?
Assignee | ||
Comment 4•18 years ago
|
||
Same place as now, I assume: when the last observer is removed we set mCacheEntry to null, after which the cache entry becomes evictable..
Updated•18 years ago
|
Attachment #234451 -
Flags: superreview?(darin) → superreview+
Updated•18 years ago
|
Attachment #234451 -
Flags: review?(pavlov) → review+
Assignee | ||
Updated•18 years ago
|
Assignee: pavlov → bzbarsky
Priority: -- → P2
Summary: Imagelib does not usefully cache 404s → [FIX]Imagelib does not usefully cache 404s
Target Milestone: --- → mozilla1.9alpha
Assignee | ||
Comment 5•18 years ago
|
||
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•