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

Issue 798310 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 797379
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

3036.2% regression in smoothness.gpu_rasterization_and_decoding.image_decoding_cases at 525532:525587

Project Member Reported by alexclarke@chromium.org, Jan 2 2018

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=798310

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=7a787aa4d9a78a8bb000ecb08b8c4e36bdf36580cbc723dbcd09f51ecc0a8004


Bot(s) for this bug's original alert(s):

android-webview-nexus5X
Cc: fmea...@chromium.org
Owner: fmea...@chromium.org
Status: Assigned (was: Untriaged)

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

Hi fmeawad@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 : Fadi Meawad
  Commit : 99a51fcacaad639093357727542fb1ef7c376a25
  Date   : Thu Dec 21 02:10:15 2017
  Subject: [PageLifecyle] Move the launch process state to Webkit/common

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : smoothness.gpu_rasterization_and_decoding.image_decoding_cases
  Metric       : frame_times/yuv_decoding.html
  Change       : 3048.06% | 25.4154419128 -> 800.0935

Revision             Result                  N
chromium@525531      25.4154 +- 1.80217      6      good
chromium@525559      25.6081 +- 1.65765      6      good
chromium@525561      25.6018 +- 1.42918      6      good
chromium@525562      25.5028 +- 1.26675      6      good
chromium@525563      834.816 +- 182.137      6      bad       <--
chromium@525566      799.949 +- 4.08231      6      bad
chromium@525573      835.261 +- 182.866      6      bad
chromium@525587      800.094 +- 6.08819      6      bad

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=yuv.decoding.html smoothness.gpu_rasterization_and_decoding.image_decoding_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8958520772884679888


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 3 2018

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

commit b85b48382bd9087a89afdc6fe0ac33dd71f1487e
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Wed Jan 03 02:18:33 2018

Revert "Reland "[PageLifecyle] Move the launch process state to Webkit/common""

This reverts commit e442ea0666d7af6e564fc1eb2c31deb05ed54035.

Reason for revert: 
This CL caused major regressions in android webview performance.
See  https://crbug.com/798310  

Original change's description:
> Reland "[PageLifecyle] Move the launch process state to Webkit/common"
> 
> This is a reland of 99a51fcacaad639093357727542fb1ef7c376a25
> 
> The failures described in  crbug.com/796940  was result of throttling
> the scheduled tasks as a result of the new backgrounding state.
> 
> The fix foregrounds the RendererScheduler before running the tests
> to avoid throttling.
> 
> Original change's description:
> > [PageLifecyle] Move the launch process state to Webkit/common
> >
> > This CL introduces Webkit/common/page/launching_process_state.h that
> > includes the launch background and launch boosted state.
> >
> > The reason this file is needed, is because the state is different per
> > OS but needs to be consistent between the RenderProcessHostImpl and
> > the RendererSchedulerImpl. To prevent future changes in one not
> > reflecting in the other, the code is moved to a common place and
> > shared.
> >
> > This CL fixes an issue where the scheduler assumes that it is
> > foregrounded while it is actually backgrounded during the initial
> > phase on Android.
> >
> > Bug:  chromium:780191 
> > Change-Id: Ib8dbfb6447f58c8b998a94ae2c69f8bc906d150f
> > Reviewed-on: https://chromium-review.googlesource.com/815678
> > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > Reviewed-by: Bo <boliu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#525563}
> 
> TBR=haraken@chromium.org,boliu@chromium.org
> 
> Bug:  chromium:780191 , chromium:796940 
> Change-Id: I117afad3b910239656dbc7e9dc3a1447291263a9
> Reviewed-on: https://chromium-review.googlesource.com/840340
> Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525841}

TBR=boliu@chromium.org,fmeawad@chromium.org,haraken@chromium.org,oysteine@chromium.org,altimin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
Change-Id: I0c8b7e075c29761d2a6f0d02461551b651682748
Reviewed-on: https://chromium-review.googlesource.com/847653
Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526586}
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator.h
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/common/BUILD.gn
[delete] https://crrev.com/8d5d19d557008d887d4ea133d9b80594908a0e3a/third_party/WebKit/common/page/launching_process_state.h

