[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

senorblanco at chromium.org senorblanco at chromium.org
Mon Feb 21 00:02:50 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 118a71a1c21d15485d548f85dae8fba249aa42bf
Author: senorblanco at chromium.org <senorblanco at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Jan 27 19:58:33 2011 +0000

    2011-01-19  Stephen White  <senorblanco at 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.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76825 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6c686df..3eeb31e 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,45 @@
+2011-01-19  Stephen White  <senorblanco at 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.
+
+
 2011-01-26  Enrica Casucci  <enrica at apple.com>
 
         Reviewed by Darin Adler and Adam Roben.
diff --git a/Source/WebCore/rendering/RenderBoxModelObject.cpp b/Source/WebCore/rendering/RenderBoxModelObject.cpp
index 7e89361..40acd03 100644
--- a/Source/WebCore/rendering/RenderBoxModelObject.cpp
+++ b/Source/WebCore/rendering/RenderBoxModelObject.cpp
@@ -50,8 +50,8 @@ bool RenderBoxModelObject::s_layerWasSelfPainting = false;
 static const double cInterpolationCutoff = 800. * 800.;
 static const double cLowQualityTimeThreshold = 0.500; // 500 ms
 
-typedef pair<RenderBoxModelObject*, const void*> LastPaintSizeMapKey;
-typedef HashMap<LastPaintSizeMapKey, IntSize> LastPaintSizeMap;
+typedef HashMap<const void*, IntSize> LayerSizeMap;
+typedef HashMap<RenderBoxModelObject*, LayerSizeMap> ObjectLayerSizeMap;
 
 // The HashMap for storing continuation pointers.
 // An inline can be split with blocks occuring in between the inline content.
@@ -68,14 +68,16 @@ class ImageQualityController {
 public:
     ImageQualityController();
     bool shouldPaintAtLowQuality(GraphicsContext*, RenderBoxModelObject*, Image*, const void* layer, const IntSize&);
-    void keyDestroyed(LastPaintSizeMapKey key);
+    void removeLayer(RenderBoxModelObject*, LayerSizeMap* innerMap, const void* layer);
+    void set(RenderBoxModelObject*, LayerSizeMap* innerMap, const void* layer, const IntSize&);
     void objectDestroyed(RenderBoxModelObject*);
+    bool isEmpty() { return m_objectLayerSizeMap.isEmpty(); }
 
 private:
     void highQualityRepaintTimerFired(Timer<ImageQualityController>*);
     void restartTimer();
 
-    LastPaintSizeMap m_lastPaintSizeMap;
+    ObjectLayerSizeMap m_objectLayerSizeMap;
     Timer<ImageQualityController> m_timer;
     bool m_animatedResizeIsActive;
 };
@@ -86,31 +88,41 @@ ImageQualityController::ImageQualityController()
 {
 }
 
-void ImageQualityController::keyDestroyed(LastPaintSizeMapKey key)
+void ImageQualityController::removeLayer(RenderBoxModelObject* object, LayerSizeMap* innerMap, const void* layer)
 {
-    m_lastPaintSizeMap.remove(key);
-    if (m_lastPaintSizeMap.isEmpty()) {
-        m_animatedResizeIsActive = false;
-        m_timer.stop();
+    if (innerMap) {
+        innerMap->remove(layer);
+        if (innerMap->isEmpty())
+            objectDestroyed(object);
     }
 }
     
-void ImageQualityController::objectDestroyed(RenderBoxModelObject* object)
+void ImageQualityController::set(RenderBoxModelObject* object, LayerSizeMap* innerMap, const void* layer, const IntSize& size)
 {
-    Vector<LastPaintSizeMapKey> keysToDie;
-    for (LastPaintSizeMap::iterator it = m_lastPaintSizeMap.begin(); it != m_lastPaintSizeMap.end(); ++it)
-        if (it->first.first == object)
-            keysToDie.append(it->first);
-    for (Vector<LastPaintSizeMapKey>::iterator it = keysToDie.begin(); it != keysToDie.end(); ++it)
-        keyDestroyed(*it);
+    if (innerMap)
+        innerMap->set(layer, size);
+    else {
+        LayerSizeMap newInnerMap;
+        newInnerMap.set(layer, size);
+        m_objectLayerSizeMap.set(object, newInnerMap);
+    }
 }
     
+void ImageQualityController::objectDestroyed(RenderBoxModelObject* object)
+{
+    m_objectLayerSizeMap.remove(object);
+    if (m_objectLayerSizeMap.isEmpty()) {
+        m_animatedResizeIsActive = false;
+        m_timer.stop();
+    }
+}
+
 void ImageQualityController::highQualityRepaintTimerFired(Timer<ImageQualityController>*)
 {
     if (m_animatedResizeIsActive) {
         m_animatedResizeIsActive = false;
-        for (LastPaintSizeMap::iterator it = m_lastPaintSizeMap.begin(); it != m_lastPaintSizeMap.end(); ++it)
-            it->first.first->repaint();
+        for (ObjectLayerSizeMap::iterator it = m_objectLayerSizeMap.begin(); it != m_objectLayerSizeMap.end(); ++it)
+            it->first->repaint();
     }
 }
 
@@ -130,17 +142,24 @@ bool ImageQualityController::shouldPaintAtLowQuality(GraphicsContext* context, R
     // is actually being scaled.
     IntSize imageSize(image->width(), image->height());
 
-    // Look ourselves up in the hashtable.
-    LastPaintSizeMapKey key(object, layer);
-    LastPaintSizeMap::iterator i = m_lastPaintSizeMap.find(key);
+    // Look ourselves up in the hashtables.
+    ObjectLayerSizeMap::iterator i = m_objectLayerSizeMap.find(object);
+    LayerSizeMap* innerMap = i != m_objectLayerSizeMap.end() ? &i->second : 0;
+    IntSize oldSize;
+    bool isFirstResize = true;
+    if (innerMap) {
+        LayerSizeMap::iterator j = innerMap->find(layer);
+        if (j != innerMap->end()) {
+            isFirstResize = false;
+            oldSize = j->second;
+        }
+    }
 
     const AffineTransform& currentTransform = context->getCTM();
     bool contextIsScaled = !currentTransform.isIdentityOrTranslationOrFlipped();
     if (!contextIsScaled && imageSize == size) {
         // There is no scale in effect. If we had a scale in effect before, we can just remove this object from the list.
-        if (i != m_lastPaintSizeMap.end())
-            m_lastPaintSizeMap.remove(key);
-
+        removeLayer(object, innerMap, layer);
         return false;
     }
 
@@ -150,39 +169,44 @@ bool ImageQualityController::shouldPaintAtLowQuality(GraphicsContext* context, R
         if (totalPixels > cInterpolationCutoff)
             return true;
     }
+
     // If an animated resize is active, paint in low quality and kick the timer ahead.
     if (m_animatedResizeIsActive) {
-        m_lastPaintSizeMap.set(key, size);
+        set(object, innerMap, layer, size);
         restartTimer();
         return true;
     }
     // If this is the first time resizing this image, or its size is the
     // same as the last resize, draw at high res, but record the paint
     // size and set the timer.
-    if (i == m_lastPaintSizeMap.end() || size == i->second) {
+    if (isFirstResize || oldSize == size) {
         restartTimer();
-        m_lastPaintSizeMap.set(key, size);
+        set(object, innerMap, layer, size);
         return false;
     }
     // If the timer is no longer active, draw at high quality and don't
     // set the timer.
     if (!m_timer.isActive()) {
-        keyDestroyed(key);
+        removeLayer(object, innerMap, layer);
         return false;
     }
     // This object has been resized to two different sizes while the timer
     // is active, so draw at low quality, set the flag for animated resizes and
     // the object to the list for high quality redraw.
-    m_lastPaintSizeMap.set(key, size);
+    set(object, innerMap, layer, size);
     m_animatedResizeIsActive = true;
     restartTimer();
     return true;
 }
 
+static ImageQualityController* gImageQualityController = 0;
+
 static ImageQualityController* imageQualityController()
 {
-    static ImageQualityController* controller = new ImageQualityController;
-    return controller;
+    if (!gImageQualityController)
+        gImageQualityController = new ImageQualityController;
+
+    return gImageQualityController;
 }
 
 void RenderBoxModelObject::setSelectionState(SelectionState s)
@@ -223,7 +247,13 @@ RenderBoxModelObject::~RenderBoxModelObject()
     // Our layer should have been destroyed and cleared by now
     ASSERT(!hasLayer());
     ASSERT(!m_layer);
-    imageQualityController()->objectDestroyed(this);
+    if (gImageQualityController) {
+        gImageQualityController->objectDestroyed(this);
+        if (gImageQualityController->isEmpty()) {
+            delete gImageQualityController;
+            gImageQualityController = 0;
+        }
+    }
 }
 
 void RenderBoxModelObject::destroyLayer()

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list