Issue metadata
Sign in to add a comment
|
1.4%-23.7% regression in system_health.memory_mobile at 476026:476274 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 2 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8977856648525847696
,
Jun 3 2017
=== Auto-CCing suspected CL author aboxhall@chromium.org === Hi aboxhall@chromium.org, the bisect results pointed to your CL, please take a look at the results. === BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : aboxhall Commit : 6281b70b9a36a64590f99c799ee55264da6cb52d Date : Thu Jun 01 10:09:50 2017 Subject: Check whether a text node may be about to be redistributed in ShouldUpdateLayoutByReattaching(). Prevents use-after-free when SetTextWithOffset is called with dirty distribution. Bisect Details Configuration: android_webview_arm64_aosp_perf_bisect Benchmark : system_health.memory_mobile Metric : memory:webview:all_processes:reported_by_os:system_memory:java_heap:proportional_resident_size_avg/load_media/load_media_dailymotion Change : 2.32% | 5070506.66667 -> 5187925.33333 Revision Result N chromium@476241 5070507 +- 51945.5 6 good chromium@476242 5649408 +- 153886 6 bad <-- chromium@476243 5585920 +- 48809.5 6 bad chromium@476244 5454165 +- 113721 6 bad chromium@476247 5323776 +- 97274.6 6 bad chromium@476253 5187925 +- 104655 6 bad Please refer to the following doc on diagnosing memory regressions: https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md To Run This Test src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.media.dailymotion system_health.memory_mobile Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8977856648525847696 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=4901431699570688 | O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq | X | for more information addressing perf regression bugs. For feedback, | / \ | file a bug with component Speed>Bisection. Thank you!
,
Jun 5 2017
This makes a lot of sense. Looking at a fix.
,
Jun 5 2017
,
Jun 12 2017
Friendly perf sheriff ping, any update on this?
,
Jun 13 2017
Apologies for the silence. Have been looking at this but it's quite complicated to fix, unfortunately.
,
Jun 14 2017
Proposed fix: https://codereview.chromium.org/2939933002/
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/303e0e83015674dec5bf2fc1a5eafe2871e867ee commit 303e0e83015674dec5bf2fc1a5eafe2871e867ee Author: Takayoshi Kochi <kochi@chromium.org> Date: Mon Jun 26 04:55:46 2017 Only call TextChanged on existing AXObjects. This avoids triggering the code which calls UpdateDistribution as part of creating a new AXObject, which was causing issues. This change should cause no behaviour changes, as in cases where an AXObject needs to be created for a text node, its parent will also have a ChildrenChanged notification fired on it, which will cause the appropriate updates to fire. Reverts https://codereview.chromium.org/2926713002/ as the code which causes UpdateDistribution to be called should never run with this change, so we don't need to check whether distribution is dirty any more. BUG= 729229 , 732200 , 734348 Review-Url: https://codereview.chromium.org/2939933002 Cr-Original-Commit-Position: refs/heads/master@{#480792} Review-Url: https://codereview.chromium.org/2957733002 . Cr-Commit-Position: refs/branch-heads/3112@{#464} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/content/test/data/accessibility/event/menulist-collapse-expected-win.txt [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/LayoutTests/accessibility/contenteditable-notifications.html [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/LayoutTests/accessibility/presentation-owned-elements.html [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/LayoutTests/resources/accessibility-helper.js [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/core/layout/LayoutMenuList.cpp [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/modules/accessibility/AXMenuList.cpp [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/modules/accessibility/AXMenuListPopup.cpp [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/modules/accessibility/AXMenuListPopup.h [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/modules/accessibility/AXObjectCacheImpl.cpp [modify] https://crrev.com/303e0e83015674dec5bf2fc1a5eafe2871e867ee/third_party/WebKit/Source/modules/accessibility/AXObjectCacheImpl.h
,
Jul 27 2017
Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md We're looking for one of the following: 1. Justification via explanation 2. Plan to revert or fix 3. Angry rage throwing of equipment at my head Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it. Note: This was a bulk edit message and not very personal.
,
Aug 4 2017
This seems to be fixed. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by m...@chromium.org
, Jun 2 2017