Changeset 232139 in webkit


Ignore:
Timestamp:
May 23, 2018 5:50:16 PM (6 years ago)
Author:
ddkilzer@apple.com
Message:

Don't create the SubimageCache just to clear an image from it
<​https://webkit.org/b/185757>

Reviewed by Said Abou-Hallawa.

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.
Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r232136 r232139  
     12018-05-23  David Kilzer  <ddkilzer@apple.com>
     2
     3        Don't create the SubimageCache just to clear an image from it
     4        <https://webkit.org/b/185757>
     5
     6        Reviewed by Said Abou-Hallawa.
     7
     8        To fix this we make SubimageCacheWithTimer::clearImage() a
     9        static class method that checks whether the cache exists before
     10        removing it.  We also make SubimageCacheWithTimer::getImage() a
     11        static class method, and move more methods into the
     12        SubimageCacheWithTimer class and make them private to reduce API
     13        footprint.
     14
     15        * platform/graphics/cg/GraphicsContextCG.cpp:
     16        (WebCore::GraphicsContext::drawNativeImage): Switch to use new
     17        SubimageCacheWithTimer::getSubimage() static class method.
     18        * platform/graphics/cg/NativeImageCG.cpp:
     19        (WebCore::clearNativeImageSubimages): Switch to use new
     20        SubimageCacheWithTimer::clearImage() static class method which
     21        returns early if the subimage cache has not been created yet.
     22        This fixes the bug.
     23
     24        * platform/graphics/cg/SubimageCacheWithTimer.cpp:
     25        (WebCore::SubimageCacheWithTimer::s_cache): Allocate space for
     26        static class variable.
     27        (WebCore::SubimageCacheWithTimer::getSubimage): Replace instance
     28        method with new static class method that gets the subimage cache
     29        singleton and calls the subimage() instance method.
     30        (WebCore::SubimageCacheWithTimer::clearImage): Replace instance
     31        methdod with new static class method that returns early if the
     32        static cache singleton doesn't exist (fixes the bug), otherwise
     33        calls the clearImageAndSubimages() instance method.
     34        (WebCore::SubimageCacheWithTimer::subimage): Rename from
     35        getSubimage().  Use `auto` after renaming SubimageCache typedef
     36        to SubimageCacheHashSet.
     37        (WebCore::SubimageCacheWithTimer::clearImageAndSubimages):
     38        Rename from clearImage().  Modernize loops.
     39        (WebCore::SubimageCacheWithTimer::subimageCache): Change
     40        WebCore::subimageCache() to a static class method that creates
     41        the subimage cache singleton if it doesn't exist yet, and
     42        returns it.
     43        (WebCore::SubimageCacheWithTimer::subimageCacheExists): Add.
     44        Returns false if the subimage cache singleton has not been
     45        created yet.
     46
     47        * platform/graphics/cg/SubimageCacheWithTimer.h:
     48        - Rename typedef SubimageCache to SubimageCacheHashSet to avoid
     49          general confusion.
     50        (WebCore::SubimageCacheWithTimer::getSubimage):
     51        (WebCore::SubimageCacheWithTimer::clearImage):
     52        - Change to static class methods.
     53        (WebCore::SubimageCacheWithTimer::SubimageCacheWithTimer):
     54        - Make private.
     55        (WebCore::SubimageCacheWithTimer::subimage):
     56        - Rename from getSubimage() and make private.
     57        (WebCore::SubimageCacheWithTimer::clearImageAndSubimages):
     58        - Rename from clearImage() and make private.
     59        (WebCore::SubimageCacheWithTimer::subimageCache):
     60        - Rename from WebCore::subimageCache() and make a private static
     61          class method.
     62        (WebCore::SubimageCacheWithTimer::subimageCacheExists):
     63        - Add private static class method.
     64        (WebCore::SubimageCacheWithTimer::s_cache):
     65        - Declare private static variable to hold singleton.
     66
    1672018-05-23  Eric Carlson  <eric.carlson@apple.com>
    268

  • trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp

    r229209 r232139  
    331331
    332332#if CACHE_SUBIMAGES
    333             subImage = subimageCache().getSubimage(subImage.get(), subimageRect);
     333            subImage = SubimageCacheWithTimer::getSubimage(subImage.get(), subimageRect);
    334334#else
    335335            subImage = adoptCF(CGImageCreateWithImageInRect(subImage.get(), subimageRect));

  • trunk/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp

    r220506 r232139  
    8787#if CACHE_SUBIMAGES
    8888    if (image)
    89         subimageCache().clearImage(image.get());
     89        SubimageCacheWithTimer::clearImage(image.get());
    9090#endif
    9191}

  • trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp

    r215152 r232139  
    3535namespace WebCore {
    3636
     37SubimageCacheWithTimer* SubimageCacheWithTimer::s_cache;
     38
    3739static const Seconds subimageCacheClearDelay { 1_s };
    3840static const int maxSubimageCacheSize = 300;
     41
     42RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, const FloatRect& rect)
     43{
     44    return subimageCache().subimage(image, rect);
     45}
     46
     47void SubimageCacheWithTimer::clearImage(CGImageRef image)
     48{
     49    if (subimageCacheExists())
     50        subimageCache().clearImageAndSubimages(image);
     51}
    3952
    4053struct SubimageRequest {
    … …  
    7487}
    7588
    76 RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, const FloatRect& rect)
     89RetainPtr<CGImageRef> SubimageCacheWithTimer::subimage(CGImageRef image, const FloatRect& rect)
    7790{
    7891    m_timer.restart();
    … …  
    8497
    8598    ASSERT(m_cache.size() < maxSubimageCacheSize);
    86     SubimageCache::AddResult result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
     99    auto result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
    87100    if (result.isNewEntry)
    88101        m_images.add(image);
    … …  
    91104}
    92105
    93 void SubimageCacheWithTimer::clearImage(CGImageRef image)
     106void SubimageCacheWithTimer::clearImageAndSubimages(CGImageRef image)
    94107{
    95108    if (m_images.contains(image)) {
    96109        Vector<SubimageCacheEntry> toBeRemoved;
    97         SubimageCache::const_iterator end = m_cache.end();
    98         for (SubimageCache::const_iterator it = m_cache.begin(); it != end; ++it) {
    99             if (it->image.get() == image)
    100                 toBeRemoved.append(*it);
     110        for (const auto& entry : m_cache) {
     111            if (entry.image.get() == image)
     112                toBeRemoved.append(entry);
    101113        }
    102114
    103         for (Vector<SubimageCacheEntry>::iterator removeIt = toBeRemoved.begin(); removeIt != toBeRemoved.end(); ++removeIt)
    104             m_cache.remove(*removeIt);
     115        for (auto& entry : toBeRemoved)
     116            m_cache.remove(entry);
    105117
    106118        m_images.removeAll(image);
    … …  
    108120}
    109121
    110 SubimageCacheWithTimer& subimageCache()
     122SubimageCacheWithTimer& SubimageCacheWithTimer::subimageCache()
    111123{
    112     static SubimageCacheWithTimer& cache = *new SubimageCacheWithTimer;
    113     return cache;
     124    if (!s_cache)
     125        s_cache = new SubimageCacheWithTimer;
     126    return *s_cache;
     127}
     128
     129bool SubimageCacheWithTimer::subimageCacheExists()
     130{
     131    return !!s_cache;
    114132}
    115133

  • trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h

    r169550 r232139  
    8181    };
    8282
    83     typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCache;
    84 
    85 public:
    86     SubimageCacheWithTimer();
    87     RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&);
    88     void clearImage(CGImageRef);
     83    static RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&);
     84    static void clearImage(CGImageRef);
    8985
    9086private:
     87    typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCacheHashSet;
     88
     89    SubimageCacheWithTimer();
    9190    void invalidateCacheTimerFired();
    9291
     92    RetainPtr<CGImageRef> subimage(CGImageRef, const FloatRect&);
     93    void clearImageAndSubimages(CGImageRef);
     94
    9395    HashCountedSet<CGImageRef> m_images;
    94     SubimageCache m_cache;
     96    SubimageCacheHashSet m_cache;
    9597    DeferrableOneShotTimer m_timer;
     98
     99    static SubimageCacheWithTimer& subimageCache();
     100    static bool subimageCacheExists();
     101    static SubimageCacheWithTimer* s_cache;
    96102};
    97 
    98 SubimageCacheWithTimer& subimageCache();
    99103
    100104#endif // CACHE_SUBIMAGES
Note: See TracChangeset for help on using the changeset viewer.