Closed Bug 736919 Opened 10 years ago Closed 6 years ago

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

Categories

(Core :: Networking: Cache, defect)

12 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: n.nethercote, Unassigned)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file)

Attached patch patch, v0Splinter Review
+++ 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.
Attachment #607047 - Flags: feedback?(jduell.mcbugs)
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.
Attachment #607047 - Flags: feedback?(jduell.mcbugs) → feedback?(michal.novotny)
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.
Attachment #607047 - Flags: feedback?(michal.novotny) → feedback-
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee: nobody → n.nethercote
Assignee: n.nethercote → nobody
patches code that is deprecated..
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.