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

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


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

android-nexus5X
chromium-rel-mac11
chromium-rel-mac11-air
chromium-rel-mac12-mini-8gb
chromium-rel-win7-dual
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-nvidia
chromium-rel-win7-x64-dual
chromium-rel-win8-dual
linux-release
win-high-dpi

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

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

Hi alexmos@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Alex Moshchuk
  Commit : 9cf0ac139cc65890b9e036f704040c8da332d6e3
  Date   : Wed Oct 25 21:26:19 2017
  Subject: Add field trial testing config for sign-in process isolation.

Bisect Details
  Configuration: win_x64_perf_bisect
  Benchmark    : system_health.memory_desktop
  Metric       : memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg/load_media/load_media_dailymotion
  Change       : 15.81% | 28647424.0 -> 33177600.0

Revision             Result                   N
chromium@511537      28647424 +- 3555894      6      good
chromium@511568      28880441 +- 6880413      9      good
chromium@511583      28647424 +- 4689374      6      good
chromium@511591      28123136 +- 1284238      6      good
chromium@511593      29608619 +- 5212207      6      good
chromium@511594      34138795 +- 4155899      6      bad       <--
chromium@511595      33876651 +- 1210791      6      bad
chromium@511598      34400939 +- 3370700      6      bad
chromium@511658      33177600 +- 0.0          6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.media.dailymotion system_health.memory_desktop

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8964327762706344928


For feedback, file a bug with component Speed>Bisection

 Issue 779479  has been merged into this issue.

 Issue 779476  has been merged into this issue.

The CL in #3 turned on sign-in process isolation by default on the waterfall.  This mode gives a dedicated process to accounts.google.com, and some changes in memory measurements are expected: namely, we should be creating more processes, which should be smaller on average.  A writeup and some analysis of metrics is available at go/sign-in-isolation-experiments.

I'm not yet sure how these memory metrics are measured.  Maybe if they add up memory use across all renderer processes, that increases due to having more processes around?  I'll look into that and also try to reproduce these stories, to see if accounts.google.com processes are indeed being used.

After looking closer at the traces before/after the change, I think all these changes are expected and due to the extra renderer process created for an accounts.google.com subframe.  I've also verified locally that, for example,  load.media.dailymotion and load.media.9gag stories create an extra subframe renderer for a.g.c, visible in the process manager, midway through loading the main page.  For 9gag.com, the iframe looks like this:

<iframe name="oauth2relay456413259" id="oauth2relay456413259" src="https://accounts.google.com/o/oauth2/postmessageRelay?parent=https%3A%2F%2F9gag.com&amp;jsh=m%3B%2F_%2Fscs%2Fapps-static%2F_%2Fjs%2Fk%3Doz.gapi.en_US.ObNtI41Gjjg.O%2Fm%3D__features__%2Fam%3DQQ%2Frt%3Dj%2Fd%3D1%2Frs%3DAGLTcCPgxy2SeuqsTM8ifu5PMBACHIKN_A#rpctoken=750387659&amp;forcesecure=1" tabindex="-1" aria-hidden="true" style="width: 1px; height: 1px; position: absolute; top: -100px;"></iframe>

msarda@/ewald@: I'm curious what the use case is for such iframes?  These benchmarks don't run with the user signed in, but in all cases the page has a sign-in link/button, and there's an option to sign in through google.  I'm guessing these hidden iframes are used for this?

The difference in memory metrics is very likely caused by measuring cumulative memory across two renderer processes instead of one.  For example, looking at the memory dump in the traces for:

ChromiumPerf/chromium-rel-win7-gpu-nvidia/system_health.memory_desktop / memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg / load_media / load_media_9gag

Before enabling sign-in isolation, there's one renderer process, and looking at its memory dump's v8 cell, effective_size is 53388K.  After enabling sign-in isolation, there are two renderer processes: for main frame process, v8 effective_size is 49292K, and for the accounts.google.com subframe process, v8 effective_size is 7044K, for a total of 56336K, which seems to match the increase shown in the graph fairly closely.  I.e., the extra renderer process carries some memory overhead and, as expected, slightly outweighs the original process using a bit less memory.  In general, this kind of tradeoff is worth the added security benefits from site isolation.

