New issue
Advanced search Search tips

Issue 623891 link

Starred by 3 users

Issue metadata

Status: Duplicate
Merged: issue 620142
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Performance Regression for removeChild on "select" Elements with "option" Children

Reported by alex.lx....@gmail.com, Jun 28 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.106 Safari/537.36

Steps to reproduce the problem:
The attached html file can be used as a simplified test to reproduce and benchmark the problem:
1. See and adapt the declared JS constants NUMBER_OF_OPTIONS and NUMBER_OF_TEXT_NODE_CHARS (see below).
2. Open the test file in Chrome and click the "Remove all children" button.
3. See an alert with the elapsed time popping up.

What is the expected behavior?
Removal of "option" child elements from "select" parent elements should be as fast as it was in Chrome 50.x

What went wrong?
Since Chrome 51.x, removeChild execution time was significantly increased when removing "option" child elements from a "select" parent element. This even leads to the browser tab hanging, when trying to remove several thousands of children.

Benchmark scenario:
Removal of 500 "option" child elements (cf. NUMBER_OF_OPTIONS constant in the test file) from a "select" parent element, where each option has a child text node containing 100 random characters (cf. NUMBER_OF_TEXT_NODE_CHARS constant in the test file)

Execution times, by browser and version:
Chrome 50.0.2661.75: 1 ms
Chrome 51.0.2704.106: 5220 ms    // regression!
Firefox 47.0 (as reference): 56 ms

Detailed execution times for Chrome 51.0.2704.106:
#removed elements		execution time in ms
100				241
200				867
300				2157
400				3336
500				5220
600				7493
700				10172
800				13373
900				16813
1k				21125

(Similar execution times were measured with 51.0.2704.8x and 51.0.2704.103)

Observations:
- During execution, a single CPU core is maxed.
- The performance regression is significantly less severe when the text nodes attached to the options are shorter (e.g. removal of 500 "option" elements with 10-character text nodes in 60 ms).

Interpretation:
- Execution time increases in a more-than-linear manner, depending on the number of removed option elements (looks much like O(n*log(n))) -- not sure if that is to be expected.

Did this work before? Yes In Chrome 50.x (tested with 50.0.2661.75)

Chrome version: 51.0.2704.106  Channel: stable
OS Version: Ubuntu 16.04 x64, Kernel 4.4.0-24
Flash Version: Shockwave Flash 22.0 r0

- The same performance regression exists in the Windows build.
- Crash Dump ID obtained while hitting a "hanging tab" (during attempted removal of 5000 options with 100-character text nodes): 78f7217c00000000
 
select_option_removal_benchmark.html
1.1 KB View Download

Comment 1 by joh...@chromium.org, Jun 28 2016

Components: -Blink Blink>Forms>Select
Labels: -Pri-2 Pri-1
Status: Untriaged (was: Unconfirmed)
Thanks for the detailed bug report!

Ouch, those execution times look like O(n^2) to me (specifically about num_elements^2 * 0.021 + 30 ms).

I can see the crash at https://crash.corp.google.com/browse?q=ReportID%3D%2778f7217c00000000. Stacktrace is:

