Last Comment Bug 641663 - Hidden memory costs of strings in nsCacheEntry instances
: Hidden memory costs of strings in nsCacheEntry instances
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: Cache (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Alon Zakai (:azakai)
:
Mentors:
Depends on:
Blocks: 634642 mslim-fx5+
  Show dependency treegraph
 
Reported: 2011-03-14 15:08 PDT by Alon Zakai (:azakai)
Modified: 2011-03-29 12:00 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
2.0next+


Attachments
patch (1.05 KB, patch)
2011-03-14 15:10 PDT, Alon Zakai (:azakai)
cbiesinger: review+
Details | Diff | Review

Description Alon Zakai (:azakai) 2011-03-14 15:08:36 PDT
nsCacheEntry reports its Size as mDataSize + mMetaData.Size(). However, it also holds a pointer to an nsCString (mKey) which it keeps alive. When testing on popular websites like google.com, you often see mKey be 300-400 bytes in length. As a result, in many cases the size of mKey is of the order of what nsCacheEntry reports as its size.

In other words, there is a potentially significant amount of memory that is used but not reported to about:cache, and not taken into consideration for how many entries to keep in the cache (but this isn't technically a leak). In a simple benchmark (see 'test page' attachment in bug 634642, run over hundreds of page loads), it looks like mKey is an additional 20% over the reported size of nsCacheEntry.
Comment 1 Alon Zakai (:azakai) 2011-03-14 15:10:27 PDT
Created attachment 519252 [details] [diff] [review]
patch

Patch makes nsCacheEntry::Size() take into account the length of mKey, so it more accurately represents how much memory the nsCacheEntry uses.
Comment 2 Alon Zakai (:azakai) 2011-03-17 15:05:21 PDT
Comment on attachment 519252 [details] [diff] [review]
patch

Looks good on try.

biesi, can you please take a look (or tell me who I should ask, I'm not sure who reviews netwerk/cache stuff)?
Comment 3 Alon Zakai (:azakai) 2011-03-17 16:54:41 PDT
Requesting blocking-fennec since this can save 1MB of memory in some cases (we allow 1MB for memory cache, and the amount of memory actually used can be twice as much due to these strings).
Comment 4 Stuart Parmenter 2011-03-18 01:00:01 PDT
Lets get this in to our next release
Comment 5 Christian :Biesinger (don't email me, ping me on IRC) 2011-03-25 15:36:08 PDT
Comment on attachment 519252 [details] [diff] [review]
patch

Yeah, cache patches get reviewed by me or one of the other necko peers
Comment 6 Alon Zakai (:azakai) 2011-03-29 12:00:29 PDT
http://hg.mozilla.org/mozilla-central/rev/8dbfdd56997b

Note You need to log in before you can comment on or make changes to this bug.