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

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


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

chromium-rel-mac-retina

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


=== Bisection aborted ===
The bisect was aborted because The metric values for the initial "good" and "bad" revisions do not represent a clear regression.
Please contact the the team (see below) if you believe this is in error.

=== Warnings ===
The following warnings were raised by the bisect job:

 * Bisect failed to reproduce the regression with enough confidence.

===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385660         8648.082535 787.971414  18          good
chromium@385669         8537.805688 566.013554  18          bad

Bisect job ran on: mac_retina_perf_bisect
Bug ID: 601835

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --also-run-disabled-tests blink_perf.canvas
Test Metric: drawimage-not-pixelaligned/drawimage-not-pixelaligned
Relative Change: 5.40%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1245
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9015929454317243824


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

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

=== 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 : Reland: Move glyph lookup to hb-ot-font and remove glyph cache in HarfBuzzFace
Author  : drott
Commit description:
  
Relanding after previously reverted because it caused linkage issues
for chromeos=1 developer builds in Linux. These should be addressed
now through dpranke@'s fixes in  issue 589342 .

Together with Behdad we identified several ways to optimize and cleanup
HarfBuzzFace:
* We moved the glyph lookup completely to the built-in HarfBuzz
  implementation in hb-ot-font.
* This allows us to remove the unbounded glyph cache we kept around in
  HarfBuzzFace.
* We also moved instantantion and ownership of HarfBuzzFontData to
  HarfBuzzFace. We only update the size and unicode-range on each call
  to getScaledFont(), saving us as the allocation and setup costs of
  this object for each shaping call.
* We removed the unneeded callback for retrieving the horizontal origin,
  which was implemented as a noop.

In local measurements on Mac this leads to large improvements in the
blink_perf.layout tests, especially in
chapter-reflow-once-random (>130%), flexbox-lots-of-data (>58%) and
ArabicLineLayout (>26%) with only a few of the microbenchmarks being
slightly slower: So this is an improvement for layout cases that do not
profit much from the word cache.

TBR'ing since this has previously been LGTMed in
https://codereview.chromium.org/1733193002

BUG= 589340 
TBR=eae,behdad,kojii

Review URL: https://codereview.chromium.org/1866863002

