Closed Bug 1397304 Opened 2 years ago Closed 2 years ago

imgLoader could avoid calling imgCacheQueue::Remove() after popping entry from queue CheckCacheLimits()

Categories

(Core :: ImageLib, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

Details

Attachments

(1 file)

After bug 1396870 the vnc_playback.html test case seems much better:

https://perfht.ml/2w7FFwQ

One thing that does still show up here, though, is that we effectively do:

1. imgLoader::CheckCacheLimits() calls imgCacheQueue::Pop()
2. Then it calls imgLoader::RemoveFromCache()
3. imgLoader::RemoveFromCache() then calls imgCacheQueue::Remove()

Step 3 is somewhat wasteful here because we 100% know the entry is no longer in the queue since we just popped it off the queue.  Functionally these calls are ok, but step 3 forces us to do an O(n) search across the entire queue each time.

This only shows up in the profile a few times, but its a minor perf penalty we could easily avoid by passing a hint to RemoveFromCache().
This patch adds an enumeration to hint to RemoveFromCache(entry) whether the entry may or may not be in the queue.  If we know its not in the queue we can avoid iterating the entire queue trying to remove it.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=edd46bd62e69027e762cd18dae23cbe1d9186ddb
Comment on attachment 8905100 [details] [diff] [review]
Avoid searching the image cache queue for an entry after we just popped it off the queue. r=tnikkel

See comment 1 for the patch description.
Attachment #8905100 - Flags: review?(tnikkel)
Attachment #8905100 - Flags: review?(tnikkel) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/615eb9ac1663
Avoid searching the image cache queue for an entry after we just popped it off the queue. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/615eb9ac1663
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.