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

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

Hi delphick@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 : delphick
  Commit : d8ade60fbe0ea954ecd5f186c36e6d0ac045c28c
  Date   : Tue Apr 11 09:56:28 2017
  Subject: No longer clamp setTimeout(..., 0) to 1ms.

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:browser_process:reported_by_os:system_memory:private_dirty_size_avg/background_search/background_search_google
  Change       : 3.02% | 29221074.6667 -> 30103080.0

Revision             Result                  N
chromium@463571      29221075 +- 181953      6      good
chromium@463588      29210152 +- 224870      6      good
chromium@463590      29346003 +- 295334      6      good
chromium@463591      30206845 +- 199880      6      bad       <--
chromium@463593      30289448 +- 137415      6      bad
chromium@463597      30163837 +- 186304      6      bad
chromium@463605      30043005 +- 171999      6      bad
chromium@463639      30053245 +- 243663      6      bad
chromium@463706      30103080 +- 168882      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=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=background.search.google system_health.memory_mobile

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8982481938747741088

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5346978201862144


| 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 Speed>Bisection.  Thank you!

Since my change makes setTimeout(..., 0) stop leaving 1ms gaps, it's possible GC was not able to run in these gaps which could have affected memory usage. I think it's unlikely this is a real issue as the GC will run later anyway.

Compiling full before and after traces to see what's really happening.

 Issue 711301  has been merged into this issue.

 Issue 711303  has been merged into this issue.

 Issue 711295  has been merged into this issue.

 Issue 711304  has been merged into this issue.

 Issue 711299  has been merged into this issue.

Issue 711856 has been merged into this issue.

Issue 711364 has been merged into this issue.

 Issue 712463  has been merged into this issue.

 Issue 712428  has been merged into this issue.

 Issue 713721  has been merged into this issue.

Seems like a number of other bisects have come to the same conclusion. Any progress in your investigation, Dan? May be worth reverting to see if the spike goes away.

I haven't had much luck tracing the memory regression. My main concern was with the firstMeaningfulPaint regression in the page_cycler tests for flickr:  Issue 711295 . Here I don't think the regression is real, but an artifact of the way the metric is calculated.

I'm wary reverting it as it might cause other problems. Apparently this change inadvertently fixed Issue 710095. If this seems serious enough perhaps Alexander could revert his change for that bug which would allow us to roll this back.

Forgot to say I'm on leave now till Wednesday so please just revert it if you think it's necessary.

If so many alerts point to this regressions unfortunately I don't think we should just go ahead assuming that is a problem with the metrics.bat very least we should prove it. 

I'll take a look tomorrow morning to see if I can fix my patch (crbug.com/710095) so we can revert Dan's patch only.

If it's about GC, wouldn't we be getting multi modal results in our graphs after this change?

I think we should revert and wait for delphick@ to return to investigate.

 Issue 713084  has been merged into this issue.

Issue 713622 has been merged into this issue.

Link for all alerts associated to this bug:
https://chromeperf.appspot.com/group_report?bug_id=711044

Primiano and I looked into one of the worse memory regression bugs: ChromiumPerf/android-nexus5/system_health.memory_mobile / memory:chrome:all_processes:reported_by_chrome:cc:effective_size_avg / background_search / background_search_google

This test searches for Tom cruise films, scrolls down and then taps the more button to expand the one-box.

Here the cc memory size grew by 10MB. Watching the test run before and after the change, it appears that they both display the page at the same speed, but the scroll then happens immediately before this change, whereas there is a several second pause after the change to setTimeout.

It appears that this change has pushed out the onLoad event by several seconds which is what triggers the test to scroll. Still not sure why this happens.

 Issue 715772  has been merged into this issue.

 Issue 715835  has been merged into this issue.

I'm rolling this back now that Alexander has fixed Issue 710095.

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

commit c41d3c785c5179a38e54c6c80ea63d56cc7cfd12
Author: delphick <delphick@chromium.org>
Date: Fri Apr 28 11:50:09 2017

Restore one millisecond minimum timeout for setTimeout.

This reverts the non-test part of https://codereview.chromium.org/2738773004/
but leaves most of the test changes in place since they should now work
regardless of the setTimeout behaviour.

BUG= 711044 

Review-Url: https://codereview.chromium.org/2853493002
Cr-Commit-Position: refs/heads/master@{#467962}

[modify] https://crrev.com/c41d3c785c5179a38e54c6c80ea63d56cc7cfd12/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe-expected.txt
[modify] https://crrev.com/c41d3c785c5179a38e54c6c80ea63d56cc7cfd12/third_party/WebKit/Source/core/frame/DOMTimer.cpp
[modify] https://crrev.com/c41d3c785c5179a38e54c6c80ea63d56cc7cfd12/third_party/WebKit/Source/core/frame/DOMTimerTest.cpp

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit b16e680e6bb4f3bb2f4c44b1599a5e3d5603fa8a
Author: Hector Dearman <hjd@google.com>
Date: Fri Apr 28 12:23:57 2017

Restore one millisecond minimum timeout for setTimeout.

This reverts the non-test part of https://codereview.chromium.org/2738773004/
but leaves most of the test changes in place since they should now work
regardless of the setTimeout behaviour.

BUG= 711044 

Review-Url: https://codereview.chromium.org/2853493002
Cr-Commit-Position: refs/heads/master@{#467962}
(cherry picked from commit c41d3c785c5179a38e54c6c80ea63d56cc7cfd12)

Review-Url: https://codereview.chromium.org/2847093002 .
Cr-Commit-Position: refs/branch-heads/3071@{#291}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/b16e680e6bb4f3bb2f4c44b1599a5e3d5603fa8a/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe-expected.txt
[modify] https://crrev.com/b16e680e6bb4f3bb2f4c44b1599a5e3d5603fa8a/third_party/WebKit/Source/core/frame/DOMTimer.cpp
[modify] https://crrev.com/b16e680e6bb4f3bb2f4c44b1599a5e3d5603fa8a/third_party/WebKit/Source/core/frame/DOMTimerTest.cpp