New issue
Advanced search Search tips

Issue 634477 link

Starred by 2 users
Status: WontFix
Owner:
drott@chromium.org
Closed: Sep 2016
Cc:
drott@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.6%-25.6% regression in blink_perf.layout at 409518:409603

Project Member Reported by sullivan@chromium.org, Aug 4 2016 
See the link to graphs below.

Comment 2 by sullivan@chromium.org, Aug 5 2016

Labels: Performance-Sheriff

Comment 3 by drott@chromium.org, Aug 9 2016 

This is most likely a side effect of significantly reducing memory consumption by removing the glyphToBoundsMap font metric caching: https://bugs.chromium.org/p/chromium/issues/detail?id=589462#c12

Especially for Android it was important to reduce memory when activating the complex text code path. This is why I believe this tradeoff is okay. 

Why the glyph metrics functions in Windows seem to have a higher overhead then on other platforms is being investigated in: https://bugs.chromium.org/p/chromium/issues/detail?id=634312#c3
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Aug 17 2016

Cc: drott@chromium.org
Owner: drott@chromium.org

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

Hi drott@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Remove glyph metrics caching on non-Mac platforms
Author  : drott
Commit description:
  
As shown in the benchmarking results discussed in  crbug.com/631032 
removing the glyph metrics caching in Blink on non Mac platforms and
only using the lower level Skia caches instead should not affect overall
layout and text performance negatively. In fact, some performance tests
such as html5-full-render and html-parser-threaded are improving in
local measurements on linux.

This should give us improvements in memory consumption ranging from
tens of kilobytes in a single-origin use renderer up to over a megabyte
on longer lifespan renderers such as in the Android WebView and help
with crbug.com/577306.

On Mac platforms we are still using path based glyph metrics, which seem
to be too slow. Here, the glyph metrics caching in Blink cannot yet be
removed, compare
https://bugs.chromium.org/p/chromium/issues/detail?id=608338#c9

BUG= 589462 , 631032 
R=kojii,tzik,eae

Review-Url: https://codereview.chromium.org/2199333008
Cr-Commit-Position: refs/heads/master@{#409564}
Commit  : 3af81d9791986c5d47b261dda6f2a672e65ae313
Date    : Wed Aug 03 17:56:10 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@409518  60.0118  0.971679  5  good
chromium@409544  60.1152  0.327255  5  good
chromium@409558  59.5102  0.171672  5  good
chromium@409561  60.4038  0.739352  5  good
chromium@409563  61.2438  0.421358  5  good
chromium@409564  75.8838  0.613477  5  bad    <--
chromium@409569  75.1856  0.894428  5  bad

Bisect job ran on: winx64ati_perf_bisect
Bug ID: 634477

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout
Test Metric: latin-complex-text/latin-complex-text
Relative Change: 25.28%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64ati_perf_bisect/builds/1501
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004136134795165584


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6461937374724096

| 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 Tests>AutoBisect.  Thank you!
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Aug 17 2016 


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Remove glyph metrics caching on non-Mac platforms
Author  : drott
Commit description:
  
As shown in the benchmarking results discussed in  crbug.com/631032 
removing the glyph metrics caching in Blink on non Mac platforms and
only using the lower level Skia caches instead should not affect overall
layout and text performance negatively. In fact, some performance tests
such as html5-full-render and html-parser-threaded are improving in
local measurements on linux.

This should give us improvements in memory consumption ranging from
tens of kilobytes in a single-origin use renderer up to over a megabyte
on longer lifespan renderers such as in the Android WebView and help
with crbug.com/577306.

On Mac platforms we are still using path based glyph metrics, which seem
to be too slow. Here, the glyph metrics caching in Blink cannot yet be
removed, compare
https://bugs.chromium.org/p/chromium/issues/detail?id=608338#c9

BUG= 589462 , 631032 
R=kojii,tzik,eae

Review-Url: https://codereview.chromium.org/2199333008
Cr-Commit-Position: refs/heads/master@{#409564}
Commit  : 3af81d9791986c5d47b261dda6f2a672e65ae313
Date    : Wed Aug 03 17:56:10 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@409558  60.261   0.459597  5  good
chromium@409561  60.2146  0.584497  5  good
chromium@409563  60.3062  0.438837  5  good
chromium@409564  75.1326  0.597747  5  bad    <--
chromium@409569  75.9372  0.639902  5  bad
chromium@409581  76.6256  0.583866  5  bad
chromium@409602  74.3146  0.445621  5  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 634477

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout
Test Metric: latin-complex-text/latin-complex-text
Relative Change: 23.32%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1809
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004136129289885872


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5811135475875840

| 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 Tests>AutoBisect.  Thank you!
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Aug 17 2016 


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Remove glyph metrics caching on non-Mac platforms
Author  : drott
Commit description:
  
As shown in the benchmarking results discussed in  crbug.com/631032 
removing the glyph metrics caching in Blink on non Mac platforms and
only using the lower level Skia caches instead should not affect overall
layout and text performance negatively. In fact, some performance tests
such as html5-full-render and html-parser-threaded are improving in
local measurements on linux.

This should give us improvements in memory consumption ranging from
tens of kilobytes in a single-origin use renderer up to over a megabyte
on longer lifespan renderers such as in the Android WebView and help
with crbug.com/577306.

On Mac platforms we are still using path based glyph metrics, which seem
to be too slow. Here, the glyph metrics caching in Blink cannot yet be
removed, compare
https://bugs.chromium.org/p/chromium/issues/detail?id=608338#c9

BUG= 589462 , 631032 
R=kojii,tzik,eae

Review-Url: https://codereview.chromium.org/2199333008
Cr-Commit-Position: refs/heads/master@{#409564}
Commit  : 3af81d9791986c5d47b261dda6f2a672e65ae313
Date    : Wed Aug 03 17:56:10 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@409562  58.741   0.375103  5  good
chromium@409563  58.7414  0.460943  5  good
chromium@409564  74.5952  1.07643   5  bad    <--
chromium@409565  74.204   0.596609  5  bad
chromium@409568  74.5904  0.747307  5  bad
chromium@409572  75.23    0.863924  5  bad
chromium@409581  76.1454  0.92314   5  bad
chromium@409603  73.7426  0.492676  5  bad

Bisect job ran on: winx64intel_perf_bisect
Bug ID: 634477

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout
Test Metric: latin-complex-text/latin-complex-text
Relative Change: 25.54%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64intel_perf_bisect/builds/1107
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004136169264694064


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5843838430609408

| 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 Tests>AutoBisect.  Thank you!

Comment 10 by rsch...@chromium.org, Sep 23 2016

Status: WontFix (was: Assigned)
Closing based on justification in comment #3.
►

Sign in to add a comment