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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg3N-upgoM


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

chromium-rel-win10

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8983134821347726960

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

Hi drott@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 : Dominik Röttsches
  Commit : 5fd631a31c688954834fc1e297a67b4b1a4f08bc
  Date   : Mon Mar 27 13:31:05 2017
  Subject: Replace subpixel font size heuristics with using OpenType gasp table

Bisect Details
  Configuration: winx64_10_perf_bisect
  Benchmark    : blink_perf.layout
  Metric       : flexbox-lots-of-data/flexbox-lots-of-data

Revision             Result                    N
chromium@459729      21.7926 +- 0.196438       6      good
chromium@459757      22.9093 +- 0.0642857      6      good
chromium@459764      22.6086 +- 0.134273       6      good
chromium@459768      22.5339 +- 0.0953828      6      good
chromium@459770      22.7521 +- 0.727729       6      good
chromium@459771      20.2441 +- 0.167561       6      bad       <--
chromium@459785      20.2684 +- 0.209819       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.layout

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8983134821347726960

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


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

I have trouble imagining how this CL could have affected this microbenchmark, but could it be that we do the gasp table processing for every glyph? In this case, we can perhaps save some cycles? 

pmeenan@, how reliable is the bisect in this case?

We definitely want to keep this change, as it improves font rendering quality on Windows.

The perf test looks like it is very consistent and the bisect looks very clean so I'm pretty confident in it but I'm happy to run again.  That said, given the perf test is doing a layout of a flexbox with a lot of content, a change to measuring font size seems like it could absolutely impact the perf.

A 10% hit to the layout perf, even for just this case is pretty massive.  It's worth looking at to see if there is something that can be done to make it faster (i.e. repeated lookups happening that could be cached, etc).

Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md

We're looking for one of the following:
1. Justification via explanation
2. Plan to revert or fix
3. Angry rage throwing of equipment at my head

Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it.

Note: This was a bulk edit message and not very personal.

This is a correctness and text rendering fidelity improvement on Windows that we'd like to keep. We saw this regression on Android and partially reverted the functionality there. Still, we should figure out why gasp processing affects this microbenchmark so much.

But we don't use FreeType on Windows, no?

I was confusing this issue with the performance impact that enabling HB/FT ligature processing for hinting had, that's why I CC'ed you. Ben is already the owner of this bug. This issue is something that needs investigation on the Skia side, not in HarfBuzz, sorry for the noise.

bungeman, drott: any update on this perf regression?

bungeman: ping on #9

Any updates? The metric has recovered recently. Should we "won't fix" this?

Closing as the change improves rendering quality on Windows and is intended, also the microbenchmark on which this regression triggered does not cover real world scenarios very well.