Mergedinto: 797379
Status: Duplicate (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 16 2018

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

commit e3b2cdc494c40bb7caf974806aad32043c7b6b62
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Tue Jan 16 21:44:42 2018

Fix Android Webview Rendrer Scheduler background state

When the renderer is run in process, the backgrounding state does not
propagate properly to the RendererScheduler. This CL enables updating
the RendererSchduler background state even if the renderer is run in
process.

This fix is required to reland:
https://chromium-review.googlesource.com/c/chromium/src/+/847653

Bug:  chromium:780191 ,  chromium:798310 
Change-Id: Ic4dc609de1884ecd9297ceb0b8774e4d4e313428
: 
Reviewed-on: https://chromium-review.googlesource.com/865769
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529505}
[modify] https://crrev.com/e3b2cdc494c40bb7caf974806aad32043c7b6b62/content/browser/renderer_host/render_process_host_impl.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 18 2018

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

commit bb30dbdbdf9a183ada3e129ffee817b8a6fd494a
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Thu Jan 18 22:04:33 2018

Reland(2): [PageLifecyle] Move the launch process state to Webkit/common

The first reland caused a major regression in Android Webview bots
running with a single process, since Webview had a bug that did not
propagate the background state properly to the renderer scheduler.

This bug was fixed in:
https://chromium-review.googlesource.com/c/chromium/src/+/865769

> Original change's description:
> > Reland "[PageLifecyle] Move the launch process state to Webkit/common"
> >
> > This is a reland of 99a51fcacaad639093357727542fb1ef7c376a25
> >
> > The failures described in  crbug.com/796940  was result of throttling
> > the scheduled tasks as a result of the new backgrounding state.
> >
> > The fix foregrounds the RendererScheduler before running the tests
> > to avoid throttling.
> >
> > Original change's description:
> > > [PageLifecyle] Move the launch process state to Webkit/common
> > >
> > > This CL introduces Webkit/common/page/launching_process_state.h that
> > > includes the launch background and launch boosted state.
> > >
> > > The reason this file is needed, is because the state is different per
> > > OS but needs to be consistent between the RenderProcessHostImpl and
> > > the RendererSchedulerImpl. To prevent future changes in one not
> > > reflecting in the other, the code is moved to a common place and
> > > shared.
> > >
> > > This CL fixes an issue where the scheduler assumes that it is
> > > foregrounded while it is actually backgrounded during the initial
> > > phase on Android.
> > >
> > > Bug:  chromium:780191 
> > > Change-Id: Ib8dbfb6447f58c8b998a94ae2c69f8bc906d150f
> > > Reviewed-on: https://chromium-review.googlesource.com/815678
> > > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > > Reviewed-by: Bo <boliu@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#525563}
> >
> > TBR=haraken@chromium.org,boliu@chromium.org
> >
> > Bug:  chromium:780191 , chromium:796940 
> > Change-Id: I117afad3b910239656dbc7e9dc3a1447291263a9
> > Reviewed-on: https://chromium-review.googlesource.com/840340
> > Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#525841}
>
> TBR=boliu@chromium.org,fmeawad@chromium.org,haraken@chromium.org,oysteine@chromium.org,altimin@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
> Change-Id: I0c8b7e075c29761d2a6f0d02461551b651682748
> Reviewed-on: https://chromium-review.googlesource.com/847653
> Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#526586}

TBR=boliu@chromium.org,altimin@chromium.org

Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
Change-Id: I213457c16ee09f272c5e188bc88e915e23ed40ea
Reviewed-on: https://chromium-review.googlesource.com/871370
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530289}
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator.h
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/common/BUILD.gn
[add] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/common/page/launching_process_state.h

Issue 798392 has been merged into this issue.
Issue 798393 has been merged into this issue.

Sign in to add a comment