Issue metadata
Sign in to add a comment
|
page_cycler_v2.tough_layout_cases failure on chromium.perf Android K |
||||||||||||||||||||||
Issue descriptionRevision range first seen: N5: 435483:435555 N7v2: 435398:435503 Link to failing step log: https://uberchromegw.corp.google.com/i/chromium.perf/builders/Android%20Nexus7v2%20Perf%20%282%29/builds/3445/steps/page_cycler_v2.tough_layout_cases/logs/stdio https://uberchromegw.corp.google.com/i/chromium.perf/builders/Android%20Nexus5%20Perf%20%282%29/builds/4512/steps/page_cycler_v2.tough_layout_cases/logs/stdio Traceback (most recent call last): File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/story_runner.py", line 87, in _RunStoryAndProcessErrorIfNeeded state.RunStory(results) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py", line 299, in RunStory self._current_page.Run(self) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/page/__init__.py", line 107, in Run shared_state.page_test.RunNavigateSteps(self, current_tab) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/page/legacy_page_test.py", line 195, in RunNavigateSteps page.RunNavigateSteps(action_runner) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/page/__init__.py", line 116, in RunNavigateSteps url, script_to_evaluate_on_commit=self.script_to_evaluate_on_commit) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/actions/action_runner.py", line 177, in Navigate timeout_in_seconds=timeout_in_seconds)) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/actions/action_runner.py", line 56, in _RunAction action.RunAction(self._tab) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/actions/navigate.py", line 29, in RunAction time_left_in_seconds) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py", line 88, in WaitForDocumentReadyStateToBeInteractiveOrBetter 'document.readyState == "complete"', timeout) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function return func(*args, **kwargs) File "/b/rr/tmpqJGypr/w/src/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py", line 142, in WaitForJavaScriptExpression e.message + '\n' + debug_message) TimeoutException: Timed out while waiting 59s for IsJavaScriptExpressionTrue. Console output: <no console output> If the test is disabled, please downgrade to Pri-2.
,
Dec 6 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8994009563680634608
,
Dec 6 2016
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40464172b1011e8b8e47b9776703bdb1eb23061e commit 40464172b1011e8b8e47b9776703bdb1eb23061e Author: sullivan <sullivan@chromium.org> Date: Tue Dec 06 17:47:22 2016 Disable mossiella.com in tough_layout_cases. It is failing on Android K. BUG= 671645 Review-Url: https://codereview.chromium.org/2557793002 Cr-Commit-Position: refs/heads/master@{#436632} [modify] https://crrev.com/40464172b1011e8b8e47b9776703bdb1eb23061e/tools/perf/page_sets/tough_layout_cases.py
,
Dec 6 2016
,
Dec 7 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Detect floats to avoid or clear due to negative margin top Author : robhogan Commit description: When a negative margin top pushes a block back up into its previous siblings we need to check for any floats in those siblings it now needs to avoid or clear. Previously we were just looking at its neighbour, we need to keep looking until we reach a sibling that we don't overlap. BUG= 666487 Review-Url: https://codereview.chromium.org/2531953002 Cr-Commit-Position: refs/heads/master@{#435497} Commit : 3c8d298acf826fc5337c526b1016a03b37c2656a Date : Thu Dec 01 00:42:03 2016 ===== TESTED REVISIONS ===== Revision Exit Code Std Dev N Good? chromium@435398 0 N/A 20 good chromium@435477 0 N/A 20 good chromium@435487 0 N/A 20 good chromium@435492 0 N/A 20 good chromium@435495 0 N/A 20 good chromium@435496 0 N/A 20 good chromium@435497 1 N/A 20 bad <-- chromium@435516 1 N/A 20 bad chromium@435555 1 N/A 20 bad Bisect job ran on: android_nexus7_perf_bisect Bug ID: 671645 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...www.mossiella.com page_cycler_v2.tough_layout_cases Test Metric: timeToFirstContentfulPaint_avg/pcv1-cold/http___www.mossiella.com Relative Change: 0.00% Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3535 Job details: https://chromeperf.appspot.com/buildbucket_job_status/8994009563680634608 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5831547628814336 | 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 Tests>AutoBisect. Thank you!
,
Dec 7 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Detect floats to avoid or clear due to negative margin top Author : robhogan Commit description: When a negative margin top pushes a block back up into its previous siblings we need to check for any floats in those siblings it now needs to avoid or clear. Previously we were just looking at its neighbour, we need to keep looking until we reach a sibling that we don't overlap. BUG= 666487 Review-Url: https://codereview.chromium.org/2531953002 Cr-Commit-Position: refs/heads/master@{#435497} Commit : 3c8d298acf826fc5337c526b1016a03b37c2656a Date : Thu Dec 01 00:42:03 2016 ===== TESTED REVISIONS ===== Revision Exit Code Std Dev N Good? chromium@435483 0 N/A 20 good chromium@435492 0 N/A 20 good chromium@435495 0 N/A 20 good chromium@435496 0 N/A 20 good chromium@435497 1 N/A 20 bad <-- chromium@435501 1 N/A 20 bad chromium@435519 1 N/A 20 bad chromium@435555 1 N/A 20 bad Bisect job ran on: android_nexus5_perf_bisect Bug ID: 671645 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...www.mossiella.com page_cycler_v2.tough_layout_cases Test Metric: timeToFirstContentfulPaint_avg/pcv1-cold/http___www.mossiella.com Relative Change: 0.00% Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/4411 Job details: https://chromeperf.appspot.com/buildbucket_job_status/8994009643695332160 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5847694659026944 | 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 Tests>AutoBisect. Thank you!
,
Dec 7 2016
robhogan: this CL is causing a Chrome crash on Android K, both on nexus 5 and nexus 7. You can repro with the test commmand from bisect comment: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...www.mossiella.com page_cycler_v2.tough_layout_cases
,
Dec 7 2016
I would have expected https://codereview.chromium.org/2547933003 to solve issues like this (though I can't recreate it locally). Looks like I don't have access to these tests either - pdr, can you help me out here by recreating locally or adding a copy of the 'story' (i.e. test) to the bug?
,
Dec 7 2016
It looks like this is indeed a test my fix did not cure: https://chromeperf.appspot.com/report?sid=abd0f887ad0b9d1de1a52c6d8ad14743fd9e77e69e04dc54ee38d7ce92282c2b pdr/kouhei - I don't have the gsutil credentials to access the test case so will definitely need you to help me with access or add something to the bug so I can progress.
,
Dec 7 2016
@Robhogan, I don't think we're able to give access to gsutil without some lawyer signoff. I'm going to try to see if I can get to the bottom of this for you. It looks like we have both a crash and a perf failure. Per Annie, the android bot is crashing with a tombstone but is not reporting a stack. @mikecase and @rnephew, can you help me get a stacktrace off this bot? In parallel, I will look at the perf regression on mac. It may be the same underlying issue.
,
Dec 8 2016
I was able to confirm this regression locally (macOS). I used archive.org to grab a publicly available copy of the page that roughly matches the one in our perf test archive (attached: mossiella.htm). The regression can be seen using devtools by just measuring the time spent in layout. The page has a ridiculous number of floating elements and the patches [1,2] introduced an O(n^2) where we iterate over each block, and then each block iterates over all previous blocks (calling addOverhangingFloats). I think we can optimize around this pathological behavior. I'm going to go ahead and roll out the two patches for now, and unskip the perf test. [1] https://crrev.com/3c8d298acf826fc5337c526b1016a03b37c2656a [2] https://crrev.com/eb8f963f9a5030c0007500620eefaad7c2c2c490 I think the android crash was a red herring; the test was just timing out. @sullivan, can you confirm this?
,
Dec 8 2016
I think that's right, there is a huge regression on mossiella page on mac bot: https://chromeperf.appspot.com/report?sid=d4721853bbdfc9f08c06a3b618edad911bb41c4037b3a137099850613befb762 It's seem like this regression makes it too slow on Android K, hence cause the timeout (see the stack in #0, it timed out while waiting for 'document.readyState == "complete"')
,
Dec 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24f05ee67737fcb83104ae4c033f87d5703ff703 commit 24f05ee67737fcb83104ae4c033f87d5703ff703 Author: pdr <pdr@chromium.org> Date: Thu Dec 08 06:19:04 2016 Revert: Detect floats to avoid or clear due to negative margin top (and followup) This patch reverts the following two patches: 1) Detect floats to avoid or clear due to negative margin top https://crrev.com/3c8d298acf826fc5337c526b1016a03b37c2656a 2) Stop searching for overhanging floats when the sibling's floats no longer overlap https://crrev.com/eb8f963f9a5030c0007500620eefaad7c2c2c490 This revert is due to a perf regression in the layout of floats. A followup patch will unskip the mossiella perf test that was skipped due to this regression. BUG= 671645 ,670325, 666487 TBR=robhogan@chromium.org Review-Url: https://codereview.chromium.org/2556373002 Cr-Commit-Position: refs/heads/master@{#437194} [modify] https://crrev.com/24f05ee67737fcb83104ae4c033f87d5703ff703/third_party/WebKit/LayoutTests/TestExpectations [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-2-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-2.html [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-3-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-3.html [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-4-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-4.html [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-5-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-5.html [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-6-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-6.html [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-expected.txt [delete] https://crrev.com/f05414aa1d1379f03644218dd8421d860234d0a3/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top.html [modify] https://crrev.com/24f05ee67737fcb83104ae4c033f87d5703ff703/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
,
Dec 8 2016
Rob, can you handle re-landing this patch? The original patch and overall idea is fine, just needs some tweaking to avoid O(n^2) (possibly rethinking the "lowestFloat > logicalTop" conditional with this pathalogical case in mind).
,
Dec 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9b43f612b231a42d0d5abb053d521ddeb8d0421 commit b9b43f612b231a42d0d5abb053d521ddeb8d0421 Author: pdr <pdr@chromium.org> Date: Thu Dec 08 23:00:12 2016 Revert of Disable mossiella.com in tough_layout_cases. (patchset #1 id:1 of https://codereview.chromium.org/2557793002/ ) Reason for revert: The perf regression has been reverted (see: crbug.com/671645 ) so we can re-enable this patch. Original issue's description: > Disable mossiella.com in tough_layout_cases. > > It is failing on Android K. > > BUG= 671645 > > Committed: https://crrev.com/40464172b1011e8b8e47b9776703bdb1eb23061e > Cr-Commit-Position: refs/heads/master@{#436632} TBR=nednguyen@google.com,sullivan@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 671645 Review-Url: https://codereview.chromium.org/2563773002 Cr-Commit-Position: refs/heads/master@{#437390} [modify] https://crrev.com/b9b43f612b231a42d0d5abb053d521ddeb8d0421/tools/perf/page_sets/tough_layout_cases.py
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c6452a360ed086b96680a2cad532e2be7d68385 commit 5c6452a360ed086b96680a2cad532e2be7d68385 Author: robhogan <robhogan@gmail.com> Date: Mon Dec 12 23:20:15 2016 Re-land "Detect floats to avoid or clear due to negative margin top (and followup)" Re-land https://codereview.chromium.org/2531953002 and https://codereview.chromium.org/2547933003. When a negative margin top pushes a block back up into its previous siblings we need to check for any floats in those siblings it now needs to avoid or clear. Previously we were just looking at its neighbour, we need to keep looking until we reach a sibling that we don't overlap. Fix the performance regression on mossiella.com in page_cycler_v2.tough_layout_cases by only looking for floats if margin collapsing has actually moved the child up. BUG= 671645 , 670325, 666487 Review-Url: https://codereview.chromium.org/2568643002 Cr-Commit-Position: refs/heads/master@{#437951} [modify] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-2-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-2.html [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-3-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-3.html [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-4-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-4.html [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-5-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-5.html [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-6-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-6.html [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top-expected.txt [add] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/LayoutTests/fast/block/float/avoid-floats-when-negative-margin-top.html [modify] https://crrev.com/5c6452a360ed086b96680a2cad532e2be7d68385/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
,
Dec 14 2016
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Dec 6 2016