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

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


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

chromium-rel-win7-gpu-nvidia

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

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

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

Hi creis@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 : Fix history nav to a script-injected about:blank frame.
Author  : Charles Reis
Commit description:
  
This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#426880}
Commit  : 37c954965f2f221aec294a0a28dfbba1c7543f2a
Date    : Fri Oct 21 20:43:14 2016


===== TESTED REVISIONS =====
Revision         Mean      Std Dev  N  Good?
chromium@426850  31942738  1426661  5  good
chromium@426868  31593676  638521   4  good
chromium@426877  31795937  731437   5  good
chromium@426879  31816908  840197   5  good
chromium@426880  38547292  2289645  5  bad    <--
chromium@426882  39618314  2578432  5  bad
chromium@426886  37751848  2141604  5  bad
chromium@426921  39388119  2345270  5  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 658661

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests system_health.memory_desktop
Test Metric: memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg/browse_news
Relative Change: 23.31%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1933
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8997934567001556224


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

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

===== BISECT JOB RESULTS =====
Status: failed


===== TESTED REVISIONS =====
Revision         Mean      Std Dev  N  Good?
chromium@426786  38485659  409332   8  good
chromium@426819  38737249  318999   8  good
chromium@426852  39336207  351540   5  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 658661

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests system_health.memory_desktop
Test Metric: memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg/memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg
Relative Change: 2.61%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1930
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8997934796635705536


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

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

I'm taking a look and trying to run these locally.

The fix itself is unlikely to cause a V8 heap regression, since it's just changing a set to a map within content/ (even though that data structure can get large due to large unique names as the keys).  The values of the map are just bools, so the incremental difference should be quite small.

*However*, if the benchmark involves going back/forward or restoring pages from history, and if happens to exercise  issue 657896  (which my CL is fixing), then I think I see why this would get blamed.  The bug being fixed is that ads aren't loading when you leave a page and come back.  That would obviously (and incorrectly) take less memory than if the ads loaded.

If that's the case, then this difference in memory use was actually detected by issue 639865 when the bug was introduced.  (We may have falsely attributed the memory improvement to passing around fewer PageStates and unique names, since we were unaware at the time that ads weren't loading on history navigations.)  This would just be putting things back to where they were, now that the bug is fixed and ads load again.

I'll try to confirm whether these benchmarks could have exercised that bug.

Ha, comment 6 is correct.  (Thanks for suggesting this as a theory, dcheng.)

I put a CHECK(false) in for the case that we would hit the bug (i.e., navigating back and finding an about:blank subframe history item to restore) and ran the tests locally.  The browse::news::cnn test crashed on it, because that benchmark apparently navigates to an article and then goes back to the main CNN page, which tries to load the ads from history items.

That indicates that the memory improvement found in issue 639865 (after turning on the new navigation path) was actually due to  issue 657896 , where we weren't loading the ads when going back.  Sigh!

The lesson here is that it's worth digging even further into memory improvements to see whether they're due to a bug.  In this case, we expected some difference in behavior but didn't nail it down to the exact cause.  If we had, we might have caught  issue 657896  sooner.  (I think this is worth discussing with the perf folks, since the benchmark output is so complex that I don't know how we would have identified this bug at the time.  Maybe they have some ideas.)

At any rate, we'll have to mark this as WontFix, because the memory improvement that we're losing was only due to a bug.