Move element flags from ElementRareData to NodeRareData

Move element flags from ElementRareData to NodeRareData

By doing this, they can be packed together with the existing bit-field
member NodeRareData::m_connectedFrameCount, and save some space.

Impact according to sizeof(ElementRareData), on 64-bit Linux (GCC and
Clang alike,) is a modest 8 byte saving, from 144 bytes to 136 bytes.

BUG=

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

[Jens Widell, 2014-03-17 15:09:57]

Please take a look.

This is a minor change that leads to an equally minor reduction in memory usage.
 :-)

[Inactive, 2014-03-17 15:19:14]

lgtm

+esprehn in cc so that he sees the change.

[eseidel, 2014-03-17 15:43:28]

MSVC is bad a packing... you shoud be sure to run this throgh teh try servers
first.

[Jens Widell, 2014-03-17 15:52:45]

On 2014/03/17 15:43:28, eseidel wrote:
> MSVC is bad a packing... you shoud be sure to run this throgh teh try servers
> first.

If this change would lead to MSVC packing the structures worse rather than
better, would the try servers notice?  I would have thought not.

Not unless the COMPILE_ASSERT() in ElementRareData.cpp triggers, but that's not
very likely since it mostly just checks that a supposedly identical structure is
in fact identical.  Which it ought to be, since I modified the supposedly
identical structure the same way...

But I will ask someone on Windows to manually check the effect there.

[Daniel Bratell, 2014-03-17 16:32:46]

On 2014/03/17 15:52:45, Jens Lindström wrote:
> On 2014/03/17 15:43:28, eseidel wrote:
> > MSVC is bad a packing... you shoud be sure to run this throgh teh try
servers
> > first.
> 
> If this change would lead to MSVC packing the structures worse rather than
> better, would the try servers notice?  I would have thought not.
> 
> Not unless the COMPILE_ASSERT() in ElementRareData.cpp triggers, but that's
not
> very likely since it mostly just checks that a supposedly identical structure
is
> in fact identical.  Which it ought to be, since I modified the supposedly
> identical structure the same way...
> 
> But I will ask someone on Windows to manually check the effect there.

I seem to get 84 bytes in VS2013 with or without this fix so Windows will not
benefit from this, only gcc (clang?).

[Daniel Bratell, 2014-03-17 20:29:10]

I can add that these objects are rare on most sites, but happened to be very
frequent when loading the html5 spec which I think one of us used as an
interesting test case. So on that site you at least didn't need a microscope.

[Jens Widell, 2014-03-18 08:55:01]

On 2014/03/17 15:43:28, eseidel wrote:
> MSVC is bad a packing... you shoud be sure to run this throgh teh try servers
> first.

Doesn't seem like "git cl try" is my friend; bots go red immediately, presumably
without try very much.

eseidel, are you happy enough with Daniel's MSVC measurements?

[esprehn, 2014-03-18 09:23:03]

ElementRareData looks really different after
https://src.chromium.org/viewvc/blink?revision=169426&view=revision

I think you should move the unsigned m_flags up into NodeRareData, and move the
get/set/clear Flag methods so it packs on all systems.

[Jens Widell, 2014-03-18 09:34:47]

On 2014/03/18 09:23:03, esprehn wrote:
> ElementRareData looks really different after
> https://src.chromium.org/viewvc/blink?revision=169426&view=revision

It certainly does. :-)

> I think you should move the unsigned m_flags up into NodeRareData, and move
the
> get/set/clear Flag methods so it packs on all systems.

Will look into that.

[Jens Widell, 2014-03-18 11:49:25]

On 2014/03/18 09:34:47, Jens Lindström wrote:
> On 2014/03/18 09:23:03, esprehn wrote:
> > ElementRareData looks really different after
> > https://src.chromium.org/viewvc/blink?revision=169426&view=revision
> 
> It certainly does. :-)
> 
> > I think you should move the unsigned m_flags up into NodeRareData, and move
> the
> > get/set/clear Flag methods so it packs on all systems.
> 
> Will look into that.

Done now in updated patch.  Please have another look.

The size change is the same, but should be more straight-forward now and not
depend so much on the compiler's packing skills.

[esprehn, 2014-03-20 08:07:08]

Lgtm, thanks!

[Jens Widell, 2014-03-20 08:52:17]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-03-20 08:52:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jl@opera.com/201153006/20001

[commit-bot: I haz the power, 2014-03-20 09:27:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 09:27:21]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[Jens Widell, 2014-03-20 10:33:37]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-03-20 10:33:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jl@opera.com/201153006/40001

[commit-bot: I haz the power, 2014-03-20 11:57:13]

Change committed as 169644