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

Issue 838458 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.5%-66.3% regression in system_health.memory_desktop at 554371:554610

Project Member Reported by m...@chromium.org, May 1 2018

Issue description

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

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


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

android-nexus5
android-nexus5X
chromium-rel-win10
chromium-rel-win7-gpu-nvidia
📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/149f2783c40000
Cc: sunn...@chromium.org briander...@chromium.org
Owner: sunn...@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/149f2783c40000

Do not throttle BeginMainFrame on SubmitCompositorFrameAck by sunnyps@chromium.org
https://chromium.googlesource.com/chromium/src/+/3e2200faabd08eb0768c5a13604f60e7061410f8

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
load_games_bubbles story regressed in memory:chrome:renderer_processes:reported_by_chrome:skia:effective_size

after: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/load_games_bubbles_2018-04-30_20-21-24_80622.html
before: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/load_games_bubbles_2018-04-30_23-28-03_30814.html

The traces show that there's an extra 6M texture in skia, but it's purgeable. The non-purgeable memory remains the same so this might be a WontFix.

+ericrk Does the above sound right? Should we do something so that only non-purgeable memory triggers alerts?
Cc: ericrk@chromium.org
+ericrk for real
Oh, the context is that we were blocking BeginMainFrame on display compositor ack before, and now we're not. This should increase throughput, but also means an extra frame in flight at certain times. Should we do something special for Android Go? Same question also applies to main frame before activation which is currently enabled for >= 4 core devices.
Project Member

Comment 7 by bugdroid1@chromium.org, May 11 2018

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

commit 3d79596806759a857e72a476ecc883a650c3327d
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Fri May 11 04:17:31 2018

Revert "Do not throttle BeginMainFrame on SubmitCompositorFrameAck"

This reverts commit 3e2200faabd08eb0768c5a13604f60e7061410f8.

Reason for revert: Caused some regressions. piman@ and I have been rethinking how throttling works. Might make sense to hold this off until we have a solid plan.

Original change's description:
> Do not throttle BeginMainFrame on SubmitCompositorFrameAck
>
> The original motivation for this code was to workaround early swap ack
> on freon, but since freon isn't a thing anymore and we have a display
> compositor that actually performs the swap, it should be ok to get rid
> of it. This should improve throughput similar to main frame before
> activation.
>
> R=​brianderson
> BUG= 311213 
>
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I5108c74a57ba201d25999989de8da4c7f81a5176
> Reviewed-on: https://chromium-review.googlesource.com/1031892
> Reviewed-by: Brian Anderson <brianderson@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554547}

TBR=brianderson@chromium.org,sunnyps@chromium.org

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

Bug:  311213 ,838469, 838462 ,838461,838460, 838458 
Change-Id: I600b48b3b24a213954b563347dc3dc87dd143ad2
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1054987
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557791}
[modify] https://crrev.com/3d79596806759a857e72a476ecc883a650c3327d/cc/scheduler/scheduler_settings.cc
[modify] https://crrev.com/3d79596806759a857e72a476ecc883a650c3327d/cc/scheduler/scheduler_settings.h
[modify] https://crrev.com/3d79596806759a857e72a476ecc883a650c3327d/cc/scheduler/scheduler_state_machine.cc
[modify] https://crrev.com/3d79596806759a857e72a476ecc883a650c3327d/cc/scheduler/scheduler_state_machine_unittest.cc
[modify] https://crrev.com/3d79596806759a857e72a476ecc883a650c3327d/cc/scheduler/scheduler_unittest.cc

Status: Fixed (was: Assigned)
Reverted the change that caused this regression.

Sign in to add a comment