FWIW, looking at data from the Win Dev Finch trial (14-day) for this (see go/sign-in-isolation-experiments) didn't show a statistically significant increase in Memory.Total.PrivateMemoryFootprint, which shows that in practice, we shouldn't see large increases in memory usage from this mode.  (We'll keep watching this data closely as we roll out the trial to Beta and Stable.)

It'd be great if someone from the perf/memory team can validate this explanation and confirm if this is ok to close as WontFix.

 Issue 779797  has been merged into this issue.

 Issue 779797  has been merged into this issue.

 Issue 779827  has been merged into this issue.

 Issue 779805  has been merged into this issue.

 Issue 779806  has been merged into this issue.

 Issue 779807  has been merged into this issue.

The explanation in #7 makes sense to me.

I am a bit surprised that ~5M increase on gmail and youtube don't show up on UMA. Is it maybe because the metric is dominated by using multiple tabs? 

In any case, if all the stake holders are informed that this will lead to a regression in memory of ~1 additional renderer per iframe of a.g.c., please close this is as WontFix yourself.

Ulan: Gmail background seems to be affected really bad (+117MB), any ideas?

CC+ Yanan and Nick as they may be able to reply to the question in comment https://bugs.chromium.org/p/chromium/issues/detail?id=779475#c7

>> msarda@/ewald@: I'm curious what the use case is for such iframes?  These benchmarks don't run with the user signed in, but in all cases the page has a sign-in link/button, and there's an option to sign in through google.  I'm guessing these hidden iframes are used for this?
I do not really know how Gmail or Youtube embed the a.g.c iframe when the user is signed out (this may also change in the future without us being aware of such a change). Maybe Nick or Yanan could help with this question.

+ another Nick since the iframes in question are for OAuth2 integrations

 Issue 779814  has been merged into this issue.

 Issue 779814  has been merged into this issue.

 Issue 780216  has been merged into this issue.

+Ryan to take a look at this thread (specifically comments #7 and #14) from a performance/memory perspective. Please cc anyone else you think should have eyes on this.

tl;dr: we're enabling site isolation for accounts.google.com as a security requirement for go/project-dice. This has the effect of creating one additional renderer process per a.g.c iframe, which in turn has a slight impact to overall memory usage.

The impact looks small, and we're measuring this as it goes through the waterfall without seeing statistically significant impacts to Memory.Total.PrivateMemoryFootprint. Do you have any other concerns?

Just to clarify: +1 to marking this as WontFix (WAI) from my perspective

+benhenry to take a look at the regression you're proposing we accept.

It does seem fairly significant since I imagine a.g.c has one or more iframes on pretty much every Google property. I'm not making these types of decisions anymore, but if I were I would want to know a bit more about the strategic value of go/project-dice before signing off. Ben, over to you :)

Just to reiterate, in practice we *don't* see a statistically significant regression here in the UMA data from our Canary/Dev trials.  Our Beta trial has just started, so there isn't much data yet to confirm, but so far the results are similar: while there's a slight increase in Memory.Total.PrivateMemoryFootprint, it's not statistically significant. E.g., see https://uma.googleplex.com/p/chrome/variations/?sid=de91c8cb13177774f9a4cd61f50bc399 for ~3 days of Beta trial data, and https://uma.googleplex.com/p/chrome/variations/?sid=1876251629fde2d902789f183264dc81 for 28 days of Dev trial data.

To clarify #20 a bit, all a.g.c iframes across all tabs will share the same process dedicated to a.g.c - this is a decision we explicitly made, ancitipating lots of tabs with Google properties having these iframes. :)   I'm guessing this contributes a lot towards what we see in the UMA data, as well the fact that those metrics come from sessions with multiple tabs.  The 5MB overhead from #7/#14 is specific to this benchmark's scenario of loading gmail in a single tab with nothing else open: in that case, yes, we pay the cost of spinning up an extra process for a.g.c.  

Woops, my apologies for mis-phrasing how many processes are created! Thanks for the clarification Alex :)

#23 explains why the impact is rather large on the benchmarks (single renderer browsing the web) vs insignificant in the wild. Thanks for clarifying!

Fine to WontFix from the benchmarks (V8) perspective.

Not sure whether it is enabled on Android or how it works there. In case its similar handling, you probably want to watch OOM rates as a few MB delta can make the difference between getting killed or not.

RE: #c25

Sign-In Isolation feature is disabled on Android - see r511562.

Thanks, closing as WontFix per #25.  If anyone has any other concerns, please bring them up.

 Issue 781155  has been merged into this issue.

 Issue 785269  has been merged into this issue.

 Issue 785269  has been merged into this issue.

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

=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Alex Moshchuk
  Commit : 9cf0ac139cc65890b9e036f704040c8da332d6e3
  Date   : Wed Oct 25 21:26:19 2017
  Subject: Add field trial testing config for sign-in process isolation.

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : system_health.memory_desktop
  Metric       : memory:chrome:renderer_processes:reported_by_chrome:v8:heap:allocated_objects_size_avg/load_media/load_media_9gag
  Change       : 7.73% | 23382666.6667 -> 25189665.3333

Revision             Result                  N
chromium@511551      23382667 +- 463444      6      good
chromium@511581      23481012 +- 297656      6      good
chromium@511589      23451129 +- 343312      6      good
chromium@511593      23457873 +- 390576      6      good
chromium@511594      25216321 +- 577800      6      bad       <--
chromium@511595      25208697 +- 208734      6      bad
chromium@511596      25197043 +- 381122      6      bad
chromium@511611      25233292 +- 198687      6      bad
chromium@511670      25189665 +- 425698      6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.media.9gag system_health.memory_desktop

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8962307712778567664


For feedback, file a bug with component Speed>Bisection