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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg4M3ItgoM


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

android-nexus9

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


===== SUSPECTED CL(s) =====
Subject : Enable GPU Rasterization for content with any author defined viewport.
Author  : vmiura
Commit description:
  
BUG= 591179 

Review URL: https://codereview.chromium.org/1817583003

Cr-Commit-Position: refs/heads/master@{#382394}
Commit  : b338ee0b3125007b94854e44ed7b6df820c72a75
Date    : Mon Mar 21 21:26:30 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@382350         47363.2     397.123658  5           good
chromium@382390         47291.2     303.267539  5           good
chromium@382393         47604.8     309.417517  5           good
chromium@382394         48905.6     290.882794  5           bad
chromium@382395         49066.4     481.660461  5           bad
chromium@382400         49003.2     412.168897  5           bad
chromium@382410         49091.2     352.640327  5           bad
chromium@382430         48898.4     273.650872  5           bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 596881

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests webrtc.peerconnection
Test Metric: vm_private_dirty_final_renderer/vm_private_dirty_final_renderer
Relative Change: 3.24%
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/1687
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017481338625951280


| 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 label Cr-Tests-AutoBisect.  Thank you!

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

Hi vmiura@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 : Enable GPU Rasterization for content with any author defined viewport.
Author  : vmiura
Commit description:
  
BUG= 591179 

Review URL: https://codereview.chromium.org/1817583003

Cr-Commit-Position: refs/heads/master@{#382394}
Commit  : b338ee0b3125007b94854e44ed7b6df820c72a75
Date    : Mon Mar 21 21:26:30 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@382376         47093.6     229.147987  5           good
chromium@382390         47331.2     152.404724  5           good
chromium@382392         47532.8     401.002743  5           good
chromium@382393         47367.2     356.947055  5           good
chromium@382394         48692.8     223.452903  5           bad
chromium@382397         48836.8     443.354486  5           bad
chromium@382403         48613.6     323.735695  5           bad
chromium@382430         48972.8     247.780548  5           bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 596881

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests webrtc.peerconnection
Test Metric: vm_private_dirty_final_renderer/vm_private_dirty_final_renderer
Relative Change: 3.99%
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/1686
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017481360001212496


| 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 label Cr-Tests-AutoBisect.  Thank you!

vmiura: any update on this?

This benchmark seems to measure memory usage.  GPU Rasterization has a different memory profile from Software Rasterization.  I don't think this is a concern.

Here are related memory numbers.  Total vm memory is actually down ~11MB.

https://chromeperf.appspot.com/report?sid=18e3685b3a7543a44eb73c1acf28c66af12d1e6f2e8bca7074235799ad4992ed

+ericrk@ FYI

+primiano@ FYI

This is also causing a +8.6% increase on Android Graphics in webview on N5 (w.r.t. previous branch point).

Relevant graphs here:
https://chromeperf.appspot.com/report?sid=fcb0185bd1f94e5145d13721f5af6d6c74073dd2253ea1ceb13a0830a7a4f4e1&start_rev=378097&end_rev=384862

Overall pss is also slightly higher (+2%).

Note: this is different from issue 600353 (different commit range).

Hmm not sure I follow the evolution on this bug. From what I see:

- Two trybots in  #2 and #3 found crrev.com/1817583003 to be a culprit for the regression, which is typically a pretty strong sign that the regression is real.
- A bunch of other non-memory metrics are linked to this bug and they seem still regressed. Please look at https://chromeperf.appspot.com/group_report?bug_id=596881
- In #5 you pointed out that some metrics went down after this CL. This is good, but doesn't explain or relate with the fact the *other* metrics went up.

Unless I am missing something, I think this cannot be marked as WontFix. There are memory, latency and perf regressions still associated to this bug.
Or am I missing something?

#9 thanks for pointing these out.  The original issue I looked at was for the regression in "webrtc.peerconnection vm_private_dirty_final_renderer metric".  I'll go through the other metrics.

* rasterize_and_record_micro.top_25_smooth regression in 'rasterize_time' on  techcrunch.com

  Speaking with vmpstr@, there may be a problem in the methodology of this benchmark when using GPU rasterization.  We may be measuring Software Raster duration with GPU Raster tilings.  This doesn't make sense in real world usage.

  There is a ~5% increase in pixels rasterized.  This doesn't explain the ~50% regression in 'rasterize_time'.  Perhaps a data cache effect of the large tiles?

It's probably a good idea to focus first on the regression in foreground-memory_android_memtrack_gl_browser which is part of the system health plan, and is likely to become a release blocker for WebView.

I've built SystemWebViewGoogle.apk from tot, ran the benchmark on a Nexus 5, then manually reverted the changes in ViewportDescription.cpp, built and ran the benchmark again.

The revert does indeed bring foreground-memory_android_memtrack_gl_browser down by about 15.17%. The page with the largest relative change (-46%) is yahoo.com.

The results are available (compares tot vs change reverted):
https://x20web.corp.google.com/~perezju/memory_health/crbug596881/results.html

Sample trace before:
https://x20web.corp.google.com/~perezju/memory_health/crbug596881/tot/4.html

Sample trace after:
https://x20web.corp.google.com/~perezju/memory_health/crbug596881/reverted/4.html

vmiura, could you investigate if it's possible to implement an alternative that does not cause the regression or, less ideal, revert your change?

You can find info about the benchmark and how to run it here:
https://sites.google.com/a/google.com/chrome-speed-infra/home/memory-infra/memory-health-plan

Thanks perezju@.  I think we're aware that Ganesh uses more GL memory, it's a memory/performance trade-off.  The mode is blacklisted on Svelte.  It's not a new feature, but it's enabling this mode on more content.  A large % of content already uses this mode.

aelias@ do you think this will block WebView?

perezju: I get a 403 when I try to view the sample traces (I can only see the memory), could you change the file permissions?

Yes, it would block release unless we can make a special argument to system health council that the tradeoff in the metrics is a wash overall.  (That's unusual but there may be no way around it for Ganesh.)  But it seems hard to make that argument with the metrics perezju@ posted -- at least as far as memory goes, every category looks neutral or regressed.

#14 agreed, though Ganesh is already on for ~25% of pages.


It may be difficult to fully close the gap since Ganesh always has images travel through discardable, to non-discardable memory for GL textures.  The GL memory will increase, and the discardable won't decrease unless there is memory pressure.

#14 sorry about that! I've just fixed file permissions.

I am sheriffing today and tomorrow. Took the occasion to look in more details at the memory dashboards.
TL;DR is from the memory viewpoint is:
 - It seems a great win on Chrome N5 in foreground.
 - There seems to be something going wrong in background. Maybe we could release something and we don't anymore?
 - It seems bad on WebView N5 both foreground and background.


Chrome on Nexus5
----------------
# In foreground
  -(bad) Ashmem: +25% (850K) [1]
  -(bad) chrome-graphics ([2] for a description of what it measures): +1.9% [3]
  +(good) private-dirty -8.8% (-7.4M) [4] (nect effect visible on pages like ebay.com and vimeo.com)
  +(good) overall PSS -5.19%  (-7.4M) (Which is a reflection of private-diryt going down)

# In background
  - (bad) overall PSS +1% (+1M) [5]
  - (good) chrome-graphics -100% (-205 K)

WebView on Nexus 5
------------------
 # In foreground
  -(bad) Ashmem +32% (850K) [6]
  -(bad) Android Graphics: +17% (8M) [7]
  -(bad) Overall PSS: +0.95% (1M) [8]

 # In background
  -(bad) Android Graphics: +6% (1.2 M) [9]

[1] https://chromeperf.appspot.com/report?sid=99cbdc7438abc8819e89c1d2c092e1bcaae4d547a990151754ddbd45ca0014e2&start_rev=382175&end_rev=383231
[2] https://chromium.googlesource.com/chromium/src/+/master/components/tracing/docs/memory_infra_gpu.md
[3] https://chromeperf.appspot.com/report?sid=10e41a1212da3a1b7def2a2c30072f2b720af4237975f6da3d68d4b855b5102c&start_rev=381439&end_rev=383467
[4] https://chromeperf.appspot.com/report?sid=24415c6d7471f3d4459c37305080e536009fa50fd717e0c7c4fd4cec0b9abff8&start_rev=382214&end_rev=382822
[5] https://chromeperf.appspot.com/report?sid=a8fca748f409883c136070861b15c755a517526b0870df7d83261779f4a993d0&start_rev=382191&end_rev=383262
[6] https://chromeperf.appspot.com/report?sid=739f035812d7bc4186bc3cae183752df743e92e78373d52ff46d68b3ff67da97&start_rev=382191&end_rev=383262
[7] https://chromeperf.appspot.com/report?sid=fcb0185bd1f94e5145d13721f5af6d6c74073dd2253ea1ceb13a0830a7a4f4e1&start_rev=382191&end_rev=383262
[8] https://chromeperf.appspot.com/report?sid=0d0e8add0332932fe4b25e3e2dfa153992cf381cd2e4205f1ed9d5c66b60cafb&start_rev=382191&end_rev=383262
[9] https://chromeperf.appspot.com/report?sid=2f397c70356b5682fc3d3f859be4a7fd600f5ae4fe54b8e0889464af4c434744&start_rev=382191&end_rev=383262

Upping priority as we are close to the branch point and the memory changes into play are non-marginal.

+amineer as this is a release blocker for WebView.

A memory regression of this size will be almost impossible to get past the System Health plan team. An 8MB graphics regression is really substantial and will probably prevent the release of M51.

If the memory increase is intentional we would need an *extremely* good UX improvement to get System Health to sign off on it. Is there a trade off we can make here, e.g. a 50% UX improvement by using only 10% of this memory?

If the memory increase is a bug and we can't fix it in time we will probably need to roll this feature over to M52 and use the extra time to fix it.

I was chatting offline with vmiura@, an option could be disabling that on WebView only until we figure out why we are not getting any saving there.

Don't have enough background: might be a side effect of single-process / renderers not being recycled / gpu clients somehow sticking across page load?

Yeah I don't really understand why memory improves on chrome, but gets worse on webview.

Webview uses EGLImages for sharing. And maybe potentially how frame resources are returned to renderer are different, and webview is always in cc "high latency" mode. Really don't see how they can impact memory for gpu raster, in opposite ways. Can't think of anything else that's different on webview.

Sounds like not device specific either if this happens on both n5 and n9.

The main reason for extending GPU rasterization at this point is for developer ergonomics.  We'd like to remove a performance variable that developers find hard to reason about.  It's not going to have a significant performance impact on web pages that aren't built to take advantage of GPU rasterization.

I would rather not add more variability between Chrome & WebView, so I'm thinking we should revert the CL, and take more time to investigate the regressions.

Sounds fine to me to revert.  It sounds like just we have 2 "bugs" increasing memory usage in background and WebView.  I'm actually happy to hear that it's not a fundamental memory problem of GPU raster.  Once we investigate and fix the problems discovered here, we can turn the policy back on.

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

commit e707452627a8ace7951e49421be443fe544aac71
Author: vmiura <vmiura@chromium.org>
Date: Wed Apr 06 20:26:11 2016

Revert of Enable GPU Rasterization for content with any author defined viewport. (patchset #2 id:20001 of https://codereview.chromium.org/1817583003/ )

Reason for revert:
Reverting until we can debug memory health regressions on WebView.

BUG= 596881 

Original issue's description:
> Enable GPU Rasterization for content with any author defined viewport.
>
> BUG= 591179 
>
> Committed: https://crrev.com/b338ee0b3125007b94854e44ed7b6df820c72a75
> Cr-Commit-Position: refs/heads/master@{#382394}

TBR=aelias@chromium.org,pdr@chromium.org,chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 591179 

Review URL: https://codereview.chromium.org/1868563003

Cr-Commit-Position: refs/heads/master@{#385547}

[modify] https://crrev.com/e707452627a8ace7951e49421be443fe544aac71/third_party/WebKit/Source/core/dom/ViewportDescription.cpp
[modify] https://crrev.com/e707452627a8ace7951e49421be443fe544aac71/third_party/WebKit/Source/web/tests/ViewportTest.cpp
[delete] https://crrev.com/b3e75571785c6e135b184806c7fd19aadf7540c7/third_party/WebKit/Source/web/tests/data/viewport/viewport-gpu-rasterization-disabled-without-viewport.html
[add] https://crrev.com/e707452627a8ace7951e49421be443fe544aac71/third_party/WebKit/Source/web/tests/data/viewport/viewport-inferred-values-do-not-trigger-gpu-rasterization.html

Oh, one theory.. toggling gpu raster on and off causes all resources to be dropped webview, and if ganesh is on for all the test pages, then that no longer happens.

Webview is still doing the single process thing where navigation happens on the same blink::WebViewImpl instance, so also the same cc instance. *However*, app is sent to the background after each between each page load in the test. So this theory only holds if UpdateTreeResourcesForGpuRasterizationIfNeeded drops more resources things than SetVisibility. Is that true..?

This could also explain why it's webview only, since chrome starts a new renderer on cross-domain navigations.

Not sure if this is still the case - but I remember seeing this a while ago: crbug.com/501426 - might explain some of this.

Basically, when webview goes invisible, it doesn't forward the call through LTHI/Renderer, which makes us miss a bit of cleanup code that exists for main browser.

Confirming results post revert to close the perf loop:
- GPU raster seems effectively an overall win on Chrome (at least on the set of pages we have), as the saving of anonymous memory outweights the GL memory increase.
(See https://chrome-health.googleplex.com/health-plan/android-chrome/memory/nexus5/)

- GPU raster seems unrleated with the Chrome background PSS regression (https://chromeperf.appspot.com/report?sid=a8fca748f409883c136070861b15c755a517526b0870df7d83261779f4a993d0&start_rev=378097&end_rev=385606) which didn't go back after the revert. So the good news here is that for Chrome it is a full win.

- Sadly I confirm also that on webview we just get the lose part as explained in #17: ashmem, pss and GL recovered after the revert.

Can someone update on the above issue?

I really appreciate the help.

Thank you!

This is already fixed by the revert, just the label wasn't updated.  We can track fixing the regressions for the reenabling in other bugs.