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

Issue 829813 link

Starred by 1 user
Status: Fixed
Owner:
mlamouri@chromium.org
Closed: Jul 23
Cc:
engedy@chromium.org
█ fsam...@chromium.org
liber...@chromium.org
mlamouri@chromium.org
sky@chromium.org
lethalantidote@chromium.org
agl@chromium.org
fdoray@chromium.org
primiano@chromium.org
hcarmona@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocked on:
issue 858826
issue 830828


Sign in to add a comment

30.5%-54.5% regression in system_health.memory_desktop at 548174:548316

Project Member Reported by primiano@chromium.org, Apr 6 2018 
See the link to graphs below.
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Apr 6 2018 

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=829813

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


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

chromium-rel-win10
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Apr 6 2018 

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/128babe8c40000
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Apr 13 2018

Cc: isherman@chromium.org mlamouri@chromium.org
Owner: mlamouri@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11e6f5c2c40000

Add fieldtrial testing config for video SurfaceLayer. by mlamouri@chromium.org
https://chromium.googlesource.com/chromium/src/+/2725fa428722a5c26d55d78cfc299069521ce78d

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Comment 6 by mlamouri@chromium.org, Apr 13 2018

Components: Internals>Compositing Internals>Media
Owner: lethalantidote@chromium.org

Comment 7 by mlamouri@chromium.org, May 16 2018

Blockedon: 830828
Labels: -M-67 M-68
It seems to be the same root cause as  bug 830828 .

Comment 8 by mlamouri@chromium.org, May 22 2018

Cc: liber...@chromium.org

Comment 9 by isherman@chromium.org, May 22 2018

Cc: -isherman@chromium.org

Comment 10 by mlamouri@chromium.org, May 25 2018

Cc: fsam...@chromium.org
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, May 29 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14b67d26240000
Project Member

Comment 14 by 42576172...@developer.gserviceaccount.com, May 29 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14d9b1ba240000
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, May 30 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/169074c6240000
Project Member

Comment 19 by 42576172...@developer.gserviceaccount.com, May 31 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/11a37c96240000
Project Member

Comment 20 by 42576172...@developer.gserviceaccount.com, May 31 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14cfb046240000
Project Member

Comment 22 by 42576172...@developer.gserviceaccount.com, Jun 1 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14828b96240000
Project Member

Comment 25 by 42576172...@developer.gserviceaccount.com, Jun 2 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14ac4cf6240000
Project Member

Comment 26 by 42576172...@developer.gserviceaccount.com, Jun 2 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14fb559e240000
Project Member

Comment 28 by 42576172...@developer.gserviceaccount.com, Jun 3 2018 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14dabc5e240000
Project Member

Comment 29 by bugdroid1@chromium.org, Jun 9 2018 

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

commit 74be4bc85264530ca32901400e3a50f9bf60dae3
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Sat Jun 09 03:31:07 2018

VideoSurfaceLayer: do not send a new frame until an ACK is received.

Bug:  829813 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4f4cb0df58a5d74a1f2d64abe309eadae38d8b06
Reviewed-on: https://chromium-review.googlesource.com/1075769
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565836}
[modify] https://crrev.com/74be4bc85264530ca32901400e3a50f9bf60dae3/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
[modify] https://crrev.com/74be4bc85264530ca32901400e3a50f9bf60dae3/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
[modify] https://crrev.com/74be4bc85264530ca32901400e3a50f9bf60dae3/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc

Comment 30 by mlamouri@chromium.org, Jun 11 2018

Cc: fdoray@chromium.org engedy@chromium.org sky@chromium.org lethalantidote@chromium.org agl@chromium.org hcarmona@chromium.org
 Issue 829815  has been merged into this issue.
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 27 2018 

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

commit a2b13fbc6b603643fa64a3e8564496ec79dea04c
Author: CJ DiMeglio <lethalantidote@chromium.org>
Date: Wed Jun 27 00:50:59 2018

Lets the SurfaceLayerBridge commuicate the opacity to the cc layer.

