Bug 188945 – Shrink size of HTMLCollection

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Shrink size of HTMLCollection

[Yusuke Suzuki]

Created attachment 348077 [details]
Patch

[Darin Adler]

Comment on attachment 348077 [details]
Patch

I wish there is something we could do to make sure people don’t accidentally undo this when making other changes in the future.

[Yusuke Suzuki]

(In reply to Darin Adler from comment #2)
> Comment on attachment 348077 [details]
> Patch
> 
> I wish there is something we could do to make sure people don’t accidentally
> undo this when making other changes in the future.

I think the only reasonable way we have right now is adding `static_assert(sizeof(HTMLCollection) <= 40, "")`.
But I don't think it is good since the size of HTMLCollection is only the part of the problem.
The rather important thing is HTMLCollection had many paddings.
The size can increase due to reasonable reasons. But increasing paddings should be avoided.
So it should be caught.

The ideal way would be having a test using dump-class-layout that ensures the padding rate is small in some important classes.
But this is not achievable without tooling and test infrastructure support.

So, in the meantime, I'll land this patch as is.

Simon, do you have any idea about introducing such a test (described above) into WebKit tests?

[Yusuke Suzuki]

Committed r235357: <https://trac.webkit.org/changeset/235357>

[Radar WebKit Bug Importer]

<rdar://problem/43746564>

[Simon Fraser (smfr)]

(In reply to Yusuke Suzuki from comment #3)
> (In reply to Darin Adler from comment #2)
> > Comment on attachment 348077 [details]
> > Patch
> > 
> Simon, do you have any idea about introducing such a test (described above)
> into WebKit tests?

We do have testing of dump-class-layout in webkitpy tests, so it wouldn't be hard to rig some up somehow.

[Simon Fraser (smfr)]

Comment on attachment 348077 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=348077&action=review

> Source/WebCore/html/HTMLCollection.h:107
>      const unsigned m_collectionType : 5;
>      const unsigned m_invalidationType : 4;

Would be nice to see comments say what the enum type is.

[Yusuke Suzuki]

Committed r235497: <https://trac.webkit.org/changeset/235497>