1442433 - Remove the refcount from static and HTML5 atoms

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

The refcount is only used for dynamic atoms.

On 64-bit, this reduces sizeof(nsStaticAtom) from 24 bytes to 16 bytes, and the
corresponding heap allocation from 32 bytes to 16 bytes. This saves 42 KiB per
process.

On 32-bit, this reduces sizeof(nsStaticAtom) from 16 bytes to 12 bytes, but the
corresponding heap allocated stays at 16 bytes, so memory usage is unchanged.

The change will also reduce the size of HTML5 atoms, but they are short-lived
so the difference won't be noticeable.

MozReview-Commit-ID: 7d9H7MRHN9a

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove the refcount from static atoms

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Make some nsAtom fields `const`

Also remove the unused SetKind() method.

MozReview-Commit-ID: CIh6BmN7OLI

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8955334 [details] [diff] [review]
Remove the refcount from static atoms

Review of attachment 8955334 [details] [diff] [review]:
-----------------------------------------------------------------

So easy.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8955409 [details] [diff] [review]
Make some nsAtom fields `const`

Review of attachment 8955409 [details] [diff] [review]:
-----------------------------------------------------------------

\o/

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Make nsAtom::mString more const

The patch also uses GetStringBuffer() in a couple of appropriate places.

MozReview-Commit-ID: JufCUgmO8JL

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: Make nsAtom::mString even more const

I'm a bit ambivalent about this one. What do you think?

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 8955967 [details] [diff] [review]
Make nsAtom::mString even more const

Review of attachment 8955967 [details] [diff] [review]:
-----------------------------------------------------------------

I think the idea is reasonable, even if the const_cast is ugly.

I wonder if we could push the constness farther, by:

1) making nsStringBuffer::FromData take `const void*`.  We'd still have to const_cast, but I think it's a little more justified inside FromData than elsewhere; and/or
2) making nsStringBuffer more const itself--mStorageSize should be const, as should the canary (I think).  We'd have to mark the refcnt `mutable`, but I think that's OK (see ConstRemovingRefPtrTraits in RefPtr.h for a discussion of the issues).

Both of those (especially #2) are followups, of course.

I think the patch improves the state of the world.

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e4bac75138f31d2c66a9ef2a235b4303c573b0c
Bug 1442433 - Remove the refcount from static atoms. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/786c4905fe76d92f6f45cdce39ea4eb1228791b9
Bug 1442433 - Make some nsAtom fields `const`. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/06e62ae8e4cc238cd75c69fa55997d28562ccf76
Bug 1442433 - Make nsAtom::mString more const. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca86b5f0a30a71d0e57e829176bd1eb611d8c2a1
Bug 1442433 - Make nsAtom::mString even more const. r=froydnj

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7e4bac75138f
https://hg.mozilla.org/mozilla-central/rev/786c4905fe76
https://hg.mozilla.org/mozilla-central/rev/06e62ae8e4cc
https://hg.mozilla.org/mozilla-central/rev/ca86b5f0a30a