Limit the total number of paint rects.

Based on some testing on a single core machine, reducing this
number below 10 doesn't seem to help.  On some of the page
cycler tests (intl2 in particular), I observed some excessive
paint rect counts (>50).  Those outliers seem to explain the
perf regression observed for single core machines.

R=brettw
BUG=29589
TEST=none

Review URL: http://codereview.chromium.org/481002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34189 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 50 insertions, 11 deletions

diff --git a/chrome/renderer/paint_aggregator.cc b/chrome/renderer/paint_aggregator.cc
index 8f77b15..ebe6314 100644
--- a/chrome/renderer/paint_aggregator.cc
+++ b/chrome/renderer/paint_aggregator.cc
@@ -28,6 +28,12 @@
 // we will tolerate before downgrading the scroll into a repaint.
 static const float kMaxRedundantPaintToScrollArea = 0.8f;
 
+// The maximum number of paint rects.  If we exceed this limit, then we'll
+// start combining paint rects (see CombinePaintRects).  This limiting is
+// important since the WebKit code associated with deciding what to paint given
+// a paint rect can be significant.
+static const size_t kMaxPaintRects = 10;
+
 gfx::Rect PaintAggregator::PendingUpdate::GetScrollDamage() const {
   // Should only be scrolling in one direction at a time.
   DCHECK(!(scroll_delta.x() && scroll_delta.y()));
@@ -94,17 +100,9 @@ void PaintAggregator::InvalidateRect(const gfx::Rect& rect) {
     }
   }
 
-  // Add a non-overlapping paint.  TODO(darin): Limit the size of this vector?
+  // Add a non-overlapping paint.
   update_.paint_rects.push_back(rect);
 
-  // Track how large the paint_rects vector grows during an invalidation
-  // sequence.  Note: A subsequent invalidation may end up being combined
-  // with all existing paints, which means that tracking the size of
-  // paint_rects at the time when GetPendingUpdate() is called may mask
-  // certain performance problems.
-  HISTOGRAM_COUNTS_10000("MPArch.RW_IntermediatePaintRectCount",
-                          update_.paint_rects.size());
-
   // If the new paint overlaps with a scroll, then it forces an invalidation of
   // the scroll.  If the new paint is contained by a scroll, then trim off the
   // scroll damage to avoid redundant painting.
@@ -118,6 +116,17 @@ void PaintAggregator::InvalidateRect(const gfx::Rect& rect) {
         update_.paint_rects.erase(update_.paint_rects.end() - 1);
     }
   }
+
+  if (update_.paint_rects.size() > kMaxPaintRects)
+    CombinePaintRects();
+
+  // Track how large the paint_rects vector grows during an invalidation
+  // sequence.  Note: A subsequent invalidation may end up being combined
+  // with all existing paints, which means that tracking the size of
+  // paint_rects at the time when GetPendingUpdate() is called may mask
+  // certain performance problems.
+  HISTOGRAM_COUNTS_100("MPArch.RW_IntermediatePaintRectCount",
+                       update_.paint_rects.size());
 }
 
 void PaintAggregator::ScrollRect(int dx, int dy, const gfx::Rect& clip_rect) {
@@ -210,3 +219,33 @@ void PaintAggregator::InvalidateScrollRect() {
   update_.scroll_delta = gfx::Point();
   InvalidateRect(scroll_rect);
 }
+
+void PaintAggregator::CombinePaintRects() {
+  // Combine paint rects down to at most two rects: one inside the scroll_rect
+  // and one outside the scroll_rect.  If there is no scroll_rect, then just
+  // use the smallest bounding box for all paint rects.
+  //
+  // NOTE: This is a fairly simple algorithm.  We could get fancier by only
+  // combining two rects to get us under the kMaxPaintRects limit, but if we
+  // reach this method then it means we're hitting a rare case, so there's no
+  // need to over-optimize it.
+  //
+  if (update_.scroll_rect.IsEmpty()) {
+    gfx::Rect bounds = update_.GetPaintBounds();
+    update_.paint_rects.clear();
+    update_.paint_rects.push_back(bounds);
+  } else {
+    gfx::Rect inner, outer;
+    for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
+      const gfx::Rect& existing_rect = update_.paint_rects[i];
+      if (update_.scroll_rect.Contains(existing_rect)) {
+        inner = inner.Union(existing_rect);
+      } else {
+        outer = outer.Union(existing_rect);
+      }
+    }
+    update_.paint_rects.clear();
+    update_.paint_rects.push_back(inner);
+    update_.paint_rects.push_back(outer);
+  }
+}
diff --git a/chrome/renderer/paint_aggregator.h b/chrome/renderer/paint_aggregator.h
index d1af49b..9a595be 100644
--- a/chrome/renderer/paint_aggregator.h
+++ b/chrome/renderer/paint_aggregator.h
@@ -50,6 +50,7 @@ class PaintAggregator {
   gfx::Rect ScrollPaintRect(const gfx::Rect& paint_rect, int dx, int dy) const;
   bool ShouldInvalidateScrollRect(const gfx::Rect& rect) const;
   void InvalidateScrollRect();
+  void CombinePaintRects();
 
   PendingUpdate update_;
 };
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 5d56a41..fd60468 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -484,8 +484,7 @@ void RenderWidget::DoDeferredUpdate() {
     bounds.set_width(canvas->getDevice()->width());
     bounds.set_height(canvas->getDevice()->height());
 
-    HISTOGRAM_COUNTS_10000("MPArch.RW_PaintRectCount",
-                           update.paint_rects.size());
+    HISTOGRAM_COUNTS_100("MPArch.RW_PaintRectCount", update.paint_rects.size());
 
     for (size_t i = 0; i < update.paint_rects.size(); ++i)
       PaintRect(update.paint_rects[i], bounds.origin(), canvas.get());