Remove HarfBuzzRun cache

Remove HarfBuzzRun cache

Remove the HarfBuzzRun cache as using the run text value as the cache
isn't good enough when shaping across runs (support for which is coming
in a separate change) or when text rendering settings have changed (CSS
font-feature-settings).
Early testing indicates that the performance implications of removing
the cache are negligible as the overhead of maintaining the cache
negates most of the savings.

R=behdad@chromium.org,leviw@chromium.org
BUG=345401
TEST=PerformanceTests/Layout/hindi-line-layout.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180408

[eae, 2014-02-25 18:03:40]

Mean              Std. Error     
   Patch          10918.40              37.43        
  No Patch        8471.90               83.78       

Turns out the cache does help for at least the http://amarchoti.blogspot.com
test case.

[eae, 2014-03-05 22:42:32]

On 2014/02/25 18:03:40, eae wrote:
> Mean              Std. Error     
>    Patch          10918.40              37.43        
>   No Patch        8471.90               83.78       
> 
> Turns out the cache does help for at least the http://amarchoti.blogspot.com
> test case.

The only reason the cache is effective in for this particular test case is
because of a bug in the itemizer where a different font is used for the
characters and spaces.

[eae, 2014-03-05 22:44:40]

On 2014/03/05 22:42:32, eae wrote:
> On 2014/02/25 18:03:40, eae wrote:
> > Mean              Std. Error     
> >    Patch          10918.40              37.43        
> >   No Patch        8471.90               83.78       
> > 
> > Turns out the cache does help for at least the http://amarchoti.blogspot.com
> > test case.
> 
> The only reason the cache is effective in for this particular test case is
> because of a bug in the itemizer where a different font is used for the
> characters and spaces.

I'd like to get data from the PLT and see if the behavior triggered by the
amarchoti page is common. If not I'd argue we should remove the existing cache
(as it is incorrect) and implement a proper cache as needed.

[eseidel, 2014-05-30 00:25:22]

I presume we're holding off on this change for now?  Is this something Behdad or
Dominik would want to pick up?

[eae, 2014-05-30 00:28:48]

On 2014/05/30 00:25:22, eseidel wrote:
> I presume we're holding off on this change for now?  Is this something Behdad
or
> Dominik would want to pick up?

Was gated on the WidtCache change I landed earlier this week. Plan to pick this
up once I have free cycles after the DirectWrite migration. If Dominik wants to
take it over he is more than welcome to, obviously.

[Dominik Röttsches, 2014-06-02 04:57:14]

On 2014/05/30 00:28:48, eae wrote:
> On 2014/05/30 00:25:22, eseidel wrote:
> > I presume we're holding off on this change for now?  Is this something
Behdad
> or
> > Dominik would want to pick up?
> 
> Was gated on the WidtCache change I landed earlier this week. Plan to pick
this
> up once I have free cycles after the DirectWrite migration. If Dominik wants
to
> take it over he is more than welcome to, obviously.

Interesting! Yes, makes sense that the HarfBuzzRunCache is not as effective
anymore now that WidthCache is used in most of the cases.

In terms of picking up this patch: Pls go ahead, Emil. I'll be looking at:
http://crbug.com/285303 (Font matching by family issue).

[eae, 2014-06-04 13:57:35]

Let's try this again now that the WidthCache change is in.

Ready for review.

[eae, 2014-06-06 18:11:41]

Ping?

[behdad_google, 2014-06-06 18:57:12]

lgtm

[leviw_travelin_and_unemployed, 2014-08-15 22:36:29]

LGTM on patchset 6 to just remove the cache.

Chatted with Emil in person and convinced him to separate out the cache removal
and shaping changes. We can land the shaping change right after to avoid a
possible perf regression, but still keep the changes small.

[eae, 2014-08-15 22:42:54]

Updated this change to just have the cache removal and split out new logic for
shaping across runs into 478823003 (which currently includes this change as
well, will rebase once this lands).

[eae, 2014-08-16 16:05:08]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2014-08-16 16:05:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/eae@chromium.org/167703008/140001

[commit-bot: I haz the power, 2014-08-16 16:08:53]

Committed patchset #6 (140001) as 180408