New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 862969 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jul 15
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

3.6% improvement in memory.top_10_mobile at 1531272300:1531282487

Project Member Reported by perezju@google.com, Jul 12

Issue description

Want to figure out the source of this improvement.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=862969

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


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

perf-go-webview-phone
Cc: khushals...@chromium.org
Owner: khushals...@chromium.org
Status: Assigned (was: Untriaged)

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

Hi khushalsagar@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 : Khushal
  Commit : 7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4
  Date   : Tue Jul 10 20:01:45 2018
  Subject: gpu: Drop GrContext cache after an idle period.

Bisect Details
  Configuration: go-webview-phone-perf-bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_os:system_memory:private_footprint_size_avg/background/after_http_yandex_ru_touchsearch_text_science
  Change       : 3.03% | 29634560.0 -> 28710229.3333

Revision                                       Result                  N
android-chrome@801e0585fe                      29634560 +- 244965      9      good
android-chrome@05848147e8                      29548544 +- 195120      9      good
android-chrome@05848147e8,chromium@573862      29373099 +- 365775      6      good
android-chrome@05848147e8,chromium@573868      29418837 +- 248498      6      good
android-chrome@05848147e8,chromium@573870      29479595 +- 255564      6      good
android-chrome@05848147e8,chromium@573871      28532053 +- 196905      6      bad       <--
android-chrome@05848147e8,chromium@573873      28620800 +- 333564      6      bad
android-chrome@6a69fc540e                      28710229 +- 427036      9      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 --story-filter=http.yandex.ru.touchsearch.text.science memory.top_10_mobile

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

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


For feedback, file a bug with component Speed>Bisection
Cc: ericrk@chromium.org enne@chromium.org
This is replicating the same optimization that's in the GPU raster path, you can see the regression from it in 571690 - 571710. I'm a bit surprised that it recovered and improved some more.

ericrk@ and I were just discussing that the GPU raster path was moved to relax the amount of purging it did in the idle case. But the timeline is:
1) Turn on OOPR on the android bots.
2) Patch for doing less purging in the GPU case lands (https://chromium-review.googlesource.com/1123169), has no effect on the bots because they run with OOPR.
3) The patch for equivalent optimization lands as pointed in #3. And that actually comes with an improvement...?
Looking at the traces before the regression before r571694 and the trace after the change in #3, the private footprint went from 28.1 -> 27.4. The only component where I can see a significant change is gpu. There's a single allocation of 1M extra earlier vs now.
Oh forgot to mention, that extra allocation is a single buffer under transfer_memory. But then comparing the trace right before the change in #3 landed and the trace with the change, both don't have this, so maybe that doesn't indicate anything.


Ran a bisect for something else that improved gpu memory by a similar amount to the real improvement here. Lets see what that comes up with.
Cc: sunxd@chromium.org
Owner: sunxd@chromium.org

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

Hi sunxd@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 : sunxd
  Commit : 3e3a37a78aa5eb7ff2da5adae3fa44efccf5773c
  Date   : Fri Jul 06 22:58:43 2018
  Subject: Add wheel event handler region in cc

Bisect Details
  Configuration: go-webview-phone-perf-bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg/background/after_http_yandex_ru_touchsearch_text_science
  Change       : 3.65% | 14374229.3333 -> 13849258.6667

Revision                                       Result                   N
android-chrome@b5f70f02d5                      14374229 +- 16315.6      6      good
android-chrome@b5f70f02d5,chromium@573079      14353749 +- 25337.9      6      good
android-chrome@b5f70f02d5,chromium@573096      14373547 +- 33916.9      6      good
android-chrome@b5f70f02d5,chromium@573104      14387883 +- 44993.9      6      good
android-chrome@b5f70f02d5,chromium@573105      14375595 +- 14109.9      6      good
android-chrome@b5f70f02d5,chromium@573106      13848576 +- 30869.8      6      bad       <--
android-chrome@b5f70f02d5,chromium@573109      13832192 +- 34562.1      6      bad
android-chrome@b5f70f02d5,chromium@573112      13830144 +- 18087.4      6      bad
android-chrome@b5f70f02d5,chromium@573178      13842432 +- 28229.7      6      bad
android-chrome@9245133ef9                      13849259 +- 35747.2      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-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http.yandex.ru.touchsearch.text.science memory.top_10_mobile

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

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


For feedback, file a bug with component Speed>Bisection
Cc: -sunxd@chromium.org
Owner: khushals...@chromium.org
Status: WontFix (was: Assigned)
Hmmm, its interesting that the change above would impact GPU memory. This seems like a timing thing.

Overall, the improvement on this bug is expected and intended from the change in #3. Its recovering from the initial regression of turning on OOP raster by replicating an optimization from GPU raster, and its great that this works better with OOP raster.
Thanks for the detailed analysis folks!

Sign in to add a comment