Closed Bug 1183852 Opened 5 years ago Closed 5 years ago
Only mark surfaces as used in the Surface
Cache if a caller requested exactly that surface
The SurfaceCache is intended to only consider a surface used if some caller is actually, well, using it. However, that's not what's happening in practice, because: (1) SurfaceCacheImpl::Insert calls SurfaceCacheImpl::Lookup, which marks the looked-up surface used. This is incorrect because Insert() is primarily called by decoders, and if we happen to create multiple decoders for the same SurfaceKey, the decoders which lose the race to create the surface should not mark that surface as used. (2) SurfaceCacheImpl::LookupBestMatch marks surfaces as used whether it's returning an exact match or not. This is a bad idea, because it can greatly increase the lifetime of a surface that we no longer need, just because we happened to call Draw() one time before the new surface finished decoding. Let's fix these issues.
Here's the patch. SurfaceCacheImpl::Lookup() gets a new boolean aMarkUsed parameter that determines whether it should mark the resulting surface used, and we pass false for this parameter in SurfaceCacheImpl::Insert(). SurfaceCacheImpl::LookupBestMatch() only marks the resulting surface used if it's an exact match. To avoid some ugly nested |if| constructs, I've gone ahead and moved the code that marks a surface used into its own private method, SurfaceCacheImpl::MarkUsed(). This seemed too trivial for its own patch.
Attachment #8633698 - Flags: review?(dholbert)
Comment on attachment 8633698 [details] [diff] [review] Only mark surfaces as used in the SurfaceCache if a caller requested exactly that surface r=me
Attachment #8633698 - Flags: review?(dholbert) → review+
Thanks for the review!
You need to log in before you can comment on or make changes to this bug.