Existence of text selection affects performance of innerHTML
Reported by
kdzwinel@gmail.com,
Jul 28 2016
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36 Steps to reproduce the problem: 1. Go to http://output.jsbin.com/muxuni 2. Open DevTools console Every 3s `output` node will be cleared using `output.innerHTML = ''` and the time this single line took to execute will be reported by console.time 3. Click on 'Load file label' to open the file dialog (it's important to click on the LABEL and not the INPUT). 4. Close the dialog (it doesn't meter if you cancel or choose a file). 5. Observe that values reported in the console are 10x bigger than before dialog was opened. What is the expected behavior? Opening file dialog should not affect performance of the website. What went wrong? As soon as the file dialog is opened performance of innerHTML greatly decreases and never recovers (you have to reload the page). Did this work before? N/A Chrome version: 52.0.2743.82 Channel: stable OS Version: OS X 10.11.5 Flash Version: - using removeChild to clear the `output` node is not affected by this bug - clicking on the INPUT directly and not the LABEL does not cause this bug - reproducible in both stable(52.0.2743.82) and canary(54.0.2810.0) - I found this working on https://github.com/notwaldorf/emojillate/pull/1
,
Jul 28 2016
,
Jul 29 2016
Confirmed on macOS 10.11 and Google Chrome 52.
,
Aug 4 2016
The bottleneck is FrameSelection::nodeWillBeRemoved(). In the fast case, FrameSelection::isNone() is true and nodeWillBeRemoved() does almost nothing. In the slow case, isNoe() is false. The file chooser dialog isn't related. Clicking on any text in the page will slow down. Calling |window.getSelection().removeAllRanges()| recovers the performance.
,
Aug 5 2016
Probably this is a dupe of Issue 139552 .
,
Aug 5 2016
Issue 259874 and Issue 505335 are also related.
,
Aug 5 2016
,
Aug 5 2016
FrameSelection::nodeWillBeRemoved() does something even if specified node isn't removed. This is "NOT GOOD".
,
Aug 5 2016
More preciously, we call Range::intersectsNode()
,
Aug 5 2016
,
Aug 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3316ab0c51851da3f4ad14a7a273c21b542ba8f4 commit 3316ab0c51851da3f4ad14a7a273c21b542ba8f4 Author: yosin <yosin@chromium.org> Date: Mon Aug 08 06:09:07 2016 Use LayoutObject::selectionState() to check whether selection contains node or not This patch changes |FrameSelection::respondToNodeModification()| to check whether node to be removed in selection or not by using value of |LayoutObject::selectionState()| instead of using |Range::intersectsNode()| to make node removal faster. Execution time of attached performance test on my local PC: Before: 3.80ms After: 2.92ms "fast/repaint/selection-after-remove.html" needs rebaseline since this patch reduce invalidate elements, e.g. HTML and BODY elements aren't invalidated. BUG= 259874 , 505335 , 632333 TEST=PerformanceTests/DOM/remove_child_with_selection.html Review-Url: https://codereview.chromium.org/2212293002 Cr-Commit-Position: refs/heads/master@{#410308} [modify] https://crrev.com/3316ab0c51851da3f4ad14a7a273c21b542ba8f4/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/3316ab0c51851da3f4ad14a7a273c21b542ba8f4/third_party/WebKit/PerformanceTests/DOM/remove_child_with_selection.html [modify] https://crrev.com/3316ab0c51851da3f4ad14a7a273c21b542ba8f4/third_party/WebKit/Source/core/editing/FrameSelection.cpp
,
Aug 8 2016
I think we can optimize drastically in this case, which removes 7000+ chid nodes of a single parent. e.g. If the current selection is not overlapped with the parent element, we don't need to call FrameSelection::nodeWillBeRemoved() at all.
,
Aug 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/50175c8dfa4b00e5ffb47796a4d54ddc26652dab commit 50175c8dfa4b00e5ffb47796a4d54ddc26652dab Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org> Date: Mon Aug 08 07:52:59 2016 Auto-rebaseline for r410308 https://chromium.googlesource.com/chromium/src/+/3316ab0c5 BUG= 632333 TBR=yosin@chromium.org Review URL: https://codereview.chromium.org/2224873002 . Cr-Commit-Position: refs/heads/master@{#410314} [modify] https://crrev.com/50175c8dfa4b00e5ffb47796a4d54ddc26652dab/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/50175c8dfa4b00e5ffb47796a4d54ddc26652dab/third_party/WebKit/LayoutTests/platform/linux/fast/repaint/selection-after-remove-expected.txt [modify] https://crrev.com/50175c8dfa4b00e5ffb47796a4d54ddc26652dab/third_party/WebKit/LayoutTests/platform/mac/fast/repaint/selection-after-remove-expected.txt [modify] https://crrev.com/50175c8dfa4b00e5ffb47796a4d54ddc26652dab/third_party/WebKit/LayoutTests/platform/win/fast/repaint/selection-after-remove-expected.txt
,
Aug 8 2016
,
Aug 8 2016
r410308 makes removeChild() faster. http://crrev.com/2221783003 will makes innerHTML setter faster by making ContanerNode::removeChildren() faster.
,
Aug 8 2016
sounds nice!
,
Aug 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0bf55edc59dd1399c5892f1afb2643ea978bddb commit d0bf55edc59dd1399c5892f1afb2643ea978bddb Author: yosin <yosin@chromium.org> Date: Tue Aug 09 10:54:06 2016 Improve canBeAnchorNode() for PositionInFlatTree constructor This patch changes |canBeAnchorNode()| for |PositionInFlatTree| constructor to check whether specified node can be participate to flat tree or not to improve debug experience. This patch is a preparation of http://crrev.com/2221783003, which attempt to call |toPositionInFlatTree()| with a position which uses |ShadowRoot| as anchor node. BUG= 139552 , 632333 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2229703002 Cr-Commit-Position: refs/heads/master@{#410629} [modify] https://crrev.com/d0bf55edc59dd1399c5892f1afb2643ea978bddb/third_party/WebKit/Source/core/editing/Position.cpp
,
Aug 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6d7af3689ae51f9c41b68c500150a1648d20241 commit a6d7af3689ae51f9c41b68c500150a1648d20241 Author: yosin <yosin@chromium.org> Date: Tue Aug 09 13:03:09 2016 Make toPositionInFlatTree() to handle a position anchored by shadow root This patch changes |toPositionInFlatTree()| to handle a position anchored by shadow root with position anchor type other than |OffsetInAnchor|. This patch is a preparation of http://crrev.com/2221783003, which attempt to call |toPositionInFlatTree()| with a position which uses |ShadowRoot| as anchor node. BUG= 139552 , 632333 TEST=run_webkit_unit_tests --gtest_filter=PositionTest.ToPositionInFlatTreeWithShadowRoot Review-Url: https://codereview.chromium.org/2229463003 Cr-Commit-Position: refs/heads/master@{#410646} [modify] https://crrev.com/a6d7af3689ae51f9c41b68c500150a1648d20241/third_party/WebKit/Source/core/editing/Position.cpp [modify] https://crrev.com/a6d7af3689ae51f9c41b68c500150a1648d20241/third_party/WebKit/Source/core/editing/PositionTest.cpp
,
Aug 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ffa8394ec09cac47f842e47f843ad13dc25feb2 commit 6ffa8394ec09cac47f842e47f843ad13dc25feb2 Author: yosin <yosin@chromium.org> Date: Wed Aug 10 06:11:30 2016 Introduce FrameSelection::nodeChildrenWillBeRemoved() This patch introduces |FrameSelection::nodeChildrenWillBeRemoved()| to process removing children once for container node rather than calling |FrameSelection::nodeWillBeRemoved()| for each descendant nodes in container to improve execution speed of callers of |ContainerNode::removeChildren()|, e.g. |innserHTML| setter, |innerText| setter, etc. After this patch execution speeof is O(1) instead of O(n), where n is number of descendant nodes in container. Updating test expectations of "svg/custom/use-clipped-hit.svg" is caused by difference of selection paint invalidation. From this patch, selection paint invalidation is done by children removal rather than caused by selected nodes. BUG= 139552 , 632333 TEST=PerformanceTests/DOM/inner_html_with_selection.html Review-Url: https://codereview.chromium.org/2221783003 Cr-Commit-Position: refs/heads/master@{#410976} [modify] https://crrev.com/6ffa8394ec09cac47f842e47f843ad13dc25feb2/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/6ffa8394ec09cac47f842e47f843ad13dc25feb2/third_party/WebKit/PerformanceTests/DOM/inner_html_with_selection.html [modify] https://crrev.com/6ffa8394ec09cac47f842e47f843ad13dc25feb2/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/6ffa8394ec09cac47f842e47f843ad13dc25feb2/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/6ffa8394ec09cac47f842e47f843ad13dc25feb2/third_party/WebKit/Source/core/editing/FrameSelection.h
,
Aug 10 2016
,
Aug 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/436d32972b32ef4cf3191d84df9be971a12bfa08 commit 436d32972b32ef4cf3191d84df9be971a12bfa08 Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org> Date: Wed Aug 10 08:16:44 2016 Auto-rebaseline for r410976 https://chromium.googlesource.com/chromium/src/+/6ffa8394e BUG= 632333 TBR=yosin@chromium.org Review URL: https://codereview.chromium.org/2228383002 . Cr-Commit-Position: refs/heads/master@{#411002} [modify] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/linux/svg/custom/use-clipped-hit-expected.txt [modify] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/mac/svg/custom/use-clipped-hit-expected.txt [modify] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/win/svg/custom/use-clipped-hit-expected.png [modify] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/win/svg/custom/use-clipped-hit-expected.txt [add] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/win7/svg/custom/use-clipped-hit-expected.png [add] https://crrev.com/436d32972b32ef4cf3191d84df9be971a12bfa08/third_party/WebKit/LayoutTests/platform/win7/svg/custom/use-clipped-hit-expected.txt
,
Aug 10 2016
Seems to have caused issue 636311 .
,
Oct 12 2016
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by kdzwinel@gmail.com
, Jul 28 2016