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

Issue 776384 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

63.4% regression in system_health.memory_mobile at 509708:509759

Project Member Reported by chiniforooshan@chromium.org, Oct 19 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Oct 19 2017

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

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


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

android-webview-nexus5X
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Oct 20 2017

Cc: ksakamoto@chromium.org
Owner: ksakamoto@chromium.org
Status: Assigned (was: Untriaged)

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

Hi ksakamoto@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 : Kunihiko Sakamoto
  Commit : 07ec68095dc07849019a7362ba6cc67625c95c0b
  Date   : Wed Oct 18 10:35:41 2017
  Subject: Simplify CSSFontFaceSource a bit

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:cc:effective_size_avg/load_news/load_news_cnn
  Change       : 63.42% | 16469668.0 -> 26914468.0

Revision             Result                    N
chromium@509707      16469668 +- 0.0           6      good
chromium@509720      16469668 +- 0.0           6      good
chromium@509727      16469668 +- 0.0           6      good
chromium@509729      16469668 +- 0.0           6      good
chromium@509730      29003428 +- 0.0           6      bad       <--
chromium@509733      26914468 +- 11441705      6      bad
chromium@509759      26914468 +- 11441705      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=load.news.cnn system_health.memory_mobile

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

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


For feedback, file a bug with component Speed>Bisection
Status: Started (was: Assigned)
My patch might have caused over-triggering of font invalidation. I'll make a fix.
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Oct 20 2017

Cc: pmeenan@chromium.org
 Issue 776761  has been merged into this issue.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Oct 21 2017

 Issue 776926  has been merged into this issue.
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Oct 21 2017

Issue 776791 has been merged into this issue.
Cc: toyoshim@chromium.org kinuko@chromium.org
 Issue 776763  has been merged into this issue.
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Oct 23 2017

Cc: petermarshall@chromium.org
 Issue 777325  has been merged into this issue.
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Oct 23 2017

 Issue 777403  has been merged into this issue.
Cc: tebbi@chromium.org
 Issue 777332  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 24 2017

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

commit cdca7f33c3b9d7796a105a94b0cb5ad63d43af19
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Tue Oct 24 03:08:15 2017

Prevent over-triggering of font invalidation

FontSelector::FontFaceInvalidated() is expensive since it causes full
style recalculation. This patch lets RemoteFontFaceSource not to call
it when unnecessary (e.g. in the middle of FontFace construction).

This will hopefully fix memory regression by https://crrev.com/c/722141.

Bug:  776384 
Change-Id: I86a9b27be2fd65f51cc6057a7c897f588c4897ce
Reviewed-on: https://chromium-review.googlesource.com/730032
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511024}
[modify] https://crrev.com/cdca7f33c3b9d7796a105a94b0cb5ad63d43af19/third_party/WebKit/Source/core/css/CSSFontFace.cpp
[modify] https://crrev.com/cdca7f33c3b9d7796a105a94b0cb5ad63d43af19/third_party/WebKit/Source/core/css/CSSFontFace.h
[modify] https://crrev.com/cdca7f33c3b9d7796a105a94b0cb5ad63d43af19/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp

Status: Fixed (was: Started)
OK, regressions recovered.

Sign in to add a comment