Closed Bug 349200 Opened 17 years ago Closed 17 years ago

[FIX]Imagelib does not usefully cache 404s

Categories

(Core :: Graphics: ImageLib, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

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?
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.
Attachment #234451 - Flags: superreview?
Attachment #234451 - Flags: review?
Attachment #234451 - Flags: superreview?(darin)
Attachment #234451 - Flags: superreview?
Attachment #234451 - Flags: review?(pavlov)
Attachment #234451 - Flags: review?
Keywords: perf
Comment on attachment 234451 [details] [diff] [review]
I think we should do this.

When does the entry get evicted from the cache?
Same place as now, I assume: when the last observer is removed we set mCacheEntry to null, after which the cache entry becomes evictable..
Attachment #234451 - Flags: superreview?(darin) → superreview+
Attachment #234451 - Flags: review?(pavlov) → review+
Assignee: pavlov → bzbarsky
Priority: -- → P2
Summary: Imagelib does not usefully cache 404s → [FIX]Imagelib does not usefully cache 404s
Target Milestone: --- → mozilla1.9alpha
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.