736919 - Make "network-memory-cache" memory reporter traversal-based

Status: RESOLVED WONTFIX

(Core :: Networking: Cache, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch, v0

+++ This bug was initially created as a clone of Bug #713203 +++

The attached patch attempts to convert the "network-memory-cache" memory reporter traversal-based instead of counter-based, to avoid problems such as the one in bug 713203, and to integrate it with DMD.

Unfortunately, I think the lion's share of the memory is stored within nsCacheEntry::mData.  jduell, is that right?  And that has type |nsISupports*|, which makes it hard to query for the size... I can probably use |nsISizeOf| somehow to measure that memory.

Comment 1

[Jason Duell]

Comment on attachment 607047 [details] [diff] [review]
patch, v0

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

Michal or Nick are better reviewers than I for this.

Comment 2

[Michal Novotny [:michal]]

Comment on attachment 607047 [details] [diff] [review]
patch, v0

(In reply to Nicholas Nethercote [:njn] from comment #0)
> Unfortunately, I think the lion's share of the memory is stored within
> nsCacheEntry::mData.

You're right, this patch won't count allocated segments of nsStorageStream::SegmentedBuffer.


> And that has type |nsISupports*|, which makes it hard to query for the size..
> I can probably use |nsISizeOf| somehow to measure that memory.

nsCacheEntry::mData can be anything when the session is NOT_STREAM_BASED, but I think that there is no code that uses our cache in non-stream mode. For a stream based session mData can be:

- nsDiskCacheBinding in case of disk cache
- nsStorageStream in case of memory cache
- nsOfflineCacheBinding in case of offline cache


BTW there is missing a cache lock in the current code in nsCacheService::MemoryDeviceSize(). This bug isn't probably critical now since we just return an integer member. But it will become critical with your patch since you're enumerating eviction lists in nsMemoryCacheDevice::SizeOfIncludingThis() while they might change on another thread.

Comment 3

[Patrick McManus [:mcmanus]]

patches code that is deprecated..