Last Comment Bug 747516 - mozilla::dom::Link should be one word smaller
: mozilla::dom::Link should be one word smaller
Status: RESOLVED FIXED
[MemShrink]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on: 757644
Blocks: 492185
  Show dependency treegraph
 
Reported: 2012-04-20 13:31 PDT by Nathan Froyd [:froydnj]
Modified: 2012-05-22 15:39 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (1.64 KB, patch)
2012-04-20 14:50 PDT, :Ehsan Akhgari
bzbarsky: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-04-20 13:31:27 PDT
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 :Ehsan Akhgari 2012-04-20 14:50:37 PDT
Created attachment 617105 [details] [diff] [review]
Patch (v1)
Comment 2 Boris Zbarsky [:bz] 2012-04-20 14:57:22 PDT
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 :Ehsan Akhgari 2012-04-20 15:07:34 PDT
(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 :Ehsan Akhgari 2012-04-20 15:09:47 PDT
Nathan, thanks for filing this bug.  :-)

https://hg.mozilla.org/projects/birch/rev/25f093f5a4bd
Comment 5 Boris Zbarsky [:bz] 2012-04-20 15:19:41 PDT
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 [:njn] 2012-04-20 15:27:54 PDT
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 [:bz] 2012-04-20 15:33:12 PDT
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 :Ehsan Akhgari 2012-04-23 08:35:42 PDT
(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 [:njn] 2012-04-23 16:18:33 PDT
> 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!

Note You need to log in before you can comment on or make changes to this bug.