Trivial changes to default Vector sizing in the shadow area.

Trivial changes to default Vector sizing in the shadow area.

This enables us to lower the default size expansion of Vector from 16 to 4
without taking a performance hit on
ShadowDOM/DistributionWithMultipleShadowRoots

These changes may even make performance faster than before the default Vector
size change, but the bots will have to be the judge of that.

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

[esprehn, 2013-09-10 03:11:40]

This is going to cause copies inside canSelectNode() since
ShadowDOMSiblingTraversalStrategy doesn't understand it's a Vector<Node, 32>
when it assigns I think?

https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h
File Source/wtf/Vector.h (right):

https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h#newcode931
Source/wtf/Vector.h:931: ALWAYS_INLINE void Vector<T,
inlineCapacity>::uncheckedAppend(const U& val)
What does this do to the binary size?

[cevans, 2013-09-10 04:02:13]

On Tue, Sep 10, 2013 at 4:11 AM, <esprehn@chromium.org> wrote:

> This is going to cause copies inside canSelectNode() since
> ShadowDOMSiblingTraversalStrat**egy doesn't understand it's a
> Vector<Node, 32>
> when it assigns I think?
>
>
>
>
>
https://codereview.chromium.**org/23766020/diff/1/Source/**wtf/Vector.h<https...
> File Source/wtf/Vector.h (right):
>
> https://codereview.chromium.**org/23766020/diff/1/Source/**
>
wtf/Vector.h#newcode931<https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h#newcode931>
> Source/wtf/Vector.h:931: ALWAYS_INLINE void Vector<T,
> inlineCapacity>::**uncheckedAppend(const U& val)
> What does this do to the binary size?
>

I'll get the answer to that, but I did want to immediately note the
weirdness that the slow call Vector::append is _already_ ALWAYS_INLINE
wheres the fast'n'dirty Vector::uncheckedAppend is missing the designation.


>
https://codereview.chromium.**org/23766020/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Chris Evans, 2013-09-11 08:01:11]

On 2013/09/10 03:11:40, esprehn wrote:
> This is going to cause copies inside canSelectNode() since
> ShadowDOMSiblingTraversalStrategy doesn't understand it's a Vector<Node, 32>
> when it assigns I think?

Whoa, good spotting Elliot!

That's a foul, foul implicit constructor. I would have preferred a compile error
by far. Let's diverge to https://codereview.chromium.org/23903041/ to make it
urgently explicit. And mop up some other probably unintended copies perhaps.


> 
> https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h
> File Source/wtf/Vector.h (right):
> 
> https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h#newcode931
> Source/wtf/Vector.h:931: ALWAYS_INLINE void Vector<T,
> inlineCapacity>::uncheckedAppend(const U& val)
> What does this do to the binary size?

[Chris Evans, 2013-09-12 07:35:22]

On 2013/09/10 03:11:40, esprehn wrote:
> This is going to cause copies inside canSelectNode() since
> ShadowDOMSiblingTraversalStrategy doesn't understand it's a Vector<Node, 32>
> when it assigns I think?

Fixed! (And the other CL makes accidental implicit copies here a compile error).

> 
> https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h
> File Source/wtf/Vector.h (right):
> 
> https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h#newcode931
> Source/wtf/Vector.h:931: ALWAYS_INLINE void Vector<T,
> inlineCapacity>::uncheckedAppend(const U& val)
> What does this do to the binary size?

The patch overall is about 300 bytes delta.

I've verified that this patch enables us to reduce the default Vector capacity
from 16 down to 4 without regressing
ShadowDOM/DistributionWithMultipleShadowRoots any more. Once this is landed, I
can re-land the Vector capacity change and we'll claim some progressions on
Parser/* hopefully :)

[Chris Evans, 2013-09-12 07:36:19]

On 2013/09/12 07:35:22, Chris Evans wrote:
> On 2013/09/10 03:11:40, esprehn wrote:
> > This is going to cause copies inside canSelectNode() since
> > ShadowDOMSiblingTraversalStrategy doesn't understand it's a Vector<Node, 32>
> > when it assigns I think?
> 
> Fixed! (And the other CL makes accidental implicit copies here a compile
error).
> 
> > 
> > https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h
> > File Source/wtf/Vector.h (right):
> > 
> >
https://codereview.chromium.org/23766020/diff/1/Source/wtf/Vector.h#newcode931
> > Source/wtf/Vector.h:931: ALWAYS_INLINE void Vector<T,
> > inlineCapacity>::uncheckedAppend(const U& val)
> > What does this do to the binary size?
> 
> The patch overall is about 300 bytes delta.
> 
> I've verified that this patch enables us to reduce the default Vector capacity
> from 16 down to 4 without regressing
> ShadowDOM/DistributionWithMultipleShadowRoots any more. Once this is landed, I
> can re-land the Vector capacity change and we'll claim some progressions on
> Parser/* hopefully :)

Forgot to add, PTAL!

[abarth-chromium, 2013-09-12 22:44:48]

lgtm

[commit-bot: I haz the power, 2013-09-12 22:45:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cevans@chromium.org/23766020/9001

[commit-bot: I haz the power, 2013-09-13 01:57:48]

Change committed as 157725