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

Issue 655673 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

1.1%-3.3% regression in memory.top_10_mobile at 424717:424762

Project Member Reported by lanwei@google.com, Oct 13 2016

Issue description

See the link to graphs below.
 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Oct 13 2016

Cc: skia-deps-roller@chromium.org
Owner: skia-deps-roller@chromium.org

=== Auto-CCing suspected CL author skia-deps-roller@chromium.org ===

Hi skia-deps-roller@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


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


===== SUSPECTED CL(s) =====
Subject : Roll src/third_party/skia/ 5772eaa91..7ab96e921 (2 commits).
Author  : skia-deps-roller
Commit description:
  
https://chromium.googlesource.com/skia.git/+log/5772eaa91d6a..7ab96e92196d

$ git log 5772eaa91..7ab96e921 --date=short --no-merges --format='%ad %ae %s'
2016-10-12 senorblanco GrTessellator: make inverse fill types more sane.
2016-10-12 ethannicholas Turned on SkSL->GLSL compiler GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2288033003

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=jvanverth@google.com

Review-Url: https://codereview.chromium.org/2419523002
Cr-Commit-Position: refs/heads/master@{#424733}
Commit  : ce86adf7be9ee2f397d9a1a093c0379ba6351a88
Date    : Wed Oct 12 14:47:03 2016


===== TESTED REVISIONS =====
Revision                         Mean      Std Dev  N   Good?
chromium@424732                  40086467  367618   12  good
chromium@424732,skia@7ab96e9219  39603139  917167   12  good
chromium@424733                  40712216  451512   12  bad    <--
chromium@424734                  40576877  509220   12  bad
chromium@424736                  40948376  596900   8   bad
chromium@424739                  40651416  297381   8   bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 655673

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile_stress
Test Metric: memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg/foreground/http_en_m_wikipedia_org_wiki_Science
Relative Change: 1.61%
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/4240
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8998902626694018736


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

| 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!
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 14 2016

Labels: Hotlist-Google
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Oct 18 2016

Cc: ethannicholas@google.com
Owner: ethannicholas@google.com

=== Auto-CCing suspected CL author ethannicholas@google.com ===

Hi ethannicholas@google.com, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


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


===== SUSPECTED CL(s) =====
Subject : Turned on SkSL->GLSL compiler
Author  : ethannicholas
Commit description:
  GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2288033003

Committed: https://skia.googlesource.com/skia/+/9b0fe3d125f237d9884732a48414fa85fc71b4e3
Committed: https://skia.googlesource.com/skia/+/b12b3c6908c62c908b3680be01e3b5bfd30de310
Committed: https://skia.googlesource.com/skia/+/f008b0a59f45c0d4bea3e66faf3b01805009ec89
Committed: https://skia.googlesource.com/skia/+/08b2ccf398e2b81bc05d2c105837e5419899469b
Committed: https://skia.googlesource.com/skia/+/dcfe6dba4a335e50e86ff68e3252065d4197432c
Committed: https://skia.googlesource.com/skia/+/ccb1dd8f267f9d7fe7c9d0ce222ebc81b41853b3
Review-Url: https://codereview.chromium.org/2288033003
Commit  : 5961bc9278a00e56dacdd9408d0744b5a0a3b493
Date    : Wed Oct 12 13:39:56 2016


===== TESTED REVISIONS =====
Revision                         Mean      Std Dev  N  Good?
chromium@424722                  26484154  85258.7  5  good
chromium@424732                  26420256  18559.6  5  good
chromium@424732,skia@5961bc9278  27078483  84000.5  5  bad    <--
chromium@424732,skia@7ab96e9219  27085651  28830.6  5  bad
chromium@424733                  27125382  85877.5  5  bad
chromium@424734                  27161632  52489.4  5  bad
chromium@424735                  27107155  48580.1  5  bad
chromium@424737                  27065581  48526.1  5  bad
chromium@424742                  27130502  45700.7  5  bad
chromium@424761                  27089133  35791.7  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 655673

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=load.search.yandex system_health.memory_mobile
Test Metric: memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg/load_search/load_search_yandex
Relative Change: 2.28%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/4254
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8998454975041296848


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

| 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!
Cc: primiano@chromium.org kerz@chromium.org jasontiller@chromium.org
Labels: -Pri-2 ReleaseBlock-Beta Pri-1
Note this CL seems to cause about ~800KB regression in overall pss, android graphics, private dirty and native heap. See:
https://chrome-health.googleplex.com/health-plan/android-chrome/memory/nexus5/?from_commit=424665&to_commit=424948

You should be able to reproduce the regression with the test command above (and try --output-format=html).
Should have a fix for this soon.
Any updates with the fix?
In the process of landing the patch in Skia as we speak, should be rolled into Chromium today.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762

commit cffaa70896fa5bc6c7bf98abbaafb1a755b49762
Author: ethannicholas <ethannicholas@google.com>
Date: Thu Oct 27 15:15:50 2016

Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG= 655673 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Review-Url: https://codereview.chromium.org/2442063002

[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/SkSLContext.h
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/SkSLIRGenerator.cpp
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/SkSLParser.cpp
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/SkSLToken.h
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/ir/SkSLFunctionCall.h
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/ir/SkSLFunctionDeclaration.h
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/ir/SkSLType.h
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/src/sksl/sksl.include
[modify] https://crrev.com/cffaa70896fa5bc6c7bf98abbaafb1a755b49762/tests/SkSLErrorTest.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/56380524d903f27627a75e2e1189463999725008

commit 56380524d903f27627a75e2e1189463999725008
Author: benjaminwagner <benjaminwagner@google.com>
Date: Thu Oct 27 16:08:06 2016

Revert of Reduced skslc memory consumption (patchset #3 id:50001 of https://codereview.chromium.org/2442063002/ )

Reason for revert:
texelFetch removed, but is used in some shaders.

Original issue's description:
> Reduced skslc memory consumption
>
> The big change here is smarter generic type handling which allows us to
> keep far fewer entries in the core symboltable. This also comments out
> a number of OpenGL builtin functions which Skia does not use and is
> unlikely to in the future.
> BUG= 655673 
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002
>
> Committed: https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762

TBR=reed@google.com,ethannicholas@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 655673 

Review-Url: https://codereview.chromium.org/2458723002

[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/SkSLContext.h
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/SkSLIRGenerator.cpp
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/SkSLParser.cpp
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/SkSLToken.h
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/ir/SkSLFunctionCall.h
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/ir/SkSLFunctionDeclaration.h
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/ir/SkSLType.h
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/src/sksl/sksl.include
[modify] https://crrev.com/56380524d903f27627a75e2e1189463999725008/tests/SkSLErrorTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 27 2016

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

commit 7a8aa74e0d0c01bdd56048b6bfe33e94bc1688d1
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Thu Oct 27 17:27:53 2016

Roll src/third_party/skia/ 96b38426b..56380524d (8 commits).

https://chromium.googlesource.com/skia.git/+log/96b38426b625..56380524d903

$ git log 96b38426b..56380524d --date=short --no-merges --format='%ad %ae %s'
2016-10-27 benjaminwagner Revert of Reduced skslc memory consumption (patchset #3 id:50001 of https://codereview.chromium.org/2442063002/ )
2016-10-27 liyuqian Do not skip fractional y for SkAAClip
2016-10-27 scroggo Revert "Always use a color table with 256 colors"
2016-10-27 egdaniel Default vulkan sdk to VULKAN_SDK env var in GN
2016-10-27 scroggo Fix decoding GIF to 565
2016-10-27 ethannicholas Reduced skslc memory consumption
2016-10-27 fmalita Avoid separate allocation of SkImageCacherator
2016-10-27 borenet recipes: Fix missing patch_set property

BUG= 655673 , 659883 , 659972 , 655673 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=reed@google.com

Review-Url: https://codereview.chromium.org/2455053003
Cr-Commit-Position: refs/heads/master@{#428068}

[modify] https://crrev.com/7a8aa74e0d0c01bdd56048b6bfe33e94bc1688d1/DEPS

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 28 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/471e89405b71f04e07ae2887bde061185e262c81

commit 471e89405b71f04e07ae2887bde061185e262c81
Author: ethannicholas <ethannicholas@google.com>
Date: Fri Oct 28 16:02:46 2016

Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG= 655673 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed: https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
Review-Url: https://codereview.chromium.org/2442063002

[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/SkSLContext.h
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/SkSLIRGenerator.cpp
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/SkSLParser.cpp
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/SkSLToken.h
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/ir/SkSLFunctionCall.h
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/ir/SkSLFunctionDeclaration.h
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/ir/SkSLType.h
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/src/sksl/sksl.include
[modify] https://crrev.com/471e89405b71f04e07ae2887bde061185e262c81/tests/SkSLErrorTest.cpp

Looks like #15 rolled into chromium at r428405, some memory measurements did drop around that change, but some new large regressions in graphics appeared. Those are being tracked in issue 660780 where a bisect is running to confirm if it's indeed due to this change.
Labels: OS-Android
Cc: benhenry@chromium.org
Adding Ben as an FYI
I don't seem to have permission to view 660780 -- did we determine whether this new regression is related to my changes or not?
Status: Assigned (was: Untriaged)
Ethan - try using your chromium.org login to view 660780.

We're having trouble bisecting on the other bug.
Do we feel the memory usage is sufficiently improved to close this?
Status: Fixed (was: Assigned)
I do think it's fine (metrics have recovered on all main {product, device} configurations). I would still like to investigate issue 660780, though. Will have a look in a couple of days and re-open if necessary.

Sign in to add a comment