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

Issue 696070 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1%-35.9% regression in memory.top_10_mobile at 452214:452358

Project Member Reported by m...@chromium.org, Feb 24 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Feb 25 2017

Cc: fmalita@chromium.org
Owner: fmalita@chromium.org

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

Hi fmalita@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 : fmalita
  Commit : 6f057b93cf230adadd55af0490c836d91ba6e1fd
  Date   : Thu Feb 23 00:55:00 2017
  Subject: Remove the text blob cache

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg/background/after_https_mobile_twitter_com_justinbieber_skip_interstitial_true
  Change       : 35.95% | 769360.0 -> 1045924.0

Revision             Result                N
chromium@452276      769360 +- 0.0         6      good
chromium@452301      811024 +- 228203      6      good
chromium@452313      811024 +- 228203      6      good
chromium@452316      807415 +- 208438      6      good
chromium@452317      769360 +- 0.0         6      good
chromium@452318      1045924 +- 0.0        6      bad       <--
chromium@452319      1045924 +- 0.0        6      bad
chromium@452325      1045924 +- 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=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/8986734194866412416

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


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

Comment 4 by m...@chromium.org, Feb 25 2017

Status: Assigned (was: Untriaged)
Labels: Performance-Memory

Comment 6 by m...@chromium.org, Jun 5 2017

Cc: -m...@chromium.org
fmalita: You removed the textblob cache and performance got worse. Please either fix the issue, or justify why there is nothing to fix and WontFix this bug. Thanks. :)
Cc: -fmalita@chromium.org bsalomon@chromium.org chrishtr@chromium.org pdr@chromium.org aelias@chromium.org reed@google.com
It is quite strange to me that only webview shows a clear mem regression (the chrome-android graphs were much noisier, and have either recovered of been swallowed by larger regressions).

For reference: https://chromeperf.appspot.com/report?sid=95614f16e41582e4f08ca92a6f3b5136c5b4f735a65552c118d74e8aa3875606&start_rev=449029&end_rev=477265

One of the theories was without a TB cache we might be rebuilding blobs more aggressively in some cases, increasing pressure on Ganesh's blob cache (the related Ganesh cache was not being purged on blob destruction).  So we landed the missing purge logic (https://skia-review.googlesource.com/c/9631/), and were hoping to see some progression.

While there's been a small (unrelated?) improvement, the webview regression has not recovered.

I've run a local/desktop/Linux sanity check with and instrumented build, and verified that nothing strange is going on with that Bieber twitter page during the test: we're allocating ~230 blobs, the number is stable and matches pretty closely the number of drawTextBlob calls in the SKP dump.

So I'm kind of lost.  There's obviously something quite different about webview which makes slimming paint caching less effective (more invalidations?).  Maybe it's in how the test is run, or some disabled SP features, or some text auto-sizing interaction.  But the regression is significant enough to make me nervous about hand-waving my way to a WontFix, so I'd like to spend more time investigating.

Adding more folks for ideas.
One difference is that some WebView apps use a very large painting surface,
in cases when they want to use the (deprecated) webview-only, synchronous
pixel grabbing API.

ericrk@ reports that WebView should be using Ganesh, so the above is my best theory.
fmalita: any update on this bug?
Status: WontFix (was: Assigned)
All the memory.top_10_mobile/memory:chrome:all_processes:reported_by_os:system_memory:java_heap:proportional_resident_size_avg graphs seem to have recovered around 453200 - 453400 (but then later got hit with other, more significant regressions).

All system_health.memory_mobile/memory:chrome:all_processes:reported_by_chrome:malloc:effective_size_avg graphs also recovered, and then some.

top_10_mobile_stress/memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg is also recovered.

That leaves a couple of top_10_mobile/memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg graphs for https_mobile_twitter_com_justinbieber_skip_interstitial_true, which show a progression around 470457-470497 -- but are still ~18% in the red.

I still don't have a clear explanation of why this metric has increased; we've removed an explicit cache (which likely reduced a different metric, not reflected here), and are now relying on the implicit caching offered by slimming paint paint ops and partial invalidation.  One more theory: during invalidation, as the new paint ops are being recorded, the old ops are still alive and only get replaced when the recording is completed/committed.  So there's a temporary lifetime overlap between the old blobs and the new blobs, which may sway these numbers for WebView.

Considering that a) most regressions have recovered, b) the remaining ones are not critical and limited to one page, and c) if my last theory is correct, the memory increase is transient (during inval only), I think I'm ready to close this -- unless others disagree.

Sign in to add a comment