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

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


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

android-webview-nexus6

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

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

Hi palmer@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 : palmer
  Commit : 40cd3c13c54f9aa73fc0b47e59a7d962037e2f94
  Date   : Wed Feb 15 06:37:05 2017
  Subject: Protect heap metadata in Oilpan.

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg/background/after_https_www_google_co_uk_hl_en_q_science
  Change       : 0.75% | 26846508.5714 -> 27048382.8571

Revision             Result                  N
chromium@450590      26846509 +- 429327      14      good
chromium@450597      26803891 +- 294708      6       good
chromium@450598      27291315 +- 442114      6       bad       <--
chromium@450599      27239773 +- 623798      6       bad
chromium@450600      27130205 +- 428866      6       bad
chromium@450603      27185501 +- 280668      6       bad
chromium@450616      27092269 +- 878336      14      bad
chromium@450642      27048383 +- 898974      14      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-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile

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

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


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

This patch is a fix for a security regression (heap metadata being too soft of a target in heap buffer overflow scenarios). jschuh and I believe the 1.2% performance loss is a reasonable trade-off here.

+jschuh, haraken FYI; thoughts?

I guess this would be something we should discuss on platform-architecture-dev@ including security folks.

A 300 KB increase on memory.top_memory_mobile (this is a very real-world benchmark) is a clear regression. Also there is a 1 MB increase on system_health.memory_mobile, which looks unacceptable.

You should definitely investigate reducing the memory footprint, and palmer@ already has some thoughts on how to address the binary size increase. However, I also need to be clear that this cannot be a debate over whether the underlying security issue gets fixed. The original security regression should never have landed, and thus simply does not provide a meaningful baseline for judging performance.

Put another way, we cannot improve performance by regressing security, and then claim that as a necessary bar for fixing the security regression.

I understand your point from the security perspective but a 1 MB increase on a real-world benchmark on low-memory Android is not something we can easily accept.

I'm not planning to say no to landing that CL but we should seriously think about a way to mitigate the memory regression.

e.g., we haven't tried to outline the meta data to a different system page (which won't increase sizeof(object)).

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

commit e3d585cce44455344c067ab7a5ebb359fae854f7
Author: palmer <palmer@chromium.org>
Date: Wed Mar 01 02:55:21 2017

Use the Oilpan heap canary only on 64-bit.

On 64-bit machines, the canary is free, space-wise. But on 32-bit, it doubles
the size of |HeapObjectHeader|, which has incurred a large run-time memory size
increase.

Turn off the canary for now, on 32-bit. Ultimately we may solve the original bug
(633030) by moving the metadata out of line instead of guarding it with a
canary.

BUG= 692931 ,692624, 633030 

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

[modify] https://crrev.com/e3d585cce44455344c067ab7a5ebb359fae854f7/third_party/WebKit/Source/platform/heap/HeapPage.cpp
[modify] https://crrev.com/e3d585cce44455344c067ab7a5ebb359fae854f7/third_party/WebKit/Source/platform/heap/HeapPage.h

I think #8 solves this.