New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 675289 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: ----

Blocking:
issue 675369



Sign in to add a comment

telemetry_perf_unittests on Android failing on chromium.android/Android N5X Swarm Builder

Project Member Reported by rjkroege@chromium.org, Dec 17 2016

Issue description

telemetry_perf_unittests on Android failing on chromium.android/Android N5X Swarm Builder

Type: build-failure

Builders failed on: 
- Android N5X Swarm Builder: 
  https://build.chromium.org/p/chromium.android/builders/Android%20N5X%20Swarm%20Builder

Two telemetry tests failing

benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.background:news:nytimes
benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.search:portal:google

jasontiller@: Can you triage?
 
 
Cc: jasontiller@chromium.org
Owner: rnep...@chromium.org
+rnephew who is the current perf bot sheriff
benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.search:portal:google

The above test appears to be failing on every run since 7323. I started a return code bisect in the range of the first failure +- a a couple on both sides.
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Dec 17 2016

Cc: eseckler@chromium.org
Owner: eseckler@chromium.org

=== TEST FAILURE ===


=== Auto-CCing suspected CL author eseckler@chromium.org ===

Hi eseckler@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : [android] Make RWHVAndroid a BeginFrameObserver.
Author  : eseckler
Commit description:
  
Instead of directly observing the Window's vsyncs, RWHVAndroid now
observes a BeginFrameSource provided by the DelegatedFrameHost (clank)
or WindowAndroid (webview). To enable this, the BeginFrameSource
originally provided by CompositorImpl moves into WindowAndroid.

The patch also makes some changes to the BeginFrameSource's observer
iteration that are necessary to support
SynchronousBrowserCompositorFilter.

BUG= 401336 

