747516 - mozilla::dom::Link should be one word smaller

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nathan Froyd [:froydnj]]

Link's members are laid out like so:

  nsLinkState mLinkState;

  mutable nsCOMPtr<nsIURI> mCachedURI;

  bool mRegistered;

  Element * const mElement;

  // Strong reference to History.  The link has to unregister before History
  // can disappear.
  nsCOMPtr<IHistory> mHistory;

Besides this introducing 11 (!) bytes of padding on a 64-bit system, there's no need for mLinkState to be a full integer.  nsLinkState only has a handful of values.  mLinkState should be a PRUInt16 or similar; mRegistered could then be packed with it, which would save a word on 32-bit and 64-bit systems.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 617105 [details] [diff] [review]
Patch (v1)

r=me.  I wish compilers would get with the program and just let us declare the sizes of enums!

Comment 3

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> r=me.  I wish compilers would get with the program and just let us declare
> the sizes of enums!

If you're holding your breath to inject some sanity into any C++ compiler soon, be warned that you might need to hold it for way longer than recommended by the medical community.  :-)

Comment 4

[(no longer active)]

Nathan, thanks for filing this bug.  :-)

https://hg.mozilla.org/projects/birch/rev/25f093f5a4bd

Comment 5

[Boris Zbarsky [:bzbarsky]]

We don't need sanity.  We just need to drop gcc 4.2 on Mac and switch to clang with -std=c++0x.  Well, and probably turn on c++0x mode in gcc on Linux too.

Comment 6

[Nicholas Nethercote [inactive]]

Nice patch!  I had to use PRInt16 instead of a sized enum in bug 712865 as well.

What were the before and after sizes (on both 32-bit and 64-bit)?  And roughly how many Link objects do we have live at a time?

Comment 7

[Boris Zbarsky [:bzbarsky]]

It depends on the page.  Basically, one per <a> or <link> or <area> element.  So order of dozens to hundreds per page, though pathological cases could have tens of thousands on a page, of course.

Comment 8

[(no longer active)]

(In reply to Nicholas Nethercote [:njn] from comment #6)
> Nice patch!  I had to use PRInt16 instead of a sized enum in bug 712865 as
> well.
> 
> What were the before and after sizes (on both 32-bit and 64-bit)?  And
> roughly how many Link objects do we have live at a time?

This should save 4 bytes on 32-bit and 8 bytes on 64-bit per Link object.

Comment 9

[Nicholas Nethercote [inactive]]

> This should save 4 bytes on 32-bit and 8 bytes on 64-bit per Link object.

I should have guessed that from the bug title :)  Thanks for clarifying!

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/25f093f5a4bd