Optimize collapsed border painting

third_party/WebKit/Source/core/layout/LayoutTableCell.h

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2009, 2013 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 252 matching lines @@
 
 #if ENABLE(ASSERT)
     bool isFirstOrLastCellInRow() const
     {
         return !table()->cellAfter(this) || !table()->cellBefore(this);
     }
 #endif
 
     const char* name() const override { return "LayoutTableCell"; }
 
+    struct CollapsedBorderValues {
+        CollapsedBorderValue startBorder;
+        CollapsedBorderValue endBorder;
+        CollapsedBorderValue beforeBorder;
+        CollapsedBorderValue afterBorder;
+    };
+    const CollapsedBorderValues* collapsedBorderValues() const { return m_collapsedBorderValues.get(); }
+
 protected:
     void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override;
     void computePreferredLogicalWidths() override;
 
     void addLayerHitTestRects(LayerHitTestRects&, const PaintLayer* currentCompositedLayer, const LayoutPoint& layerOffset, const LayoutRect& containerRect) const override;
 
 private:
     bool isOfType(LayoutObjectType type) const override { return type == LayoutObjectTableCell || LayoutBlockFlow::isOfType(type); }
 
     void willBeRemovedFromTree() override;
@@ skipping 65 matching lines @@
     unsigned m_hasColSpan: 1;
     unsigned m_hasRowSpan: 1;
 
     // The intrinsic padding.
     // See class comment for what they are.
     //
     // Note: Those fields are using non-subpixel units (int)
     // because we don't do fractional arithmetic on tables.
     int m_intrinsicPaddingBefore;
     int m_intrinsicPaddingAfter;
+
+    OwnPtr<CollapsedBorderValues> m_collapsedBorderValues;

[Xianzhu, 2015/12/28 21:27:16]

Using this field (instead of global maps), we will get 17% performance gain over
the old code, and 7% gain over one global map for all collapsed border method
(Patch Set 2).

(Actually I'm testing performance of converting some other heavily used global
maps into fields. According to haraken@'s memory profiling results, size of
LayoutObjects is a minor contributor to overall memory usage. It seems worth to
take a bit more memory for better performance.)

[Julien - ping for review, 2016/01/08 14:18:15]

I also think it's a good tradeoff to make but do we have a sense of how much the
memory overhead is? The reason is that it's now a cost for all table cells,
regardless of whether they have collapsing borders. Considering that the default
is border-collapse: separate, it means we will add overhead in the default case.

That said this is also a local optimization. If we really want good gains, we
need to change the algorithm, not just patch over the existing one. FF is a lot
faster than us because they have a more sensible algorithm and data-structure.
If you want to get to the bottom of this slowness, I would rather discuss the
broader direction and maybe forgo this optimization if it prevents those
changes.

[Xianzhu, 2016/01/08 17:50:45]

For the performance issue of small mutation in a large table/layer, the root
cause is the current way of slimming-paint handles incremental painting. I'm
also working on this. However, micro optimization is still useful for the first
painting.

About tradeoff, with
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xWMmQfD_X0A I
noticed that the the cache locality effect is more important than memory size by
increasing size of many small objects. Considering the effects (17% gain/7%
gain, and memory increase/cache locality), now I prefer Patch Set 2 (using
global map) to Patch Set 3. Besides performance I think it also makes code
cleaner by moving collapsed border caching code out of unrelated
LayoutTableSection into LayoutTableCell.

[Julien - ping for review, 2016/01/11 18:11:36]

I also prefer having the code separated. FF has this code split to helper
structures (BCMapCellInfo) and there is a huge value in doing something similar.

 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutTableCell, isTableCell());
 
 inline LayoutTableCell* LayoutTableCell::previousCell() const
 {
     return toLayoutTableCell(LayoutObject::previousSibling());
 }
 
 inline LayoutTableCell* LayoutTableCell::nextCell() const
 {
     return toLayoutTableCell(LayoutObject::nextSibling());
 }
 
 inline LayoutTableCell* LayoutTableRow::firstCell() const
 {
     ASSERT(children() == virtualChildren());
     return toLayoutTableCell(children()->firstChild());
 }
 
 inline LayoutTableCell* LayoutTableRow::lastCell() const
 {
     ASSERT(children() == virtualChildren());
     return toLayoutTableCell(children()->lastChild());
 }
 
 } // namespace blink
 
 #endif // LayoutTableCell_h