third_party/skia/src/core/SkPaint.cpp:1684 - SkPaint::descriptorProc
third_party/skia/src/core/SkPaint.cpp:1714 - SkPaint::getTextWidths
chrome + 0x0143f51f -
chrome + 0x04c6edcf -
third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp:156 - blink::harfBuzzGetGlyphHorizontalAdvance
third_party/harfbuzz-ng/src/hb-font-private.hh:197 - _hb_ot_shape
third_party/harfbuzz-ng/src/hb-shape-plan.cc:373 - hb_shape_plan_create_cached
third_party/harfbuzz-ng/src/hb-shaper-list.hh:43 - hb_shape_plan_execute
third_party/harfbuzz-ng/src/hb-shape.cc:377 - hb_shape
third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp:299 - blink::HarfBuzzShaper::shapeRange
third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp:561 - blink::HarfBuzzShaper::shapeResult
third_party/WebKit/Source/wtf/PartitionAlloc.cpp:792 - WTF::partitionAllocSlowPath
third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.h:82 - blink::CachingWordShapeIterator::shapeWordWithoutSpacing
chrome + 0x055465ca - _fini
libstdc++.so.6.0.21 + 0x0011de4a -
third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.h:95 - blink::CachingWordShapeIterator::shapeWord
third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.h:170 - blink::CachingWordShapeIterator::shapeToEndIndex
third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.h:103 - blink::CachingWordShapeIterator::next
third_party/WebKit/Source/platform/fonts/shaping/CachingWordShaper.cpp:45 - blink::CachingWordShaper::width
chrome + 0x02f216df -
third_party/WebKit/Source/platform/fonts/Font.cpp:733 - blink::Font::width
third_party/WebKit/Source/wtf/text/WTFString.cpp:403 - WTF::String::simplifyWhiteSpace
third_party/WebKit/Source/core/html/HTMLOptionElement.cpp:368 - blink::HTMLOptionElement::textIndentedToRespectGroupLabel
third_party/WebKit/Source/core/layout/TextRunConstructor.cpp:98 - blink::constructTextRun
third_party/WebKit/Source/core/layout/LayoutMenuList.cpp:202 - blink::LayoutMenuList::updateOptionsWidth
third_party/WebKit/Source/core/layout/LayoutMenuList.cpp:208 - blink::LayoutMenuList::updateFromElement
third_party/WebKit/Source/core/html/HTMLSelectElement.cpp:1050 - blink::HTMLSelectElement::selectOption
third_party/WebKit/Source/core/html/HTMLSelectElement.cpp:878 - blink::HTMLSelectElement::resetToDefaultSelection
third_party/WebKit/Source/core/html/HTMLSelectElement.cpp:992 - blink::HTMLSelectElement::optionRemoved
third_party/WebKit/Source/core/html/HTMLOptionElement.cpp:410 - blink::HTMLOptionElement::removedFrom
third_party/WebKit/Source/core/dom/ContainerNode.cpp:868 - blink::ContainerNode::notifyNodeRemoved
third_party/WebKit/Source/core/dom/ContainerNode.cpp:595 - blink::ContainerNode::removeChild
third_party/WebKit/Source/core/dom/Node.cpp:488 - blink::Node::removeChild

Looks like it's recalculating the width of the select element, by measuring the width of all the options, each time one of the options is removed! That's clearly wrong, since there's nothing in your innner loop `while(select.firstChild) { select.removeChild(select.firstChild); }` that should trigger a layout.

Comment 2 by joh...@chromium.org, Jun 28 2016

Blockedon: 620142
Cc: cbiesin...@chromium.org schenney@chromium.org
Owner: tkent@chromium.org
Status: Assigned (was: Untriaged)
In theory this is fixed by  issue 620142 , but actually this isn't fully fixed. Removing 20000 options took 39 ms in m50, but takes over 8000 ms in m53 (r400904) - still a 200x regression (though tkent's patch has definitely helped - extrapolating suggests that it would take 10 million ms in m51!).

More specifically, removing options one by one took linear time in m50, whereas it takes quadratic time in m53 even with tkent's patch (though the constant multiple is greatly improved compared to m51, which was also quadratic).

See the graph I gathered: https://docs.google.com/spreadsheets/d/1GzQoexpYS5BBf5uBNVXu4m3c6cofUw7D4G_VFkxd1-w/edit

I used test page: https://jsbin.com/fojaza (based on the reporter's select_option_removal_benchmark.html - thanks!).

For m50 I tested on https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/378072/
For m53 I tested on https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/400904/

Comment 3 by tkent@chromium.org, Jun 28 2016

Blockedon: -620142
Mergedinto: 620142
Status: Duplicate (was: Assigned)

Comment 4 by tkent@chromium.org, Jun 28 2016

Blockedon: 620142

Comment 5 by tkent@chromium.org, Jun 29 2016

Blockedon: -620142

Sign in to add a comment