Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1549693002:
Optimize collapsed border painting (Closed)
|
Created:
5 years ago by Xianzhu Modified:
4 years, 7 months ago Reviewers:
chrishtr CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-paint_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dshwang, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, slimming-paint-reviews_chromium.org, szager+layoutwatch_chromium.org, zoltan1 Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionOptimize collapsed border painting
Save cached collapsed borders in LayoutTableCell OwnPtr field instead
of in LayoutTableSection hash map to reduce number of hash lookups.
Previously during collectBorderValues, each cell needed 4 hash
lookups, and during painting, each cell needed another 4 hash lookups.
BUG=34736
Committed: https://crrev.com/e2e24217bf6051b94756337f8d8cd183ffbebd30
Cr-Commit-Position: refs/heads/master@{#391867}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : Field instead of global HashMap #
Total comments: 13
Patch Set 4 : Rebase #Patch Set 5 : Rebase #Patch Set 6 : Remove extra dtor #Patch Set 7 : Rebase #Patch Set 8 : #Patch Set 9 : #
Total comments: 6
Patch Set 10 : #
Total comments: 6
Patch Set 11 : #
Messages
Total messages: 27 (7 generated)
wangxianzhu@chromium.org changed reviewers: + chrishtr@chromium.org, jchaffraix@chromium.org
https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:367: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; 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.)
I really like the optimization around not caching invisible collapsing borders. The other one needs to be evaluated against the broader roadmap. This means that we should probably land the first optimization on its own to avoid having it tangled with more complicated changes. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:931: if (!newValues.startBorder.isVisible() && !newValues.endBorder.isVisible() && !newValues.beforeBorder.isVisible() && !newValues.afterBorder.isVisible()) { How frequent is this case? It seems like a corner-case optimization that adds more complexity than it saves memory. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:935: changed = true; Do we really need that for the first this code runs? If it is needed, we should put a comment as to *why*. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:367: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; On 2015/12/28 at 21:27:16, Xianzhu wrote: > 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.) 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. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/CollapsedBorderValue.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/CollapsedBorderValue.h:80: return isVisible() && isSameIgnoringColor(tableCurrentBorderValue); Shouldn't this be ASSERT(isVisible()) as we don't want to see invisible borders when painting? Overall we should add more performance ASSERTs to make sure we don't have a bug adding those back down the road.
https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:931: if (!newValues.startBorder.isVisible() && !newValues.endBorder.isVisible() && !newValues.beforeBorder.isVisible() && !newValues.afterBorder.isVisible()) { On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > How frequent is this case? It seems like a corner-case optimization that adds > more complexity than it saves memory. This optimization does improve performance of PerformanceTest/Paint/large-table-background-change-with-invisible-collapsed-borders.html (previously PerformanceTest/Layout/large-table-background-change-with-collapsed-borders.html) from 91ms to 42ms :) (http://storage.googleapis.com/chromium-telemetry/html-results/results-2015-12...) by eliminating the whole loop and treewalks for collapsed borders in TablePainter::paintObject(). I think the case is reasonable in pages that set all tables to collapse borders but don't specify borders for all tables. I can stat with top 10k pages. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:935: changed = true; On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > Do we really need that for the first this code runs? If it is needed, we should > put a comment as to *why*. Sorry I'm not sure, do you mean I should comment on the purpose of the 'changed' variable? https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:367: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > On 2015/12/28 at 21:27:16, Xianzhu wrote: > > 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.) > > 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. 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. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/CollapsedBorderValue.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/CollapsedBorderValue.h:80: return isVisible() && isSameIgnoringColor(tableCurrentBorderValue); On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > Shouldn't this be ASSERT(isVisible()) as we don't want to see invisible borders > when painting? > > Overall we should add more performance ASSERTs to make sure we don't have a bug > adding those back down the road. A cell may have e.g. 2 visible borders and 2 invisible borders. In the case we need to check shouldPaint() during painting also for the invisible borders, so we can't assert here.
Stat showed the following results about collapsed borders on top 10k sites (https://docs.google.com/spreadsheets/d/1i_kqo71EAiqyphgDFKwRThH7Oq69MWHncH-lS...): - 5.74% LayoutBoxes are LayoutTableCells; - 6.89% LayoutTableCells have visible collapsed borders; - 11.72% LayoutTableCells have invisible collapsed borders: optimization of invisible collapsed borders is necessary; - Adding a pointer in LayoutTableCell increases memory size of all LayoutTableCells by 3%, memory size of all LayoutBoxes by 0.2%; - If we let LayoutBox::m_rareData point to LayoutTableCellRareData containing the pointer for LayoutTableCells, the memory increase is similar; - Considering previously about 18% cells have map entries in LayoutTableSection::CellsCollapsedBordersMap and the extra cost of map, memory size of the latest patch might be similar to before. I can separate invisible collapsed border optimization into another CL. Let's decide this on Monday's discussion.
https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:931: if (!newValues.startBorder.isVisible() && !newValues.endBorder.isVisible() && !newValues.beforeBorder.isVisible() && !newValues.afterBorder.isVisible()) { On 2016/01/08 at 17:50:45, Xianzhu wrote: > On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > > How frequent is this case? It seems like a corner-case optimization that adds > > more complexity than it saves memory. > > This optimization does improve performance of PerformanceTest/Paint/large-table-background-change-with-invisible-collapsed-borders.html (previously PerformanceTest/Layout/large-table-background-change-with-collapsed-borders.html) from 91ms to 42ms :) (http://storage.googleapis.com/chromium-telemetry/html-results/results-2015-12...) by eliminating the whole loop and treewalks for collapsed borders in TablePainter::paintObject(). > > I think the case is reasonable in pages that set all tables to collapse borders but don't specify borders for all tables. > > I can stat with top 10k pages. This really should have a comment about what it's targeting and include the great statistics: "- 11.72% LayoutTableCells have invisible collapsed borders: optimization of invisible collapsed borders is necessary;" Also having a helper function for the 4 checks would be nice as it would clarify what the code is testing. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:935: changed = true; On 2016/01/08 at 17:50:45, Xianzhu wrote: > On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > > Do we really need that for the first this code runs? If it is needed, we should > > put a comment as to *why*. > > Sorry I'm not sure, do you mean I should comment on the purpose of the 'changed' variable? m_collapsedBorderValue will be nullptr the first time we run this code as this is run for every paint. I was assuming this wouldn't be needed in this case but it should be required in a subsequent paint as we could get out of the optimization above. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:367: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; On 2016/01/08 at 17:50:45, Xianzhu wrote: > On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > > On 2015/12/28 at 21:27:16, Xianzhu wrote: > > > 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.) > > > > 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. > > 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. 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. https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/CollapsedBorderValue.h (right): https://codereview.chromium.org/1549693002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/CollapsedBorderValue.h:80: return isVisible() && isSameIgnoringColor(tableCurrentBorderValue); On 2016/01/08 at 17:50:45, Xianzhu wrote: > On 2016/01/08 14:18:15, Julien Chaffraix - PST wrote: > > Shouldn't this be ASSERT(isVisible()) as we don't want to see invisible borders > > when painting? > > > > Overall we should add more performance ASSERTs to make sure we don't have a bug > > adding those back down the road. > > A cell may have e.g. 2 visible borders and 2 invisible borders. In the case we need to check shouldPaint() during painting also for the invisible borders, so we can't assert here. OK!
Description was changed from ========== Optimize collapsed border painting - Don't cache/paint invisible borders. Previously a table with 'collapse-border:collapse' but no borders still needed to walk all of the cells twice during painting because of the invisible borders. - Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 ========== to ========== Optimize collapsed border painting Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 ==========
What's going on with this CL?
wangxianzhu@chromium.org changed reviewers: - jchaffraix@chromium.org
On 2016/01/28 19:00:11, chrishtr wrote: > What's going on with this CL? This CL was held off as a fail-safe of Julien's more aggressive global optimization (vs this CL as a micro optimization) of collapsed borders (https://docs.google.com/document/d/1lKrvTI5gfNdYfR9bVlVaFDO1rFJoOlTz06SdrNsHKJM/). He planned to work on that in his last week in Chrome team, but there seems no luck. I think Julien's design doc still makes a lot of sense. Might work on it after spv2 work or find another one who will be interested (e.g. atotic@). Will sync this CL for review.
Uploaded rebased patch. Ptal.
Is this CL still relevant? I think it fell through the cracks..
On 2016/05/03 16:11:09, chrishtr wrote: > Is this CL still relevant? I think it fell through the cracks.. It did fall cracks when atotic@ planned to work on a more complete solution based on jchaffraix@'s proposal. Now atotic@ has moved to work on layout issues, so I'm picking this up. Will let you know when it is ready for review.
It's ready for review now. Ptal. https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/style/CollapsedBorderValue.h (right): https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/style/CollapsedBorderValue.h:71: return true; Previously cell borders were cached in LayoutTableSection, separately for the 4 sides and invisible borders were not cached. If a new border previously invisible is now still invisible (though with some of other fields changed), we not cache it, and treat it as not changed. Now we need the above change to treat invisible borders which is previously invisible unchanged no matter of other fields. This is because now we cache the 4 borders (including invisible ones) of a cell in the same structure, when any of the borders is visible. https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/style/CollapsedBorderValue.h:82: return isVisible() && isSameIgnoringColor(tableCurrentBorderValue); The reason of this change is the same as the above one.
https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:922: // FIXME: Need a way to invalidate/repaint the borders only. crbug.com/451090#c5. This is obsolete due to other work? https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.h:369: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; Is border collapsing common enough that we should just store it directly without a pointer?
https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:922: // FIXME: Need a way to invalidate/repaint the borders only. crbug.com/451090#c5. On 2016/05/04 23:24:18, chrishtr wrote: > This is obsolete due to other work? No. Restored. https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/1549693002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.h:369: OwnPtr<CollapsedBorderValues> m_collapsedBorderValues; On 2016/05/04 23:24:18, chrishtr wrote: > Is border collapsing common enough that we should just store it directly without > a pointer? It's not common: 6.9% cells have visible collapsed borders at least on one side.
https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:921: // In slimming paint mode, we need to invalidate all cells with collapsed border changed. Still gone? https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:926: changed = !m_collapsedBorderValues->startBorder.visuallyEquals(newValues.startBorder) Add a comment that we check visuallyEquals so that the table cell is invalidated only if a changed collapsed border is visible in the first place.
https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:921: // In slimming paint mode, we need to invalidate all cells with collapsed border changed. On 2016/05/05 15:31:36, chrishtr wrote: > Still gone? It's now at new line 934-935. https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:926: changed = !m_collapsedBorderValues->startBorder.visuallyEquals(newValues.startBorder) On 2016/05/05 15:31:36, chrishtr wrote: > Add a comment that we check visuallyEquals so that the table cell is invalidated > only if a changed collapsed border > is visible in the first place. Done.
lgtm Looks good modulo the comment below. https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:926: changed = !m_collapsedBorderValues->startBorder.visuallyEquals(newValues.startBorder) On 2016/05/05 at 17:38:49, Xianzhu wrote: > On 2016/05/05 15:31:36, chrishtr wrote: > > Add a comment that we check visuallyEquals so that the table cell is invalidated > > only if a changed collapsed border > > is visible in the first place. > > Done. Don't see the added comment in this patch set?
https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/1549693002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:926: changed = !m_collapsedBorderValues->startBorder.visuallyEquals(newValues.startBorder) On 2016/05/05 17:42:12, chrishtr wrote: > On 2016/05/05 at 17:38:49, Xianzhu wrote: > > On 2016/05/05 15:31:36, chrishtr wrote: > > > Add a comment that we check visuallyEquals so that the table cell is > invalidated > > > only if a changed collapsed border > > > is visible in the first place. > > > > Done. > > Don't see the added comment in this patch set? Sorry, didn't press <Enter> after git cl upload.
The CQ bit was checked by wangxianzhu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1549693002/#ps200001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1549693002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1549693002/200001
Message was sent while issue was closed.
Description was changed from ========== Optimize collapsed border painting Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 ========== to ========== Optimize collapsed border painting Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Optimize collapsed border painting Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 ========== to ========== Optimize collapsed border painting Save cached collapsed borders in LayoutTableCell OwnPtr field instead of in LayoutTableSection hash map to reduce number of hash lookups. Previously during collectBorderValues, each cell needed 4 hash lookups, and during painting, each cell needed another 4 hash lookups. BUG=34736 Committed: https://crrev.com/e2e24217bf6051b94756337f8d8cd183ffbebd30 Cr-Commit-Position: refs/heads/master@{#391867} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/e2e24217bf6051b94756337f8d8cd183ffbebd30 Cr-Commit-Position: refs/heads/master@{#391867} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
