Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2176633002:
[css-grid] Avoid the HashMap for the override containing block size (Closed)
|
Created:
4 years, 5 months ago by jfernandez Modified:
4 years, 4 months ago CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, svillar, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, Manuel Rego, jchaffraix+rendering, blink-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[css-grid] Avoid the HashMap for the override containing block size
The change made in r406557 caused a 12% regression on the grid layout
performance tests. The root cause is that as part of the changes
required to implement the orthogonal flows support we decided to use
physical directions to determine whether a grid item overflows its
grid area or not. Since we store the logical sizes in a HashMap, we
implemented some utility methods that, based on the grid container's
writing mode, retrieve the stored width or height from the HashMap.
Since the previous logic was reusing the already computed logical
value, the change introduced a considerable performance regression.
This patch address the issue by removing the utility methods and
determine the physical sizes directly inside the layout logic.
BUG=630448
Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b
Committed: https://crrev.com/e2bf9ba77a6b9add6c7a703be1fa85814c79c12e
Cr-Original-Commit-Position: refs/heads/master@{#407762}
Cr-Commit-Position: refs/heads/master@{#408203}
Patch Set 1 #
Messages
Total messages: 24 (11 generated)
Description was changed from ========== [css-grid] Don't use the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 ==========
jfernandez@igalia.com changed reviewers: + cbiesinger@chromium.org, rego@igalia.com, svillar@igalia.com
Never liked those methods :) lgtm
lgtm (longer term it may also be helpful to store the override heights in LayoutBoxRareData - https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...)
On 2016/07/22 16:36:38, cbiesinger wrote: > lgtm > > (longer term it may also be helpful to store the override heights in > LayoutBoxRareData - > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...) It's an interesting suggestion, indeed. I'll add a TODO comment.
The CQ bit was checked by jfernandez@igalia.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by jfernandez@igalia.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2180153003/ by robliao@chromium.org. The reason for reverting is: Speculative Revert for Win10 Failing Tests: fast/text/emphasis-combined-text.html svg/W3C-SVG-1.1/filters-composite-02-b.svg.
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ==========
The CQ bit was checked by robliao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Cr-Commit-Position: refs/heads/master@{#407762} ========== to ========== [css-grid] Avoid the HashMap for the override containing block size The change made in r406557 caused a 12% regression on the grid layout performance tests. The root cause is that as part of the changes required to implement the orthogonal flows support we decided to use physical directions to determine whether a grid item overflows its grid area or not. Since we store the logical sizes in a HashMap, we implemented some utility methods that, based on the grid container's writing mode, retrieve the stored width or height from the HashMap. Since the previous logic was reusing the already computed logical value, the change introduced a considerable performance regression. This patch address the issue by removing the utility methods and determine the physical sizes directly inside the layout logic. BUG=630448 Committed: https://crrev.com/2cf9f05031414616d3d1eb1e5ffa5013e396d61b Committed: https://crrev.com/e2bf9ba77a6b9add6c7a703be1fa85814c79c12e Cr-Original-Commit-Position: refs/heads/master@{#407762} Cr-Commit-Position: refs/heads/master@{#408203} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/e2bf9ba77a6b9add6c7a703be1fa85814c79c12e Cr-Commit-Position: refs/heads/master@{#408203} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
