Issue metadata
Sign in to add a comment
|
Regression: Slow word-break: break-all layout |
||||||||||||||||||||||
Issue descriptionWhen an element has word-break set to break-all (maybe some others as well) the layout takes a long time if there are no breaks in text. See file. Test in Chrome (stable and/or ToT). Then test in Firefox. This is affecting devtools in mostly networking and console and largely because of large URLs.
,
Jun 23 2016
I did some bisecting and here's the patch that caused the problems: https://codereview.chromium.org/1766243003 Tagging kojii@
,
Jun 23 2016
Boosting priority of this, users are reporting 10 seconds layouts: http://stackoverflow.com/questions/37022408/chrome-v50-network-inspector-really-slow
,
Jun 24 2016
#3: devtool uses "break-word", not "break-all", it's about issue 603398 and the fix was merged back to M52. Removing RBS label. #0: Thank you for reporting. For "break-all", please refer to comment #42 of issue 591793 #c42. We're planning a large change that should fix this too, but it's going to take a bit. If you have real URL that is suffered by this problem, could you please let us know, so that we can investigate to see if there were any good short term solutions.
,
Jun 24 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 1 2016
This is a massive issue for us. On my machine it takes *60 seconds* to do one Layout. Here's a repro: 1. New tab 2. open devtools first, turn on device mode (the phone icon in the top right) 2. navigate the tab to https://m.washingtonpost.com/ 3. Now in devtools go to Network panel. 4. once the page has loaded for several seconds, hit the red circle to stop listening to network activity. (Not required but makes it easier) 5. on the filter bar, make sure "Hide data urls" is not checked 6. scroll down to a data url. There are several "data:application/font-woff" about 30 requests from the top. 7. Click on one of them. Results: Laying out the Headers tab for this request takes well-over 30 seconds. Attached: * screenshot showing the long layout * screenshot showing our use of break-all
,
Jul 2 2016
Issue 621165 has been merged into this issue.
,
Jul 3 2016
This issue has been moved once and is lower than Pri-1. Removing the milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 4 2016
Issue 621165 has been merged into this issue.
,
Jul 4 2016
#6: thanks for reporting, confirmed that Network tab, header view uses break-all in network/requestHeadersView.css.
,
Jul 4 2016
network/requestHeaderView.css:
.request-headers-view .outline-disclosure .header-value {
display: inline;
margin-right: 1em;
white-space: pre-wrap;
word-break: break-all;
margin-top: 1px;
}
And also devices/devicesView.css uses break-all too:
.device-page-url a {
color: #777;
word-break: break-all;
}
,
Jul 4 2016
WIP here https://codereview.chromium.org/2077313002 but this figured out that the fix for issue 603679 was not correct, so the fix will be a bit more complicated. We need to try break-all, but if it fails and has floats, we need to push the line down and try break-all again. If no floats, we should fallback to first breakable.
,
Jul 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d24d249700deef0ed63bf12ebb6af3b28f8225fb commit d24d249700deef0ed63bf12ebb6af3b28f8225fb Author: kojii <kojii@chromium.org> Date: Tue Jul 05 23:24:00 2016 Prevent to measure the whole word when break-all This patch applies the same heuristic fix as break-word to break-all. Since the switch to the complex path, r385693 fixed the performance and correctness issue of break-word and break-all in common cases. However, it also introduced a performance regression when a word is really long, such as minimized JS. This can be fixed by taking over the ShapeResult to the next lines as Gecko does, but it will require larger overhaul of the line breaker and need more time to bake. As an interim fix, r387974 introduced a heuristic fix for break-word. This patch applies the same heuristic to break-all. This patch also revealed that the fix for crbug.com/603679 was not correct. MidWordBreak must be recomputed when floats pushed the line down and available width changed. This patch includes the fix. BUG= 622810 , 603679 Review-Url: https://codereview.chromium.org/2077313002 Cr-Commit-Position: refs/heads/master@{#403830} [modify] https://crrev.com/d24d249700deef0ed63bf12ebb6af3b28f8225fb/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h
,
Jul 8 2016
From comment #41 of issue 591793 : > This "word-break: break-all" example shows very poor loading and resizing performance: > https://jsfiddle.net/4ztpbnm0/1/ > while "overflow-wrap: break-word" performs much better: > https://jsfiddle.net/746g71wb/1/ Confirmed in Canary both runs in the similar performance.
,
Jul 8 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b6b39703c17c81d31c03ed8d6b4ee9991a246d3 commit 8b6b39703c17c81d31c03ed8d6b4ee9991a246d3 Author: Koji Ishii <kojii@chromium.org> Date: Fri Jul 08 07:08:17 2016 Prevent to measure the whole word when break-all This patch applies the same heuristic fix as break-word to break-all. Since the switch to the complex path, r385693 fixed the performance and correctness issue of break-word and break-all in common cases. However, it also introduced a performance regression when a word is really long, such as minimized JS. This can be fixed by taking over the ShapeResult to the next lines as Gecko does, but it will require larger overhaul of the line breaker and need more time to bake. As an interim fix, r387974 introduced a heuristic fix for break-word. This patch applies the same heuristic to break-all. This patch also revealed that the fix for crbug.com/603679 was not correct. MidWordBreak must be recomputed when floats pushed the line down and available width changed. This patch includes the fix. BUG= 622810 , 603679 Review-Url: https://codereview.chromium.org/2077313002 Cr-Commit-Position: refs/heads/master@{#403830} (cherry picked from commit d24d249700deef0ed63bf12ebb6af3b28f8225fb) Review URL: https://codereview.chromium.org/2127393002 . Cr-Commit-Position: refs/branch-heads/2785@{#55} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/8b6b39703c17c81d31c03ed8d6b4ee9991a246d3/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h
,
Jul 8 2016
Note that the merge to M52 would probably require manual merge as I had some changes recently.
,
Jul 9 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jul 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15a2ac4a185a2ab90a7381791ea10b403bcebf62 commit 15a2ac4a185a2ab90a7381791ea10b403bcebf62 Author: Koji Ishii <kojii@chromium.org> Date: Mon Jul 11 12:49:07 2016 Prevent to measure the whole word when break-all This patch applies the same heuristic fix as break-word to break-all. Since the switch to the complex path, r385693 fixed the performance and correctness issue of break-word and break-all in common cases. However, it also introduced a performance regression when a word is really long, such as minimized JS. This can be fixed by taking over the ShapeResult to the next lines as Gecko does, but it will require larger overhaul of the line breaker and need more time to bake. As an interim fix, r387974 introduced a heuristic fix for break-word. This patch applies the same heuristic to break-all. This patch also revealed that the fix for crbug.com/603679 was not correct. MidWordBreak must be recomputed when floats pushed the line down and available width changed. This patch includes the fix. BUG= 622810 , 603679 Review-Url: https://codereview.chromium.org/2077313002 Cr-Commit-Position: refs/heads/master@{#403830} (cherry picked from commit d24d249700deef0ed63bf12ebb6af3b28f8225fb) Review URL: https://codereview.chromium.org/2136013002 . Cr-Commit-Position: refs/branch-heads/2743@{#607} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/15a2ac4a185a2ab90a7381791ea10b403bcebf62/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h
,
Jul 12 2016
Tested the issue on Windows 7, Mac 10.11.5, Ubuntu 14.04 using 53.0.2785.14 as per steps in comment #6.Observed that headers tab for 'data:application' requests loaded without any delay. Please find attached screencast. Marking it as TE-Verified.
,
Jul 12 2016
Can you add a performance test for it so it does not regress again?
,
Jul 13 2016
Tested the issue on Windows 7, Mac 10.11.5, Ubuntu 14.04 using 52.0.2743.75 as per steps in comment #6.Observed that headers tab for 'data:application' requests loaded without any delay. Marking it as TE-Verified-M52.
,
Jul 19 2016
Tested the issue on Windows 7, Mac 10.11.5, Ubuntu 14.04 using 53.0.2785.21 as per steps in comment #6.Observed that headers tab for 'data:application' requests loaded without any delay.
,
Jul 29 2016
,
Aug 2 2016
Issue 633410 has been merged into this issue.
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b56d7b9d53d796b72659d6ea0a70044a74c67ca commit 1b56d7b9d53d796b72659d6ea0a70044a74c67ca Author: kojii <kojii@chromium.org> Date: Tue Aug 30 04:25:56 2016 Add performance tests for long words/lines This patch adds performance tests for: 1. To watch break-all/break-word performance for a very long word, which required several fixes in the past. 2. To investigate whitespace collapsing in a long line. For 1, a test[1] indicates we're linear to the length of the word, but Gecko/Edge looks consistent and faster than us. For 2, we're slower than Gecko/Edge by the factor of 10 or so when the line is very long. I will investigate it further. [1] http://bl.ocks.org/stestagg/aa6db5d177b09e74825c6e4b02cb3f07 BUG= 622810 , 583711 Review-Url: https://codereview.chromium.org/2284113003 Cr-Commit-Position: refs/heads/master@{#415092} [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/long-line-nowrap-collapse.html [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/long-line-nowrap-spans-collapse.html [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/long-line-nowrap.html [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/resources/line-layout-perf-test.js [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/word-break-break-all.html [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/word-break-break-word.html [add] https://crrev.com/1b56d7b9d53d796b72659d6ea0a70044a74c67ca/third_party/WebKit/PerformanceTests/Layout/word-wrap-break-word.html |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by allada@chromium.org
, Jun 23 2016