This will reduce memory usage by 30--45 KB per process on 64-bit, and slightly more on 32-bit. It will also make Pref storage more flexible, which will allow further memory usage reductions in the future, e.g. bug 1437168.

This has two advantages. First, it reduces memory usage, as per the following calculation. 64-bit: - Old sizes: - sizeof(Pref) = 32 - New sizes: - sizeof(PrefEntry) = 16 - sizeof(Pref) = 32 - Change: - -16 per empty slot in the hash table - +16 per used slot - A win if less than half the table slots are used 32-bit - Old sizes: - sizeof(Pref) = 20 - New sizes: - sizeof(PrefEntry) = 8 - sizeof(Pref) = 16 - Change: - -12 per empty slot in the hash table - +4 per used slot in the hash table - A win if table is < 75% full Table size: - The table is currently less than half full: ~3100 used out of 8192 slots. - The table is always <= 75% full, because that's the max load factor (for non-gigantic tables). - Therefore it's a win for both cases. Old sizes, chrome process, 64-bit: > 718,712 B (00.36%) -- preferences > +--262,176 B (00.13%) -- hash-table > +--197,384 B (00.10%) -- callbacks > +--114,688 B (00.06%) -- pref-name-arena > +---92,240 B (00.05%) -- root-branches > +---30,456 B (00.02%) -- string-values > +---21,688 B (00.01%) -- cache-data > +-------80 B (00.00%) -- misc New sizes, chrome process, 64-bit: > 672,568 B (00.41%) -- preferences > +--181,160 B (00.11%) -- callbacks > +--131,104 B (00.08%) -- hash-table # smaller > +--114,688 B (00.07%) -- pref-name-arena > +--101,152 B (00.06%) -- pref-values # new > +---92,240 B (00.06%) -- root-branches > +---30,456 B (00.02%) -- string-values > +---21,688 B (00.01%) -- cache-data > +-------80 B (00.00%) -- misc Old sizes, smallest content process, 64-bit: > 500,712 B (02.89%) -- preferences > +--262,176 B (01.51%) -- hash-table > +--114,688 B (00.66%) -- pref-name-arena > +---62,520 B (00.36%) -- callbacks > +---30,456 B (00.18%) -- string-values > +---17,832 B (00.10%) -- cache-data > +---12,960 B (00.07%) -- root-branches > +-------80 B (00.00%) -- misc New sizes, smallest content process, 64-bit: > 470,792 B (02.70%) -- preferences > +--131,104 B (00.75%) -- hash-table # smaller > +--114,688 B (00.66%) -- pref-name-arena > +--101,152 B (00.58%) -- pref-values # new > +---62,520 B (00.36%) -- callbacks > +---30,456 B (00.17%) -- string-values > +---17,832 B (00.10%) -- cache-data > +---12,960 B (00.07%) -- root-branches > +-------80 B (00.00%) -- misc The "hash-table" values drop by more than the size of the new "pref-values" value. On 64-bit, this reduces memory usage per process by 30--40 KB. On 32-bit, the number is slightly more. The second major advantage of this change is flexibility -- it opens up the possibility of different Pref objects being stored in different way. For example, static Prefs could be stared statically, letting them be shared between processes so long as they don't change (see bug 1437168). MozReview-Commit-ID: KmgbJaoOQ1J

Comment on attachment 8950842 [details] [diff] [review] Store a Pref* in each hash table slot instead of a Pref Review of attachment 8950842 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the thorough measurments! This seems fine as-is, but is a bit tedious. WDYT about taking the opportunity to switch to nsClassHashtable<nsDepCharHashKey, Pref>? If you remove `Pref::mName` I *think* it should have the same overhead. It looks like there are only a few spots that care about the name and it's only during iteration so you could use `iter.Key()` instead. ::: modules/libpref/Preferences.cpp @@ +374,2 @@ > { > + if (mName == aPrefName) { I get it's not new, but this test seems really weird, like what are the odds we're feeding in the same (arena allocated) string?

> WDYT about taking the opportunity to switch to > nsClassHashtable<nsDepCharHashKey, Pref>? The API of that class is sufficiently different to that of PLDHashTable that I would prefer to stick with PLDHashTable for now. > > + if (mName == aPrefName) { > > I get it's not new, but this test seems really weird, like what are the odds > we're feeding in the same (arena allocated) string? Good point. I adding a debugging printf and it was never true during a short stint of browsing. I will remove it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1980472ad0854bda24d089bcc32d92df0a579c4c Bug 1438088 - Store a Pref* in each hash table slot instead of a Pref. r=erahm.