Changeset 76866 in webkit


Ignore:
Timestamp:
Jan 27, 2011 4:58:02 PM (13 years ago)
Author:
senorblanco@chromium.org
Message:

2011-01-27 Stephen White <​senorblanco@chromium.org>

Reviewed by Darin Adler.

Fix performance regression in ImageQualityController::objectDestroyed().
​https://bugs.webkit.org/show_bug.cgi?id=52645

In r72282, I inadvertently introduced this regression by using a
linear search through the hash map on object destruction. This was
because the hash key consisted of both object pointer and layer id,
but on object destruction we only know the object pointer, requiring
a search to find all the layers.
By replacing the hash map with two nested hash maps, where the outer key
is the object and the inner key is the layer, we can find all the
relevant data for an object in one hash lookup.

  • rendering/RenderBoxModelObject.cpp: Replace the (object,layer)->size HashMap with object->layer and layer->size HashMaps. (WebCore::ImageQualityController::isEmpty): Implement isEmpty() for the outer HashMap. (WebCore::ImageQualityController::removeLayer): When a layer is removed, remove it from the inner hash map. (WebCore::ImageQualityController::set): Implement set(): if the inner map exists, set the layer->size tuple directly. If not, create a new inner map, set the tuple, and insert it in the outer map. (WebCore::ImageQualityController::objectDestroyed): Look up the object in the outer map only. (WebCore::ImageQualityController::highQualityRepaintTimerFired): Cosmetic changes for the renamed now-outer hash map. (WebCore::ImageQualityController::shouldPaintAtLowQuality): Do both outer and inner hash map lookups. Call set() to add/update entries to the hash maps. keyDestroyed() is now removeLayer(). (WebCore::imageQualityController): Make the ImageQualityController a file-static global, so it can be created and destroyed on the fly. (WebCore::RenderBoxModelObject::~RenderBoxModelObject): If there is no ImageQualityController, don't call objectDestroyed(). If it's empty, delete it.
  • rendering/RenderImage.cpp: (WebCore::RenderImage::paintIntoRect): Also pass the Image* as the (void*) layer, since 0 is not a valid HashMap key.
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r76864 r76866  
     12011-01-27  Stephen White  <senorblanco@chromium.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Fix performance regression in ImageQualityController::objectDestroyed().
     6        https://bugs.webkit.org/show_bug.cgi?id=52645
     7
     8        In r72282, I inadvertently introduced this regression by using a
     9        linear search through the hash map on object destruction.  This was
     10        because the hash key consisted of both object pointer and layer id,
     11        but on object destruction we only know the object pointer, requiring
     12        a search to find all the layers.
     13        By replacing the hash map with two nested hash maps, where the outer key
     14        is the object and the inner key is the layer, we can find all the
     15        relevant data for an object in one hash lookup.
     16
     17        * rendering/RenderBoxModelObject.cpp:
     18        Replace the (object,layer)->size HashMap with object->layer and
     19        layer->size HashMaps.
     20        (WebCore::ImageQualityController::isEmpty):
     21        Implement isEmpty() for the outer HashMap.
     22        (WebCore::ImageQualityController::removeLayer):
     23        When a layer is removed, remove it from the inner hash map.
     24        (WebCore::ImageQualityController::set):
     25        Implement set():  if the inner map exists, set the layer->size tuple
     26        directly.  If not, create a new inner map, set the tuple, and insert
     27        it in the outer map.
     28        (WebCore::ImageQualityController::objectDestroyed):
     29        Look up the object in the outer map only.
     30        (WebCore::ImageQualityController::highQualityRepaintTimerFired):
     31        Cosmetic changes for the renamed now-outer hash map.
     32        (WebCore::ImageQualityController::shouldPaintAtLowQuality):
     33        Do both outer and inner hash map lookups.  Call set() to add/update
     34        entries to the hash maps.  keyDestroyed() is now removeLayer().
     35        (WebCore::imageQualityController):
     36        Make the ImageQualityController a file-static global, so it can be
     37        created and destroyed on the fly.
     38        (WebCore::RenderBoxModelObject::~RenderBoxModelObject):
     39        If there is no ImageQualityController, don't call objectDestroyed().
     40        If it's empty, delete it.
     41        * rendering/RenderImage.cpp:
     42        (WebCore::RenderImage::paintIntoRect):
     43        Also pass the Image* as the (void*) layer, since 0 is not a valid
     44        HashMap key.
     45
    1462011-01-27  Adrienne Walker  <enne@google.com>
    247

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r76836 r76866  
    5151static const double cLowQualityTimeThreshold = 0.500; // 500 ms
    5252
    53 typedef pair<RenderBoxModelObject*, const void*> LastPaintSizeMapKey;
    54 typedef HashMap<LastPaintSizeMapKey, IntSize> LastPaintSizeMap;
     53typedef HashMap<const void*, IntSize> LayerSizeMap;
     54typedef HashMap<RenderBoxModelObject*, LayerSizeMap> ObjectLayerSizeMap;
    5555
    5656// The HashMap for storing continuation pointers.
    … …  
    6969    ImageQualityController();
    7070    bool shouldPaintAtLowQuality(GraphicsContext*, RenderBoxModelObject*, Image*, const void* layer, const IntSize&);
    71     void keyDestroyed(LastPaintSizeMapKey key);
     71    void removeLayer(RenderBoxModelObject*, LayerSizeMap* innerMap, const void* layer);
     72    void set(RenderBoxModelObject*, LayerSizeMap* innerMap, const void* layer, const IntSize&);
    7273    void objectDestroyed(RenderBoxModelObject*);
     74    bool isEmpty() { return m_objectLayerSizeMap.isEmpty(); }
    7375
    7476private:
    … …  
    7678    void restartTimer();
    7779
    78     LastPaintSizeMap m_lastPaintSizeMap;
     80    ObjectLayerSizeMap m_objectLayerSizeMap;
    7981    Timer<ImageQualityController> m_timer;
    8082    bool m_animatedResizeIsActive;
    … …  
    8789}
    8890
    89 void ImageQualityController::keyDestroyed(LastPaintSizeMapKey key)
    90 {
    91     m_lastPaintSizeMap.remove(key);
    92     if (m_lastPaintSizeMap.isEmpty()) {
     91void ImageQualityController::removeLayer(RenderBoxModelObject* object, LayerSizeMap* innerMap, const void* layer)
     92{
     93    if (innerMap) {
     94        innerMap->remove(layer);
     95        if (innerMap->isEmpty())
     96            objectDestroyed(object);
     97    }
     98}
     99   
     100void ImageQualityController::set(RenderBoxModelObject* object, LayerSizeMap* innerMap, const void* layer, const IntSize& size)
     101{
     102    if (innerMap)
     103        innerMap->set(layer, size);
     104    else {
     105        LayerSizeMap newInnerMap;
     106        newInnerMap.set(layer, size);
     107        m_objectLayerSizeMap.set(object, newInnerMap);
     108    }
     109}
     110   
     111void ImageQualityController::objectDestroyed(RenderBoxModelObject* object)
     112{
     113    m_objectLayerSizeMap.remove(object);
     114    if (m_objectLayerSizeMap.isEmpty()) {
    93115        m_animatedResizeIsActive = false;
    94116        m_timer.stop();
    95117    }
    96118}
    97    
    98 void ImageQualityController::objectDestroyed(RenderBoxModelObject* object)
    99 {
    100     Vector<LastPaintSizeMapKey> keysToDie;
    101     for (LastPaintSizeMap::iterator it = m_lastPaintSizeMap.begin(); it != m_lastPaintSizeMap.end(); ++it)
    102         if (it->first.first == object)
    103             keysToDie.append(it->first);
    104     for (Vector<LastPaintSizeMapKey>::iterator it = keysToDie.begin(); it != keysToDie.end(); ++it)
    105         keyDestroyed(*it);
    106 }
    107    
     119
    108120void ImageQualityController::highQualityRepaintTimerFired(Timer<ImageQualityController>*)
    109121{
    110122    if (m_animatedResizeIsActive) {
    111123        m_animatedResizeIsActive = false;
    112         for (LastPaintSizeMap::iterator it = m_lastPaintSizeMap.begin(); it != m_lastPaintSizeMap.end(); ++it)
    113             it->first.first->repaint();
     124        for (ObjectLayerSizeMap::iterator it = m_objectLayerSizeMap.begin(); it != m_objectLayerSizeMap.end(); ++it)
     125            it->first->repaint();
    114126    }
    115127}
    … …  
    131143    IntSize imageSize(image->width(), image->height());
    132144
    133     // Look ourselves up in the hashtable.
    134     LastPaintSizeMapKey key(object, layer);
    135     LastPaintSizeMap::iterator i = m_lastPaintSizeMap.find(key);
     145    // Look ourselves up in the hashtables.
     146    ObjectLayerSizeMap::iterator i = m_objectLayerSizeMap.find(object);
     147    LayerSizeMap* innerMap = i != m_objectLayerSizeMap.end() ? &i->second : 0;
     148    IntSize oldSize;
     149    bool isFirstResize = true;
     150    if (innerMap) {
     151        LayerSizeMap::iterator j = innerMap->find(layer);
     152        if (j != innerMap->end()) {
     153            isFirstResize = false;
     154            oldSize = j->second;
     155        }
     156    }
    136157
    137158    const AffineTransform& currentTransform = context->getCTM();
    … …  
    139160    if (!contextIsScaled && imageSize == size) {
    140161        // There is no scale in effect. If we had a scale in effect before, we can just remove this object from the list.
    141         if (i != m_lastPaintSizeMap.end())
    142             m_lastPaintSizeMap.remove(key);
    143 
     162        removeLayer(object, innerMap, layer);
    144163        return false;
    145164    }
    … …  
    151170            return true;
    152171    }
     172
    153173    // If an animated resize is active, paint in low quality and kick the timer ahead.
    154174    if (m_animatedResizeIsActive) {
    155         m_lastPaintSizeMap.set(key, size);
     175        set(object, innerMap, layer, size);
    156176        restartTimer();
    157177        return true;
    … …  
    160180    // same as the last resize, draw at high res, but record the paint
    161181    // size and set the timer.
    162     if (i == m_lastPaintSizeMap.end() || size == i->second) {
     182    if (isFirstResize || oldSize == size) {
    163183        restartTimer();
    164         m_lastPaintSizeMap.set(key, size);
     184        set(object, innerMap, layer, size);
    165185        return false;
    166186    }
    … …  
    168188    // set the timer.
    169189    if (!m_timer.isActive()) {
    170         keyDestroyed(key);
     190        removeLayer(object, innerMap, layer);
    171191        return false;
    172192    }
    … …  
    174194    // is active, so draw at low quality, set the flag for animated resizes and
    175195    // the object to the list for high quality redraw.
    176     m_lastPaintSizeMap.set(key, size);
     196    set(object, innerMap, layer, size);
    177197    m_animatedResizeIsActive = true;
    178198    restartTimer();
    … …  
    180200}
    181201
     202static ImageQualityController* gImageQualityController = 0;
     203
    182204static ImageQualityController* imageQualityController()
    183205{
    184     static ImageQualityController* controller = new ImageQualityController;
    185     return controller;
     206    if (!gImageQualityController)
     207        gImageQualityController = new ImageQualityController;
     208
     209    return gImageQualityController;
    186210}
    187211
    … …  
    224248    ASSERT(!hasLayer());
    225249    ASSERT(!m_layer);
    226     imageQualityController()->objectDestroyed(this);
     250    if (gImageQualityController) {
     251        gImageQualityController->objectDestroyed(this);
     252        if (gImageQualityController->isEmpty()) {
     253            delete gImageQualityController;
     254            gImageQualityController = 0;
     255        }
     256    }
    227257}
    228258

  • trunk/Source/WebCore/rendering/RenderImage.cpp

    r76571 r76866  
    377377    HTMLImageElement* imageElt = (node() && node()->hasTagName(imgTag)) ? static_cast<HTMLImageElement*>(node()) : 0;
    378378    CompositeOperator compositeOperator = imageElt ? imageElt->compositeOperator() : CompositeSourceOver;
    379     bool useLowQualityScaling = shouldPaintAtLowQuality(context, m_imageResource->image().get(), 0, rect.size());
     379    Image* image = m_imageResource->image().get();
     380    bool useLowQualityScaling = shouldPaintAtLowQuality(context, image, image, rect.size());
    380381    context->drawImage(m_imageResource->image(rect.width(), rect.height()).get(), style()->colorSpace(), rect, compositeOperator, useLowQualityScaling);
    381382}
Note: See TracChangeset for help on using the changeset viewer.