Closed Bug 1047220 Opened 7 years ago Closed 7 years ago

Shrink the static atoms table

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: n.nethercote, Assigned: n.nethercote)

Details

(Whiteboard: [MemShrink])

Attachments

(1 file)

The static atoms table has this type:

> nsDataHashtable<nsStringHashKey, nsIAtom*>

Each entry's key is an nsString and its value is a nsIAtom pointer... but those
types store basically the same data. We can remove the nsString and reduce the
size of entries and the whole table as follows (if my calculations are
correct).

* 32-bit: entries: 20 bytes to 8 bytes;  table:  80 KiB to 32 KiB
* 64-bit: entries: 32 bytes to 16 bytes; table: 128 KiB to 64 KiB
Suggestion on a better way to compare two char16_t strings are welcome.
Attachment #8465980 - Flags: review?(bzbarsky)
Comment on attachment 8465980 [details] [diff] [review]
Shrink the static atoms table

>+  typedef char16ptr_t KeyType;

Why not use |const nsAString&| as the KeyType?

>+  typedef char16ptr_t KeyTypePointer;

And |const nsAString*| here.  Basically crib nsStringHashKey.

>+    return nsString(aKey).Equals(nsString(mAtom->GetUTF16String()));

And this would become:

  return mAtom->Equals(aKey);

>+        gStaticAtomTable->PutEntry(atom->GetUTF16String());

  PutEntry(nsDependentAtomString(atom))

>+    gStaticAtomTable->GetEntry(PromiseFlatString(aUTF16String).get());

  GetEntry(aUTF16String)

And then you won't have issues with embedded nulls (not like someone should be doing that with static atoms anyway).

r=me with that.
Attachment #8465980 - Flags: review?(bzbarsky) → review+
> Why not use |const nsAString&| as the KeyType?

Mmm, good idea.

A try run (without that change) looks good: https://tbpl.mozilla.org/?tree=Try&rev=67eaeb215af7
Another good-looking try run, this time with bz's suggested change:
https://tbpl.mozilla.org/?tree=Try&rev=cf2ab78eef8c
https://hg.mozilla.org/mozilla-central/rev/99a0f93939d2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.