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

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


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

android-one

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8997520215794355136

=== Auto-CCing suspected CL author yunchao.he@intel.com ===

Hi yunchao.he@intel.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 : Remove runtime flag 'UnsafeES3APIs' from blink webgl.
Author  : yunchao.he
Commit description:
  
This is the first step to remove unsafe mode and enable
ES3 APIs for WebGL2 and ES3 context by default.
Command line option "--enable-unsafe-es3-apis" would be
unnecessary and removed afterwards.

Intent to implement:
https://www.chromestatus.com/features/6694359164518400

Launch bug:
http://crbug.com/641635
(Note that this is not the launch CL -- it's still not possible to create a WebGL 2.0 context without the flag)

BUG= 654787 , 641635
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2451943002
Cr-Commit-Position: refs/heads/master@{#428253}
Commit  : 58dd74648c572ceaf28175b2a2802d3c50a9b316
Date    : Fri Oct 28 02:27:13 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@428230  3389962  15487.0  5  good
chromium@428245  3401272  0.0      5  good
chromium@428249  3395692  12477.3  5  good
chromium@428251  3401198  162.348  5  good
chromium@428252  3401273  1.78885  5  good
chromium@428253  3451588  0.0      5  bad    <--
chromium@428260  3451588  0.0      5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 660562

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_chrome:v8:effective_size_avg/foreground/http_m_intl_taobao_com_group_purchase_html
Relative Change: 1.82%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1770
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8997520215794355136


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

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

Adding reviewers of code change to cc list.

I think my patch at https://codereview.chromium.org/2451943002/ will not cause any regression. The only one exception is that the perf test will iterate all APIs exposed by Blink, just like the WebKit Layout test at src/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt. And that slight perf regression is expected. 

WDYT, zmo@, kbr@ and all reviewers? 

It's expected that Yunchao's CL will increase memory consumption slightly. It adds WebGL2RenderingContext to the Window object, which adds many methods and attributes visible to JavaScript.

To confirm this, a run can be done with the earlier build (chromium@428252) with --enable-unsafe-es3-apis . If it has essentially the same memory consumption as the build with Yunchao's CL then this should be closed as WontFix.

@miu, could you retest the perf by running chromium with --enable-unsafe-es3-apis command line option w/ and w/o my patch, then compare the result? 

My perf sheriff shift has long since ended. Perhaps kbr@ or some other Google colleagues could work with you on this?

I ran the following configurations:

./tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile_stress --output-dir=mobile-stress-428246

./tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile_stress --output-dir=mobile-stress-428258

./tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile_stress --extra-browser-args="--enable-unsafe-es3-apis" --output-dir=mobile-stress-428246-enable-unsafe-es3-apis

The result JSON files are attached.

I scanned through them but the memory numbers look pretty highly variable so I can't make comparisons. Annie, are there tools to do this with local JSON files? Could you find someone to help analyze them?

Deleted: mobile-stress-results.zip 286 KB

mobile-stress-results.zip 286 KB Download

(All of the above runs were done on a Nexus 5X)

We lazy allocate interfaces so the size of the new context type doesn't
matter. 1.5% is also massive, a few new interfaces shouldn't be increasing
memory usage that much. Something more is definitely going on here. :)

picksi, can someone on your team take a look at the results in #9, since this affects android memory?

+Juan, is this regression something we've seen elsewhere? The regression is very unclear on some graphs, and a couple of them have improved significantly.

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8994654786959779376

That v8 regression is small enough (<100KiB), I think it shouldn't be a big concern.

I've kicked off a bisect on another test with a much larger relative change (indexeddb_perf, somehow got merged into this bug too) just to see if there is anything there.

=== Auto-CCing suspected CL author yunchao.he@intel.com ===

Hi yunchao.he@intel.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 : Remove runtime flag 'UnsafeES3APIs' from blink webgl.
Author  : yunchao.he
Commit description:
  
This is the first step to remove unsafe mode and enable
ES3 APIs for WebGL2 and ES3 context by default.
Command line option "--enable-unsafe-es3-apis" would be
unnecessary and removed afterwards.

Intent to implement:
https://www.chromestatus.com/features/6694359164518400

Launch bug:
http://crbug.com/641635
(Note that this is not the launch CL -- it's still not possible to create a WebGL 2.0 context without the flag)

BUG= 654787 , 641635
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2451943002
Cr-Commit-Position: refs/heads/master@{#428253}
Commit  : 58dd74648c572ceaf28175b2a2802d3c50a9b316
Date    : Fri Oct 28 02:27:13 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@428235  45756.0  11016.0  8  good
chromium@428251  43325.5  6910.04  8  good
chromium@428252  47507.5  17814.6  8  good
chromium@428253  63024.0  8751.12  5  bad    <--
chromium@428255  62846.4  10627.7  5  bad
chromium@428259  62823.2  14999.2  5  bad
chromium@428267  63605.6  10836.2  5  bad
chromium@428298  60336.8  5231.27  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 660562

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests indexeddb_perf
Test Metric: vm_private_dirty_final_renderer/vm_private_dirty_final_renderer
Relative Change: 31.87%

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


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

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

Yunchao won't be able to take action on this bug.

This was one of the CLs required to ship WebGL 2.0 in  Issue 295792 . As esprehn@ pointed out in #11 above, this exposed some new and large interfaces to JavaScript, but theoretically it should not have had this sort of memory impact.

I attempted to diagnose this locally in #9 above, but I don't know how to read the results.

Please tell me what the next step is on this bug. It is not feasible to revert https://codereview.chromium.org/2451943002 -- the feature has shipped, and it is not workable to remove it from M56.

Regarding the regressions in memory.top_10_mobile_*, I think they are small, so no need of further action there. +memory folks FYI.

+cmumford, listed as owner of indexeddb_perf, to make a call about the 31% regression on that benchmark.

+1 to juan comments, from the bisect results it looks like we are all sitting here discussing about ~60 KB. 
IMHO it would have been worth investigating more if this was an unintended side-effect of a CL (e.g. if we spot +60K for what should have been a no-op refactoring)
This one though seems an actual feature and IMHO 60k for a feature seems a reasonable cost.
Hence I'm closing this bug. benhenry@: feel free to reopen if you disagree.

FWIW I disagree with #11:
> We lazy allocate interfaces so the size of the new context type doesn't matter. 
Well it clearly showed up in a benchmark so apparently it does matter.

> 1.5% is also massive, a few new interfaces shouldn't be increasing memory usage that much.
You see, this is the key problem here: % values are misleading in bugs.
1.5% *looks* massive. The problem here that this was a page where the memory used by v8 was really small (~5 MB), so 60 KB shows as a scary 1.5%
If would have been actually massive if this was 1.5% of a page with one order of magnitude higher footprint.

Thanks very much Primiano and Juan, Primiano in particular for pointing out what the numbers actually meant. For people not familiar with these benchmarks, some sort of explanation of what the measurements are (i.e., "V8 heap size") would be really helpful.