Switch arguments to const Length references for performance

Switch arguments to const Length references for performance

The Length destructor is nontrivial and every non-const Length
argument adds an unnecessary branch from calling Length's destructor.
This patch removes most of the non-const Length arguments which should
lead to a small performance improvement.

A few LengthSize, LengthPoint, and LengthBox arguments were also
constref'ified for consistency.

No new tests as there is no observable change in behavior.

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

[eseidel, 2014-05-29 06:53:06]

lgtm

OK.  Should we be making ~Length() cheaper instead?

[Mike Lawther (Google), 2014-05-29 08:13:36]

On 2014/05/29 06:53:06, eseidel wrote:
> lgtm
> 
> OK.  Should we be making ~Length() cheaper instead?

The non-triviality in ~Length is tracking refs for calc values. 

Now that Alan has optimised the calc expression trees, we just had a brief chat
with timloh@ about cramming the calc values into Length to remove the ref
counting complexity. It would be easy if we can grow Length by 32bits (from
64->96 bits). It's possible without making Length larger, but would need masking
the LSB of a float for a flag.

[pdr., 2014-05-29 14:07:40]

On 2014/05/29 08:13:36, Mike Lawther (Google) wrote:
> On 2014/05/29 06:53:06, eseidel wrote:
> > lgtm
> > 
> > OK.  Should we be making ~Length() cheaper instead?
> 
> The non-triviality in ~Length is tracking refs for calc values. 
> 
> Now that Alan has optimised the calc expression trees, we just had a brief
chat
> with timloh@ about cramming the calc values into Length to remove the ref
> counting complexity. It would be easy if we can grow Length by 32bits (from
> 64->96 bits). It's possible without making Length larger, but would need
masking
> the LSB of a float for a flag.

Is this possible to do in a way that works with oilpan?

[pdr., 2014-05-29 14:08:44]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-05-29 14:09:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pdr@chromium.org/305883002/1

[Mike Lawther (Google), 2014-05-29 23:33:20]

On 2014/05/29 14:07:40, pdr wrote:
> On 2014/05/29 08:13:36, Mike Lawther (Google) wrote:
> > On 2014/05/29 06:53:06, eseidel wrote:
> > > lgtm
> > > 
> > > OK.  Should we be making ~Length() cheaper instead?
> > 
> > The non-triviality in ~Length is tracking refs for calc values. 
> > 
> > Now that Alan has optimised the calc expression trees, we just had a brief
> chat
> > with timloh@ about cramming the calc values into Length to remove the ref
> > counting complexity. It would be easy if we can grow Length by 32bits (from
> > 64->96 bits). It's possible without making Length larger, but would need
> masking
> > the LSB of a float for a flag.
> 
> Is this possible to do in a way that works with oilpan?

Ideally Oilpan wouldn't get a look in :) What we'd want to do is to cram the
calc data into the bits of the Length data structure. As Length itself isn't
refcounted, this should Just Work :)

[ojan, 2014-05-29 23:46:39]

There are ~70 Lengths on a RenderStyle and there are ~1:1 RenderStyles for each
rendered DOM Node. So, on a page with 300000 DOM nodes, 4 bytes * 70 * 300000 =
80MB. That's way too much IMO for the benefit we get by changing this.

What is the proposal that uses the LSB of the float? We chatted about ideas like
that and couldn't come up with anything we were happy with.

[eseidel, 2014-05-30 00:01:32]

For context, how many DOM nodes does the HTML5 spec have?  300,000 sounds like a
lot to me.

[ojan, 2014-05-30 00:06:38]

On 2014/05/30 00:01:32, eseidel wrote:
> For context, how many DOM nodes does the HTML5 spec have?  300,000 sounds like
a
> lot to me.

I think the HTML5 spec has more than that. It certainly is a lot and is not the
content we're primarily optimizing for.

[pdr., 2014-05-30 00:10:43]

On 2014/05/30 00:01:32, eseidel wrote:
> For context, how many DOM nodes does the HTML5 spec have?  300,000 sounds like
a
> lot to me.

Will document.querySelectorAll('*') get them all? That returns 124,647 nodes on
the single page html5 spec.

[eseidel, 2014-05-30 00:50:55]

Donno if that will get text nodes... 
querySelectorAll('*') in gmail says 6800 nodes (on desktop).

[pdr., 2014-05-30 01:35:42]

On 2014/05/30 00:50:55, eseidel wrote:
> Donno if that will get text nodes... 
> querySelectorAll('*') in gmail says 6800 nodes (on desktop).

Counted these by hand and there are 128,516 elements, 187,995 text nodes, and a
total of 319,735 nodes.

[esprehn, 2014-05-30 01:40:13]

On 2014/05/30 01:35:42, pdr wrote:
> On 2014/05/30 00:50:55, eseidel wrote:
> > Donno if that will get text nodes... 
> > querySelectorAll('*') in gmail says 6800 nodes (on desktop).
> 
> Counted these by hand and there are 128,516 elements, 187,995 text nodes, and
a
> total of 319,735 nodes.

Yeah it seems I was thinking of Nodes not Elements. There's at most ~15% style
sharing so lets say there's about 110000 unique RenderStyle's. That means:

110000*70*8bytes = 59MB.

I'm not sure if we think that's acceptable or not. How much do we really care
about the HTML spec?

For Gmail or Facebook it's much closer to 2MB. That's not trivial, but it might
be worth it for the code simplicity.

[eseidel, 2014-05-30 01:47:24]

On 2014/05/30 01:35:42, pdr wrote:
> On 2014/05/30 00:50:55, eseidel wrote:
> > Donno if that will get text nodes... 
> > querySelectorAll('*') in gmail says 6800 nodes (on desktop).
> 
> Counted these by hand and there are 128,516 elements, 187,995 text nodes, and
a
> total of 319,735 nodes.

I'm confused by this discrepancy.  Does gmail just hold a ton of content in
iframes?

[eseidel, 2014-05-30 01:48:10]

Maybe the gmail count includes lots of parsed responseXML content?  (Which there
is some question as to if we should be keeping that around at all.)

[pdr., 2014-05-30 01:54:23]

On 2014/05/30 01:47:24, eseidel wrote:
> On 2014/05/30 01:35:42, pdr wrote:
> > On 2014/05/30 00:50:55, eseidel wrote:
> > > Donno if that will get text nodes... 
> > > querySelectorAll('*') in gmail says 6800 nodes (on desktop).
> > 
> > Counted these by hand and there are 128,516 elements, 187,995 text nodes,
and
> a
> > total of 319,735 nodes.
> 
> I'm confused by this discrepancy.  Does gmail just hold a ton of content in
> iframes?

I'm sorry, my comment was confusing. Those numbers were for the html5 single
page spec.

[pdr., 2014-05-30 03:47:17]

The CQ bit was unchecked by pdr@chromium.org

[pdr., 2014-05-30 03:47:22]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-05-30 03:48:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pdr@chromium.org/305883002/1

[commit-bot: I haz the power, 2014-05-30 04:25:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9848)

[commit-bot: I haz the power, 2014-05-30 05:19:39]

Change committed as 175107