Review-Url: https://codereview.chromium.org/2564403002
Cr-Commit-Position: refs/heads/master@{#439072}
Commit  : 1a6bd5037fc10e45646715d675e63146d23fc28d
Date    : Fri Dec 16 09:11:59 2016


===== TESTED REVISIONS =====
Revision         Exit Code  Std Dev  N  Good?
chromium@439070  0          N/A      2  good
chromium@439071  0          N/A      2  good
chromium@439072  1          N/A      2  bad    <--
chromium@439073  1          N/A      2  bad
chromium@439076  1          N/A      2  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 675289

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=search.portal.google system_health.common_mobile
Test Metric: timeToOnload_avg/search_portal/search_portal_google
Relative Change: 0.00%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/1002
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8993069553703984496


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5811158160769024

| 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!
Labels: -Pri-1 Pri-2
Went green after revert. Moving down to pri 2.
Blocking: 675369
Cc: nednguyen@chromium.org
Reverting this caused a crash seen in crbug.com/675369

Ned, in this case I'm guessing telemetry_perf_unittests is more important than a single benchmark failing a single page, so keep the revert in place?
Can you send a CL to disable smoke testing the benchmark instead? Look in benchmark_smoke_unittest.py
Sorry but please ignore my comment in #8. THe most important thing is this test caught a problem introduced https://codereview.chromium.org/2564403002, so let keep that revert.

For issue 675369, can you loop in Charlie? The failure there is not a browser crash but trace size explosion.
Labels: -Sheriff-Chromium Performance-Sheriff-Regressions
Cc: skyos...@chromium.org boliu@chromium.org
This is caused by a loop in RenderWidgetHostViewAndroid::OnBeginFrame(), caused by begin_frame_source_->AddObserver() during OnBeginFrame(), see stack trace below.

Sending a fixed patchset on https://codereview.chromium.org/2564403002/ for relanding.

Stack trace of the crash:
I    7.861s Main  Stack Trace:
I    7.862s Main    RELADDR   FUNCTION
I    7.862s Main    000000000006a144  tgkill+8
I    7.862s Main    00000000000678d4  pthread_kill+68
I    7.862s Main    0000000000023838  raise+28
I    7.862s Main    000000000001dfd8  abort+60
I    7.862s Main    v------>  DebugBreak
I    7.862s Main    0000000000867404  base::debug::BreakDebugger()
I    7.862s Main    0000000000881d68  logging::LogMessage::~LogMessage()
I    7.862s Main    0000000000850aac  base::android::GetClass(_JNIEnv*, char const*)
I    7.862s Main    0000000000850c40  base::android::GetJavaExceptionInfo(_JNIEnv*, _jthrowable*)
I    7.862s Main    0000000000850ea0  base::android::CheckException(_JNIEnv*)
I    7.862s Main    v------>  jni_generator::CheckException(_JNIEnv*)
I    7.862s Main    0000000001730954  Java_WindowAndroid_requestVSyncUpdate
I    7.862s Main    00000000017309d0  ui::WindowAndroid::RequestVSyncUpdate()
I    7.862s Main    00000000017310dc  ui::WindowAndroid::WindowBeginFrameSource::AddObserver(cc::BeginFrameObserver*)
I    7.862s Main    00000000024c34fc  content::RenderWidgetHostViewAndroid::AddBeginFrameRequest(content::RenderWidgetHostViewAndroid::BeginFrameRequestType)
I    7.862s Main    00000000024c3598  content::RenderWidgetHostViewAndroid::OnSetNeedsFlushInput()
I    7.862s Main    00000000023bc304  content::RenderWidgetHostImpl::SetNeedsFlush()
I    7.862s Main    0000000002361d34  content::SyntheticGestureController::Flush(base::TimeTicks)
I    7.862s Main    00000000023ba7c4  content::RenderWidgetHostImpl::FlushInput()
I    7.862s Main    00000000024c8fa8  content::RenderWidgetHostViewAndroid::OnBeginFrame(cc::BeginFrameArgs const&)
I    7.862s Main    0000000001731144  ui::WindowAndroid::WindowBeginFrameSource::AddObserver(cc::BeginFrameObserver*)
I    7.862s Main    00000000024c34fc  content::RenderWidgetHostViewAndroid::AddBeginFrameRequest(content::RenderWidgetHostViewAndroid::BeginFrameRequestType)
I    7.863s Main    00000000024c3598  content::RenderWidgetHostViewAndroid::OnSetNeedsFlushInput()
I    7.863s Main    00000000023bc304  content::RenderWidgetHostImpl::SetNeedsFlush()
I    7.863s Main    0000000002361d34  content::SyntheticGestureController::Flush(base::TimeTicks)
I    7.863s Main    00000000023ba7c4  content::RenderWidgetHostImpl::FlushInput()
I    7.863s Main    00000000024c8fa8  content::RenderWidgetHostViewAndroid::OnBeginFrame(cc::BeginFrameArgs const&)
I    7.863s Main    0000000001731144  ui::WindowAndroid::WindowBeginFrameSource::AddObserver(cc::BeginFrameObserver*)
I    7.863s Main    00000000024c34fc  content::RenderWidgetHostViewAndroid::AddBeginFrameRequest(content::RenderWidgetHostViewAndroid::BeginFrameRequestType)
I    7.863s Main    00000000024c3598  content::RenderWidgetHostViewAndroid::OnSetNeedsFlushInput()
I    7.863s Main    00000000023bc304  content::RenderWidgetHostImpl::SetNeedsFlush()
I    7.863s Main    0000000002361d34  content::SyntheticGestureController::Flush(base::TimeTicks)
I    7.863s Main    00000000023ba7c4  content::RenderWidgetHostImpl::FlushInput()
I    7.863s Main    00000000024c8fa8  content::RenderWidgetHostViewAndroid::OnBeginFrame(cc::BeginFrameArgs const&)
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84

commit 8c15fc3eb477cc05d742736cee65b6ea7e1c5f84
Author: eseckler <eseckler@chromium.org>
Date: Tue Dec 20 20:22:20 2016

[android] Make RWHVAndroid a BeginFrameObserver.

Instead of directly observing the Window's vsyncs, RWHVAndroid now
observes a BeginFrameSource provided by the DelegatedFrameHost (clank)
or WindowAndroid (webview). To enable this, the BeginFrameSource
originally provided by CompositorImpl moves into WindowAndroid.

The patch also makes some changes to the BeginFrameSource's observer
iteration that are necessary to support
SynchronousBrowserCompositorFilter.

BUG= 401336 ,  675289 

Committed: https://crrev.com/1a6bd5037fc10e45646715d675e63146d23fc28d
Review-Url: https://codereview.chromium.org/2564403002
Cr-Original-Commit-Position: refs/heads/master@{#439072}
Cr-Commit-Position: refs/heads/master@{#439877}

[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/android/synchronous_compositor_browser_filter.cc
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/android/synchronous_compositor_browser_filter.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/DEPS
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/window_android.cc
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/window_android.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/window_android_compositor.h
[modify] https://crrev.com/8c15fc3eb477cc05d742736cee65b6ea7e1c5f84/ui/android/window_android_observer.h

Status: Fixed (was: Assigned)
Looks like the bot survived the reland, closing this.
Labels: -Performance-Sheriff-Regressions Performance-Sheriff

Sign in to add a comment