Cr-Commit-Position: refs/heads/master@{#385631}
Commit  : b2a8582e83ef3022ac1cc65648348dfd168b77cd
Date    : Thu Apr 07 02:41:08 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385598         10799.93428629.992807   8           good
chromium@385616         10810.72120228.359728   5           good
chromium@385623         10767.32928321.124635   5           good
chromium@385628         10717.247423178.118126  8           good
chromium@385630         10780.02147754.898404   8           good
chromium@385631         8855.172427 1062.349125 5           bad         <-
chromium@385633         8387.593345 53.172541   5           bad
chromium@385669         9009.038372 1082.287985 12          bad

Bisect job ran on: mac_retina_perf_bisect
Bug ID: 601835

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --also-run-disabled-tests blink_perf.canvas
Test Metric: drawimage-not-pixelaligned/drawimage-not-pixelaligned
Relative Change: 17.52%
Score: 98.0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1246
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9015929443200775568


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

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

 Issue 601797  has been merged into this issue.

Issue 601804 has been merged into this issue.

Regression alerts aren't getting combined properly on merge, please see those bugs for more regressions.

+eae since it's drott's night. Emil, do you think we should revert this? It's coming back with tons and tons of regressions, see the merged bugs for more.

Increasing memory consumption and regressing performance? Yikes! It was supposed to do the opposite.

Yeah, we should probably revert it.

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

commit abed5a107d06843c622bf704088618572ec4f83f
Author: eae <eae@chromium.org>
Date: Mon Apr 11 21:13:08 2016

Revert "Reland: Move glyph lookup to hb-ot-font and remove glyph cache in HarfBuzzFace"

Revert r385631 (commit b2a8582e83ef3022ac1cc65648348dfd168b77cd) causing
an increase in memory consumption and regressing performance on multiple
platforms, most notably on Android. Reverting to give us time to triage.

TBR=drott@chromium.org
BUG= 601835 

Review URL: https://codereview.chromium.org/1880623002

Cr-Commit-Position: refs/heads/master@{#386473}

[modify] https://crrev.com/abed5a107d06843c622bf704088618572ec4f83f/third_party/WebKit/Source/platform/fonts/FontPlatformData.cpp
[modify] https://crrev.com/abed5a107d06843c622bf704088618572ec4f83f/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
[modify] https://crrev.com/abed5a107d06843c622bf704088618572ec4f83f/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.h
[modify] https://crrev.com/abed5a107d06843c622bf704088618572ec4f83f/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp

This revert needs to be put in the M51 branch 2704.

> Revert r385631 (commit b2a8582e83ef3022ac1cc65648348dfd168b77cd) causing
> an increase in memory consumption and regressing performance on multiple
> platforms, most notably on Android. Reverting to give us time to triage.

Ok, interesting.  There are two things that need to be fixed:

  - hb-ot-font currently loads the 'glyf' table; I'll make that lazy so it wouldn't,

  - More importantly, this shows that the reference_table_func() passed to hb_face_create_for_tables() is an inefficient implementation that copies the table memory.  Indeed, when we ported Android to use hb-ot-font, we faced the same issue, because the Skia getTable() API is implemented that way.  Android was fixed by accessing raw font data and passing it directly to hb_face_create().  This change, in fact, resulted in reduced memory consumption since now even GSUB/GPOS tables won't be copied anymore.  A similar change seems to be needed in Blink before we can move to hb-ot-font.  I'll work with Dominik on that.

As you say, the Skia function is copy only: So you mean we should put copies of the needed tables (which? or all?) into HarfBuzzFontData and return those when the functions are called? Or lazily copy a table from Skia into HarfBuzzFontdata when the function is called with a new TAG, otherwise return the cached version?

Still, without the HarfBuzzFace cache we have a different SkTypeface for every size instance of a font, so we would use more memory now, despite the tables being the same for all sizes, IIUC. :-(

Your change meets the bar and is auto-approved for M51 (branch: 2704)

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

commit a8c05871f3a521846d96c966eea9886f129d5606
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Apr 12 20:44:06 2016

Revert "Reland: Move glyph lookup to hb-ot-font and remove glyph cache in HarfBuzzFace"

Revert r385631 (commit b2a8582e83ef3022ac1cc65648348dfd168b77cd) causing
an increase in memory consumption and regressing performance on multiple
platforms, most notably on Android. Reverting to give us time to triage.

TBR=drott@chromium.org
BUG= 601835 

Review URL: https://codereview.chromium.org/1880623002

Cr-Commit-Position: refs/heads/master@{#386473}
(cherry picked from commit abed5a107d06843c622bf704088618572ec4f83f)

Review URL: https://codereview.chromium.org/1882233002 .

Cr-Commit-Position: refs/branch-heads/2704@{#14}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/a8c05871f3a521846d96c966eea9886f129d5606/third_party/WebKit/Source/platform/fonts/FontPlatformData.cpp
[modify] https://crrev.com/a8c05871f3a521846d96c966eea9886f129d5606/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
[modify] https://crrev.com/a8c05871f3a521846d96c966eea9886f129d5606/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.h
[modify] https://crrev.com/a8c05871f3a521846d96c966eea9886f129d5606/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp

I'm perf sheriffing today and, FWIW, the revert CL (in c#18) actually undid a significant performance improvement:

ChromiumPerf/chromium-rel-win7-dual/blink_perf.layout / character_fallback

https://chromeperf.appspot.com/report?sid=d5e651d5d37107fd55544f74dde379977c65f1d700620a60546c236a91c7a7e0&rev=386477

> I'm perf sheriffing today and, FWIW, the revert CL (in c#18) actually undid a
> significant performance improvement:

Right.  Has to wait till we resolve the increased-memory issue and then land again.

*sniff* - thanks for the info, miu@.

> Still, without the HarfBuzzFace cache we have a different SkTypeface for every 

We definitely need a HarfBuzzFace cache.  Let's talk about this in Zurich.

In short, if you can get the binary font blob out of Skia, that would be idea.