New issue
Advanced search Search tips

Issue 617467 link

Starred by 0 users

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.6%-24.9% regression in smoothness.top_25_smooth at 397717:397765

Project Member Reported by rmcilroy@chromium.org, Jun 5 2016

Issue description

See the link to graphs below.
 
Cc: ericrk@chromium.org
Owner: ericrk@chromium.org

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

Hi ericrk@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 : Modify ImageTransportSurfaceOverlayMac to allow pipelining
Author  : ericrk
Commit description:
  
This change allows 1 additional frame of pipelining in
ImageTransportSurfaceOverlayMac by switching from a glFinish based
approach to a glFence based one.

Tested for CA framerate regressions - the following cases starve CA
when used with a simple glFlush. This solution (and the previous
glFinish) appear to successfully prevent CA starvation:

Animometer - bouncing png images:
https://trac.webkit.org/export/HEAD/trunk/PerformanceTests/Animometer/developer.html?test-interval=20&display=progress-bar&controller=adaptive&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=SVGsuite&test-name=SVGbouncingPNGimages&complexity=200
Both glFinish and glFence produce ~50 fps in CA/Chrome.

WebGL Liquid Face:
http://alteredqualia.com/xg/examples/liquid_face.html
For liquid face, note that with the glFinish, CA framerate is slightly
higher than Chrome reported framerate (20 vs 15) - with the new
approach, they match more closely (16 vs 15). This may be a negative,
but is still much better than the starvation which is seen with glFlush (5
vs 15).

Improvements:
Telemetry smoothness.top_25_smooth: This change produces good
improvements in first_gesture_scroll_update_latency,
mean_input_event_latency, and mean_main_thread_scroll_latency,
without any significant regressions. See:
https://drive.google.com/file/d/0B2nwXDxTDpGGU215X2xYTE9Bdnc/view?usp=sharing
Animometer Benchmark: This change produces good improvements in a
number of animometer benchmarks. See:
https://docs.google.com/spreadsheets/d/1qK6LfDVMKydbKfkGEi1DuGoSqY2wPJAjPiAL1CCUW-4/edit?usp=sharing

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2028303002
Cr-Commit-Position: refs/heads/master@{#397738}
Commit  : 54e8e3975826ceb886a7e0d8ec07bb38b1192311
Date    : Fri Jun 03 17:19:46 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N  Good?
chromium@397720  16.8022  0.151097   5  good
chromium@397731  16.7287  0.0756626  5  good
chromium@397737  17.2437  0.707986   8  good
chromium@397738  19.3509  1.83329    5  bad    <--
chromium@397739  18.5936  0.514154   5  bad
chromium@397740  19.3979  1.75774    8  bad
chromium@397742  20.1671  1.6815     8  bad
chromium@397763  20.9674  1.3171     5  bad

Bisect job ran on: mac_10_10_perf_bisect
Bug ID: 617467

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.tough_webgl_cases
Test Metric: frame_times/http___webglsamples.org_aquarium_aquarium.html
Relative Change: 24.79%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_10_perf_bisect/builds/2131
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9010662577497008480


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

| 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!
The blink_perf.bindings bug grouped in here for the linux-release bot is unrelated. This change only modified Mac related code.

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


===== SUSPECTED CL(s) =====
Subject : Modify ImageTransportSurfaceOverlayMac to allow pipelining
Author  : ericrk
Commit description:
  
This change allows 1 additional frame of pipelining in
ImageTransportSurfaceOverlayMac by switching from a glFinish based
approach to a glFence based one.

Tested for CA framerate regressions - the following cases starve CA
when used with a simple glFlush. This solution (and the previous
glFinish) appear to successfully prevent CA starvation:

Animometer - bouncing png images:
https://trac.webkit.org/export/HEAD/trunk/PerformanceTests/Animometer/developer.html?test-interval=20&display=progress-bar&controller=adaptive&frame-rate=50&kalman-process-error=1&kalman-measurement-error=4&time-measurement=performance&suite-name=SVGsuite&test-name=SVGbouncingPNGimages&complexity=200
Both glFinish and glFence produce ~50 fps in CA/Chrome.

WebGL Liquid Face:
http://alteredqualia.com/xg/examples/liquid_face.html
For liquid face, note that with the glFinish, CA framerate is slightly
higher than Chrome reported framerate (20 vs 15) - with the new
approach, they match more closely (16 vs 15). This may be a negative,
but is still much better than the starvation which is seen with glFlush (5
vs 15).

Improvements:
Telemetry smoothness.top_25_smooth: This change produces good
improvements in first_gesture_scroll_update_latency,
mean_input_event_latency, and mean_main_thread_scroll_latency,
without any significant regressions. See:
https://drive.google.com/file/d/0B2nwXDxTDpGGU215X2xYTE9Bdnc/view?usp=sharing
Animometer Benchmark: This change produces good improvements in a
number of animometer benchmarks. See:
https://docs.google.com/spreadsheets/d/1qK6LfDVMKydbKfkGEi1DuGoSqY2wPJAjPiAL1CCUW-4/edit?usp=sharing

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2028303002
Cr-Commit-Position: refs/heads/master@{#397738}
Commit  : 54e8e3975826ceb886a7e0d8ec07bb38b1192311
Date    : Fri Jun 03 17:19:46 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@397716  17.2132  0.520163  5  good
chromium@397729  17.1334  0.604746  5  good
chromium@397735  17.3415  0.801977  5  good
chromium@397737  16.9857  0.564532  5  good
chromium@397738  21.683   1.01548   5  bad    <--
chromium@397741  20.3734  0.53695   5  bad
chromium@397765  20.2228  1.43796   8  bad

Bisect job ran on: mac_10_10_perf_bisect
Bug ID: 617467

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.tough_webgl_cases
Test Metric: frame_times/http___webglsamples.org_aquarium_aquarium.html
Relative Change: 15.67%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_10_perf_bisect/builds/2139
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9010583787218770240


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

| 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!
Labels: -performance-sheriff Performance-Sheriff
FYI, this change also appears to be responsible for a huge increase in idle_wakeups on Mac:  issue 619976 .

Comment 6 Deleted

Comment 7 by ericrk@chromium.org, Jun 17 2016

This bug covers 4 issues:
1) The two smoothness.webgl issues are fixed by https://codereview.chromium.org/2064853002
2) The wordpress issue isn't fixed, but given that this is a very small regression (1.6%), and is a break-even on most other sites and for the overall metric, I think we shouldn't worry about this one. This regression is even smaller when compared against the pre-GPU-raster value (0.6%), and GPU-raster depends on this CL to prevent regressions in other areas.

3) The remaining issue is the idle wakeup issue. Note that this only occurs on one mac configuration (macHDD). On Mac11 and Mac10 we see an improvement (not a regression), and on MacRetina we see no change.

After some digging, I believe this case is exposing a metric bug on MacHDD, and not an actual regression in the overall idle_wakeups_gpu metric:

There is one test (video.html?src_tulip2.mp4) which is providing invalid (negative) idle wakeup counts before my patch. Because these invalid values are huge negative numbers (-90k), these throw the rest of the results off.

To get a better idea of the real impact of my change, I zeroed the negative value from the problem case and re-averaged the results before and after my change. If we zero this value, my patch shows around a ~25% improvement, not a regression at all. This matches two of the other mac bots (Mac10 and Mac11) which also show a 20-30% improvement.

Closing this bug as won't fix (it's a bit of a mix, two of the issues were actually fixed, two are won't fix).

Given that the bot is no longer reporting negative values after my change, not sure if we want to file a telemetry bug to track down where these negative values came from?

Sign in to add a comment