New issue
Advanced search Search tips

Issue 806826 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 830770
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4.7%-16.6% regression in thread_times.key_silk_cases at 531575:531699

Project Member Reported by primiano@chromium.org, Jan 29 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 29 2018

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

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


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

android-nexus6
android-one
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 29 2018

📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/16bc5e9a840000
Components: Internals>GPU>Metrics

Comment 6 by sadrul@chromium.org, Feb 12 2018

Labels: Needs-Feedback
Started a bisect with a wider range.
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Feb 15 2018

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14a9b6cd840000
Cc: weiliangc@chromium.org enne@chromium.org
Owner: enne@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1689c2a2440000

cc: Don't clamp tex coords for nearest neighbor filtering by enne@chromium.org
https://chromium.googlesource.com/chromium/src/+/0a7947a6984a2131616c5c24e7c6c4043ba60eb2

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/12fe5ce2440000
Project Member

Comment 13 by 42576172...@developer.gserviceaccount.com, Mar 21 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1780fb35440000

viz: Use texture clamping shader always by enne@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/972546/2

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

Comment 14 by bugdroid1@chromium.org, Apr 3 2018

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

commit ac2b2c6531e82191b77795f62280acddbf098c05
Author: Adrienne Walker <enne@chromium.org>
Date: Tue Apr 03 18:58:24 2018

viz: Use texture clamping shader always

https://chromium-review.googlesource.com/c/chromium/src/+/883944
restricted how often we would use shaders that clamped tex coords.
However, this confusingly regressed perf a little bit on Android,
which implies that program switching is expensive.

This patch changes GLRenderer to always texture clamp these quads
which reduces the number of shaders we use.

Bug:  806836 , 806826 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2b2e71b362d6a12f17758abe2edddf1759536016
Reviewed-on: https://chromium-review.googlesource.com/972546
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547780}
[modify] https://crrev.com/ac2b2c6531e82191b77795f62280acddbf098c05/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/ac2b2c6531e82191b77795f62280acddbf098c05/content/test/gpu/gpu_tests/pixel_expectations.py

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 6 2018

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

commit d669cd65fee123b17f001052130fe9ce43a21bd4
Author: Adrienne Walker <enne@chromium.org>
Date: Fri Apr 06 22:09:13 2018

Revert "viz: Use texture clamping shader always"

This reverts commit ac2b2c6531e82191b77795f62280acddbf098c05.

Reason for revert: causes texture filtering issues, even when the clamp value is infinite.
Bug: 828736

Original change's description:
> viz: Use texture clamping shader always
> 
> https://chromium-review.googlesource.com/c/chromium/src/+/883944
> restricted how often we would use shaders that clamped tex coords.
> However, this confusingly regressed perf a little bit on Android,
> which implies that program switching is expensive.
> 
> This patch changes GLRenderer to always texture clamp these quads
> which reduces the number of shaders we use.
> 
> Bug:  806836 , 806826 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I2b2e71b362d6a12f17758abe2edddf1759536016
> Reviewed-on: https://chromium-review.googlesource.com/972546
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Reviewed-by: Eric Karl <ericrk@chromium.org>
> Commit-Queue: enne <enne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547780}

TBR=kbr@chromium.org,enne@chromium.org,ericrk@chromium.org

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

Bug:  806836 ,  806826 
Change-Id: Ib12b7439cc8b1de70b39b42c43bc3b7f34f5dd92
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/998805
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548944}
[modify] https://crrev.com/d669cd65fee123b17f001052130fe9ce43a21bd4/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/d669cd65fee123b17f001052130fe9ce43a21bd4/content/test/gpu/gpu_tests/pixel_expectations.py

Comment 16 by enne@chromium.org, Apr 16 2018

Mergedinto: 830770
Status: Duplicate (was: Assigned)

Sign in to add a comment