1476416 - Optimize the memory usage of preference var caches

Status: NEW

(Core :: Preferences: Backend, enhancement)

Description

[Kris Maglione [:kmag]]

Most of the remaining per-process memory overhead in the preference service. In an empty content process, they add about 20K of CacheData structures (and the array that contains them) and another 30K or so of callback objects.

At a minimum, each entry currently takes up 72 bytes.

We could get rid of a lot of this overhead by using a single callback for all var caches, and maintaining a separate list for each type of cache:

  struct VarCacheEntry
  {
    nsCString mPref;
    DefaultPrefValue mDefaultValue;
    void* mCacheLocation;
  };

  struct VarCacheType
  {
    PrefChangedFunc mFunc;
    nsTArray<VarCacheEntry> mEntries;
  };

  VarCacheType varCaches[] = {
    { &IntVarChanged },
    { &AtomicIntVarChanged },
    { &FloatVarChanged },
    ...
  };

That would get us closer to 24 bytes per entry, and save about 35K per
process.

For static entries, we could probably go a lot further, though. We should be able to just store a static list of offsets to preference names, cache types, and default values, and just iterate over them. That would probably save us closer to 45K per process. The only unshared memory it would require would be for the members we store the cached values in.

Comment 1

[Kris Maglione [:kmag]]

Nick, do you have any current plans for static var cache preferences, or is this something I should work on?

If it is, do you have any opinions on the approach?

Comment 2

[Nicholas Nethercote [inactive]]

I am not working on this so please feel free to do so.

Here's how I had thought about this in the past: a big part of the problem is the need to store a default value. If we could get rid of that, then each CallbackNode could point directly to the global variable, and then CacheData and gCacheData could both be removed.

I think the only way that the default value is ever used was if a pref with a VarCache was deleted (which seems unlikely) -- then the table lookup would fail. One possibility would be to disallow deletion of such prefs. Another possibility would be to just leave the VarCache value as whatever it was just before the pref was deleted.

Your suggested approach may be a better alternative.

Comment 3

[Nicholas Nethercote [inactive]]

Here's how I had thought about this in the past: a big part of the problem
is the need to store a default value. If we could get rid of that, then each
CallbackNode could point directly to the global variable, and then CacheData
and gCacheData could both be removed.

This is now done, and it saved 20-32 KiB per process (on 64-bit).

A more optimized callback representation is still possible, and would save some more space.

Comment 4

[Nicholas Nethercote [inactive]]

Note to self: further improvement would be simplified by disallowing deletion of static prefs (bug 1576537).

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.