Do not cache the result of RenderStyle::computedLineHeight()

Do not cache the result of RenderStyle::computedLineHeight()

We currently use 26-bits in RenderBlock to cache the result of
RenderStyle::computedLineHeight(). However when you look at the
computations behind that function the main components are already cached
so at most we are saving a handful of computations per call.

In https://codereview.chromium.org/256593009/ we added a test that calls
RenderBlock::lineHeight() ~425,000 times on each run. This CL results in
no measurable degradation in performance against that test.

So since we cannot get a measurable improvement in performance as a result
of the caching when testing locally, remove it and free up space for more useful
bits in RenderBlock. We will let it cycle through the perf bots for a while
before allowing people to start using the spare bits though.

The cached result originally sat in RenderText and was inherited from the
fork from KHTML. http://trac.webkit.org/changeset/6010 moved it to RenderBlock
on the basis that "calls to fontMetrics().lineSpacing() are expensive." This
is no longer the case as far as I can tell, since both lineSpacing() and
fontMetrics() are now cached themselves.

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

[leviw_travelin_and_unemployed, 2014-04-28 20:42:49]

LGTM, but please wait for perf bot baselines from
https://codereview.chromium.org/256593009/ before landing (so give it a bit)

[rhogan, 2014-04-29 18:34:17]

On 2014/04/28 20:42:49, leviw wrote:
> LGTM, but please wait for perf bot baselines from
> https://codereview.chromium.org/256593009/ before landing (so give it a bit)

After re-testing some more today I'm seeing 0.75% to 1% regression in
performance in the line-layout tests with this change. So I'm a bit cooler on
proceeding with it since it now seems more of a trade-off than I'd originally
thought.

It will be hard to roll back on this CL once people start using the free bits so
are we willing to take the performance hit here on the promise of gaining more
elsewhere?

[leviw_travelin_and_unemployed, 2014-04-29 20:15:28]

On 2014/04/29 18:34:17, rhogan wrote:
> On 2014/04/28 20:42:49, leviw wrote:
> > LGTM, but please wait for perf bot baselines from
> > https://codereview.chromium.org/256593009/ before landing (so give it a bit)
> 
> After re-testing some more today I'm seeing 0.75% to 1% regression in
> performance in the line-layout tests with this change. So I'm a bit cooler on
> proceeding with it since it now seems more of a trade-off than I'd originally
> thought.
> 
> It will be hard to roll back on this CL once people start using the free bits
so
> are we willing to take the performance hit here on the promise of gaining more
> elsewhere?

I guess I think a better idea would be to socialize (perhaps just with a
comment) that we could remove this if there's a need for the bits that would get
us worthwhile performance. I suspect there is, but I'd rather not regress us
until that happens.

[rhogan, 2014-06-16 19:13:58]

The CQ bit was checked by robhogan@gmail.com

[commit-bot: I haz the power, 2014-06-16 19:15:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/robhogan@gmail.com/260073005/30001

[commit-bot: I haz the power, 2014-06-16 19:18:35]

Change committed as 176233