This CL is 1/3 in effort to fix the regressions caused by sending
unneeded compositor frames.

This CL gives the SurfaceLayerBridge control over setting the opacity.
We need this because we will be creating the SurfaceLayer before we have
video, and need to be able to set the opacity to false when we have no
frames.

Bug:  829813 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I9eed6b08be0b050ad5c735762d824728263ff091
Reviewed-on: https://chromium-review.googlesource.com/1114092
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570591}
[modify] https://crrev.com/a2b13fbc6b603643fa64a3e8564496ec79dea04c/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/a2b13fbc6b603643fa64a3e8564496ec79dea04c/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/a2b13fbc6b603643fa64a3e8564496ec79dea04c/third_party/blink/public/platform/web_surface_layer_bridge.h
[modify] https://crrev.com/a2b13fbc6b603643fa64a3e8564496ec79dea04c/third_party/blink/renderer/platform/graphics/surface_layer_bridge.cc
[modify] https://crrev.com/a2b13fbc6b603643fa64a3e8564496ec79dea04c/third_party/blink/renderer/platform/graphics/surface_layer_bridge.h
Project Member

Comment 32 by bugdroid1@chromium.org, Jul 4 

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

commit 96c18b698ee5f6bf462fbffdc684bba0bd70fcbe
Author: CJ DiMeglio <lethalantidote@chromium.org>
Date: Wed Jul 04 03:39:06 2018

Creates the SurfaceLayer early for Video.

This CL is 2/3 in effort to fix the regressions caused by sending
unneeded compositor frames.

Previously, we created the SurfaceLayer once we pushed the first compositor frame. That first
compositor frame was pushed whether we were on screen or not. We would like to prevent that.
In order to that, we must create the SurfaceLayer much earlier, such that the signal to not submit
frames is accessible.

This CL moves SurfaceLayer creation to just after SurfaceLayerBridge creation. Eventually we would
want it be during SurfaceLayerBridge creation, but it has yet to be determined if non-video code
paths can also move to this new model. The SurfaceLayer is given an id, but kept hidden until
we have videos to display. This allows us to have access to the information we need (to be used in
a subsequent CL).

Bug:  829813 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I49d320a93f26c6bbf94582c22c93539db1cfd5a1
Reviewed-on: https://chromium-review.googlesource.com/1115579
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572462}
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/content/renderer/media/media_factory.cc
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/media/blink/webmediaplayer_params.cc
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/media/blink/webmediaplayer_params.h
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/third_party/blink/public/platform/web_surface_layer_bridge.h
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/third_party/blink/renderer/platform/graphics/surface_layer_bridge.cc
[modify] https://crrev.com/96c18b698ee5f6bf462fbffdc684bba0bd70fcbe/third_party/blink/renderer/platform/graphics/surface_layer_bridge.h

Comment 33 by kbr@chromium.org, Jul 6

Blockedon: 858826

Comment 35 by mlamouri@chromium.org, Jul 18

Owner: mlamouri@chromium.org
Project Member

Comment 36 by 42576172...@developer.gserviceaccount.com, Jul 18 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/141f16e5a40000
Project Member

Comment 37 by bugdroid1@chromium.org, Jul 19 

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

commit 8b8a99818cc435403653b6ac259eca34b4b06f6a
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Thu Jul 19 02:22:43 2018

VideoSurfaceLayer: submit empty CompositorFrame before stopping frame submission.

This leads to better memory performance as the last frame will be discarded.

Bug:  829813 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7bc753e7e8d74d80d8c8390bd0f33ed45003ed5a
Reviewed-on: https://chromium-review.googlesource.com/1141291
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576331}
[modify] https://crrev.com/8b8a99818cc435403653b6ac259eca34b4b06f6a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
[modify] https://crrev.com/8b8a99818cc435403653b6ac259eca34b4b06f6a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
[modify] https://crrev.com/8b8a99818cc435403653b6ac259eca34b4b06f6a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
►

Sign in to add a comment