The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla15

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Assigned: Ehsan)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 617105 [details] [diff] [review]
Patch (v1)
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #617105 - Flags: review?(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!
Attachment #617105 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

5 years ago
(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.  :-)
(Assignee)

Comment 4

5 years ago
Nathan, thanks for filing this bug.  :-)

https://hg.mozilla.org/projects/birch/rev/25f093f5a4bd
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Whiteboard: [MemShrink]
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.
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?
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.
(Assignee)

Comment 8

5 years ago
(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.
> 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!
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/mozilla-central/rev/25f093f5a4bd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 757644
You need to log in before you can comment on or make changes to this bug.