WebKit Bugzilla
Attachment 341101 Details for
Bug 185757
: Don't create the SubimageCache just to clear an image from it
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch v4
bug-185757-20180523101820.patch (text/plain), 9.55 KB, created by
David Kilzer (:ddkilzer)
on 2018-05-23 10:18:21 PDT
(
hide
)
Description:
Patch v4
Filename:
MIME Type:
Creator:
David Kilzer (:ddkilzer)
Created:
2018-05-23 10:18:21 PDT
Size:
9.55 KB
patch
obsolete
>Subversion Revision: 231940 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 385dd61ab755a5d049b333bcb2ce9f0b359a9764..5c558f0f3f8854490782db966894708c6bb9f1cc 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,69 @@ >+2018-05-23 David Kilzer <ddkilzer@apple.com> >+ >+ Don't create the SubimageCache just to clear an image from it >+ <https://webkit.org/b/185757> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ To fix this we make SubimageCacheWithTimer::clearImage() a >+ static class method that checks whether the cache exists before >+ removing it. We also make SubimageCacheWithTimer::getImage() a >+ static class method, and move more methods into the >+ SubimageCacheWithTimer class and make them private to reduce API >+ footprint. >+ >+ * platform/graphics/cg/GraphicsContextCG.cpp: >+ (WebCore::GraphicsContext::drawNativeImage): Switch to use new >+ SubimageCacheWithTimer::getSubimage() static class method. >+ * platform/graphics/cg/NativeImageCG.cpp: >+ (WebCore::clearNativeImageSubimages): Switch to use new >+ SubimageCacheWithTimer::clearImage() static class method which >+ returns early if the subimage cache has not been created yet. >+ This fixes the bug. >+ >+ * platform/graphics/cg/SubimageCacheWithTimer.cpp: >+ (WebCore::SubimageCacheWithTimer::s_cache): Allocate space for >+ static class variable. >+ (WebCore::SubimageCacheWithTimer::getSubimage): Replace instance >+ method with new static class method that gets the subimage cache >+ singleton and calls the subimage() instance method. >+ (WebCore::SubimageCacheWithTimer::clearImage): Replace instance >+ methdod with new static class method that returns early if the >+ static cache singleton doesn't exist (fixes the bug), otherwise >+ calls the clearImageAndSubimages() instance method. >+ (WebCore::SubimageCacheWithTimer::subimage): Rename from >+ getSubimage(). Use `auto` after renaming SubimageCache typedef >+ to SubimageCacheHashSet. >+ (WebCore::SubimageCacheWithTimer::clearImageAndSubimages): >+ Rename from clearImage(). Modernize loops. >+ (WebCore::SubimageCacheWithTimer::subimageCache): Change >+ WebCore::subimageCache() to a static class method that creates >+ the subimage cache singleton if it doesn't exist yet, and >+ returns it. >+ (WebCore::SubimageCacheWithTimer::subimageCacheExists): Add. >+ Returns false if the subimage cache singleton has not been >+ created yet. >+ >+ * platform/graphics/cg/SubimageCacheWithTimer.h: >+ - Rename typedef SubimageCache to SubimageCacheHashSet to avoid >+ general confusion. >+ (WebCore::SubimageCacheWithTimer::getSubimage): >+ (WebCore::SubimageCacheWithTimer::clearImage): >+ - Change to static class methods. >+ (WebCore::SubimageCacheWithTimer::SubimageCacheWithTimer): >+ - Make private. >+ (WebCore::SubimageCacheWithTimer::subimage): >+ - Rename from getSubimage() and make private. >+ (WebCore::SubimageCacheWithTimer::clearImageAndSubimages): >+ - Rename from clearImage() and make private. >+ (WebCore::SubimageCacheWithTimer::subimageCache): >+ - Rename from WebCore::subimageCache() and make a private static >+ class method. >+ (WebCore::SubimageCacheWithTimer::subimageCacheExists): >+ - Add private static class method. >+ (WebCore::SubimageCacheWithTimer::s_cache): >+ - Declare private static variable to hold singleton. >+ > 2018-05-17 David Kilzer <ddkilzer@apple.com> > > Lazily create WebCore::Timer for WebCore::Image >diff --git a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp >index 895c90ed10c13eaed66558f2ab78dcb9d6e77654..0b89d20c0fe4b06fa20426d503753955a8d919fa 100644 >--- a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp >+++ b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp >@@ -330,7 +330,7 @@ void GraphicsContext::drawNativeImage(const RetainPtr<CGImageRef>& image, const > adjustedDestRect.setHeight(subimageRect.height() / yScale); > > #if CACHE_SUBIMAGES >- subImage = subimageCache().getSubimage(subImage.get(), subimageRect); >+ subImage = SubimageCacheWithTimer::getSubimage(subImage.get(), subimageRect); > #else > subImage = adoptCF(CGImageCreateWithImageInRect(subImage.get(), subimageRect)); > #endif >diff --git a/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp b/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp >index b08026bd1d77d2e2c5e4803832f65a1fd4fc68c9..987536bc88307506bee224292ef79ffad88b5938 100644 >--- a/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp >+++ b/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp >@@ -86,7 +86,7 @@ void clearNativeImageSubimages(const NativeImagePtr& image) > { > #if CACHE_SUBIMAGES > if (image) >- subimageCache().clearImage(image.get()); >+ SubimageCacheWithTimer::clearImage(image.get()); > #endif > } > >diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp >index 33923ec49011ac34ebf9d0751c9708617c138aaa..cfde29f1bf3930c6b4a54b92e9ba4fd33810df9d 100644 >--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp >+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp >@@ -34,9 +34,22 @@ > > namespace WebCore { > >+SubimageCacheWithTimer* SubimageCacheWithTimer::s_cache; >+ > static const Seconds subimageCacheClearDelay { 1_s }; > static const int maxSubimageCacheSize = 300; > >+RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, const FloatRect& rect) >+{ >+ return subimageCache().subimage(image, rect); >+} >+ >+void SubimageCacheWithTimer::clearImage(CGImageRef image) >+{ >+ if (subimageCacheExists()) >+ subimageCache().clearImageAndSubimages(image); >+} >+ > struct SubimageRequest { > CGImageRef image; > const FloatRect& rect; >@@ -73,7 +86,7 @@ void SubimageCacheWithTimer::invalidateCacheTimerFired() > m_cache.clear(); > } > >-RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, const FloatRect& rect) >+RetainPtr<CGImageRef> SubimageCacheWithTimer::subimage(CGImageRef image, const FloatRect& rect) > { > m_timer.restart(); > if (m_cache.size() == maxSubimageCacheSize) { >@@ -83,34 +96,39 @@ RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, cons > } > > ASSERT(m_cache.size() < maxSubimageCacheSize); >- SubimageCache::AddResult result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect)); >+ auto result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect)); > if (result.isNewEntry) > m_images.add(image); > > return result.iterator->subimage; > } > >-void SubimageCacheWithTimer::clearImage(CGImageRef image) >+void SubimageCacheWithTimer::clearImageAndSubimages(CGImageRef image) > { > if (m_images.contains(image)) { > Vector<SubimageCacheEntry> toBeRemoved; >- SubimageCache::const_iterator end = m_cache.end(); >- for (SubimageCache::const_iterator it = m_cache.begin(); it != end; ++it) { >- if (it->image.get() == image) >- toBeRemoved.append(*it); >+ for (const auto& entry : m_cache) { >+ if (entry.image.get() == image) >+ toBeRemoved.append(entry); > } > >- for (Vector<SubimageCacheEntry>::iterator removeIt = toBeRemoved.begin(); removeIt != toBeRemoved.end(); ++removeIt) >- m_cache.remove(*removeIt); >+ for (auto& entry : toBeRemoved) >+ m_cache.remove(entry); > > m_images.removeAll(image); > } > } > >-SubimageCacheWithTimer& subimageCache() >+SubimageCacheWithTimer& SubimageCacheWithTimer::subimageCache() >+{ >+ if (!s_cache) >+ s_cache = new SubimageCacheWithTimer; >+ return *s_cache; >+} >+ >+bool SubimageCacheWithTimer::subimageCacheExists() > { >- static SubimageCacheWithTimer& cache = *new SubimageCacheWithTimer; >- return cache; >+ return !!s_cache; > } > > } >diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h >index 9033e797372335afffc712dd36f71ce30593af54..fd6dfe7583f99a1557798f92bbf655ac8a8cc778 100644 >--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h >+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h >@@ -80,22 +80,26 @@ public: > static const bool safeToCompareToEmptyOrDeleted = true; > }; > >- typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCache; >+ typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCacheHashSet; > >-public: >- SubimageCacheWithTimer(); >- RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&); >- void clearImage(CGImageRef); >+ static RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&); >+ static void clearImage(CGImageRef); > > private: >+ SubimageCacheWithTimer(); > void invalidateCacheTimerFired(); > >+ RetainPtr<CGImageRef> subimage(CGImageRef, const FloatRect&); >+ void clearImageAndSubimages(CGImageRef); >+ > HashCountedSet<CGImageRef> m_images; >- SubimageCache m_cache; >+ SubimageCacheHashSet m_cache; > DeferrableOneShotTimer m_timer; >-}; > >-SubimageCacheWithTimer& subimageCache(); >+ static SubimageCacheWithTimer& subimageCache(); >+ static bool subimageCacheExists(); >+ static SubimageCacheWithTimer* s_cache; >+}; > > #endif // CACHE_SUBIMAGES >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 185757
:
340676
|
340700
|
340863
|
340952
| 341101