1441754 - Shrink CallbackNode

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Description

[Nicholas Nethercote [inactive]]

CallbackNode is inefficiently packed, which wastes memory.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441754 - clang-format some Preferences.cpp code that needs it.

Review commit: https://reviewboard.mozilla.org/r/223766/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223766/

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441754 - Split measurements of pref callbacks in two.

This shows that the objects themselves are accounting for about 60% of the
callback memory on 64-bit, and the domains are about 40%.

Review commit: https://reviewboard.mozilla.org/r/223768/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223768/

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441754 - Privatize CallbackNode's field.

This isn't compelling on its own, but it's necessary for the next patch.

Review commit: https://reviewboard.mozilla.org/r/223770/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223770/

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441754 - Shrink CallbackNode by combining mMatchKind and mNext into a tagged pointer.

The following table shows the effect of this change:

>                           old 64-bit  new 64-bit  old 32-bit  new 32-bit
> sizeof(CallbackNode)      40          32          20          16
> size when heap allocated  48          32          32          16

This reduces memory usage by about 15--40 KB per process.

Review commit: https://reviewboard.mozilla.org/r/223772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223772/

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441754 - Use MOZ_ASSERT instead of NS_ASSERTION and NS_PRECONDITION in Preferences.cpp.

I also removed the explanation string in cases where I felt it was obvious.

Review commit: https://reviewboard.mozilla.org/r/223774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223774/

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8954663 [details]
Bug 1441754 - clang-format some Preferences.cpp code that needs it.

https://reviewboard.mozilla.org/r/223766/#review230534

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8954664 [details]
Bug 1441754 - Split measurements of pref callbacks in two.

https://reviewboard.mozilla.org/r/223768/#review230536

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8954665 [details]
Bug 1441754 - Privatize CallbackNode's field.

https://reviewboard.mozilla.org/r/223770/#review230538

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8954666 [details]
Bug 1441754 - Shrink CallbackNode by combining mMatchKind and mNext into a tagged pointer.

https://reviewboard.mozilla.org/r/223772/#review230540

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8954667 [details]
Bug 1441754 - Use MOZ_ASSERT instead of NS_ASSERTION and NS_PRECONDITION in Preferences.cpp.

https://reviewboard.mozilla.org/r/223774/#review230542

Comment 11

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a6b832ce2521677f35b380245a621c2d46eb03c
Bug 1441754 - clang-format some Preferences.cpp code that needs it. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/17fe71b2e622c2ae4bfcc963156ddc8815f40dd1
Bug 1441754 - Split measurements of pref callbacks in two. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/13e0252c67c9caeb7fa73ae5b2e16080c84cb728
Bug 1441754 - Privatize CallbackNode's field. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6680fcb17a0392208b7413a214a662f8b8ca284
Bug 1441754 - Shrink CallbackNode by combining mMatchKind and mNext into a tagged pointer. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/76a740cc4454f92c44b10e63abcc3d68daf8ee06
Bug 1441754 - Use MOZ_ASSERT instead of NS_ASSERTION and NS_PRECONDITION in Preferences.cpp. r=glandium

Comment 12

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/4a6b832ce252
https://hg.mozilla.org/mozilla-central/rev/17fe71b2e622
https://hg.mozilla.org/mozilla-central/rev/13e0252c67c9
https://hg.mozilla.org/mozilla-central/rev/e6680fcb17a0
https://hg.mozilla.org/mozilla-central/rev/76a740cc4454