Reduce the size of `ManagedContainer`
Categories
(Core :: IPC, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox90 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
Details
Attachments
(1 file)
Some protocols, most notably PBackground, have a very large number of different types of managed actors. As each managed actor type gets its own hashtable (which is IIRC ~5 pointers wide), this can end up making PBackground a fairly large object. Given that in the majority of cases, we don't have very many managed actors, using a full hashtable seems somewhat inefficient here. We should experiment with using a smaller datastructure such as a nsTArray to store the list of managed actors instead.
|
Assignee | |
Comment 1•3 years ago
|
||
|
||
Comment 2•3 years ago
|
||
Do we have data about the container sizes? Not necessarily Telemetry, but maybe simple instrumentation that could be used locally. I'm wondering:
- What's the largest size we typically see? I'm worried that being deliberately quadratic here could cause problems.
- How often are the sizes 0 (or maybe 1)? If we could achieve most of the memory savings just by boxing the hash set and freeing it when it's empty, that would be useful to know.
|
|
Assignee | |
Comment 3•3 years ago
|
||
(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #2)
Do we have data about the container sizes? Not necessarily Telemetry, but maybe simple instrumentation that could be used locally. I'm wondering:
- What's the largest size we typically see?
I don't know the exact sizes, but I don't think they're very large in most cases. Most larger sets I can think of would probably be types like necko channels and IndexedDB/ServiceWorker operation actors, which are fairly short lived. Given how expensive each actor is, though, I'm somewhat skeptical that we'd have enough for this to be an issue. I suppose we could do some local instrumentation.
I'm worried that being deliberately quadratic here could cause problems.
The only real use of ManagedContainer outside of IPDL is for iterating (e.g. enumerating all managed PBrowser instances or getting the singleton PNecko instance), which should be faster with arrays as they're packed more densely and iteration is simpler than for a nsTHashSet. Iteration is also used when collecting all managed actors for methods like DestroySubtree.
Within IPDL we also need to handle insertion/deletion, which may be slower for for very large numbers of actors, but still should only be O(N), as we store the nsTArray ordered (using RemoveElementSorted and IndexOfFirstElementGt), and use an O(log(N)) binary search to find where to insert or which index to remove, followed by an O(N) memcpy to move elements around. It's admittedly not amazing for cache locality, but shouldn't be quadratic IIRC.
I don't think there should be any quadratic behaviour here off the top of my head, but perhaps I'm missing something?
- How often are the sizes 0 (or maybe 1)? If we could achieve most of the memory savings just by boxing the hash set and freeing it when it's empty, that would be useful to know.
I think that could definitely get us some of the memory savings, but I'm not sure the extra complexity of doing that is worth it, and it would make things like the somewhat common iteration tasks more complex. An adaptive "probably-small" table type might be interesting to have, but given that one hasn't already been written, I'm not sure it's worth writing just for this use-case.
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dbdb5b128732 Reduce the size of ManagedContainer types, r=mccr8
|
||
Comment 5•3 years ago
|
||
| bugherder | ||
Description
•