Last Comment Bug 480843 - Implement memory reporter for mailnews
: Implement memory reporter for mailnews
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: 22
: All All
: -- enhancement with 1 vote (vote)
: Thunderbird 22.0
Assigned To: Joshua Cranmer [:jcranmer]
:
Mentors:
: 697156 (view as bug list)
Depends on: 805023 857373 aboutmemory 857036
Blocks: tb-logging 787751 796989 799535 823573 826494 844937
  Show dependency treegraph
 
Reported: 2009-03-01 12:07 PST by Wayne Mery (:wsmwk, use Needinfo for questions)
Modified: 2013-04-13 16:20 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Prototype of database memory reporter (17.06 KB, patch)
2012-10-10 21:39 PDT, Joshua Cranmer [:jcranmer]
neil: feedback+
Details | Diff | Review
Add a reporter for databases (16.65 KB, patch)
2013-03-21 20:28 PDT, Joshua Cranmer [:jcranmer]
irving: review+
Details | Diff | Review
External API build fix (704 bytes, patch)
2013-04-06 07:00 PDT, neil@parkwaycc.co.uk
irving: review+
Details | Diff | Review

Description Wayne Mery (:wsmwk, use Needinfo for questions) 2009-03-01 12:07:41 PST
Implement memory reporter for mailnews.
xref Bug 472209 which begins implementing reporters for core
Comment 1 Wayne Mery (:wsmwk, use Needinfo for questions) 2011-08-23 11:46:31 PDT
Asuth from PM, "Now that Taras Glek has added telemetry probes, there may be even more value in exposing that information, as it can automatically capture and report histograms of data.  So we could see how long it is taking to open folders for people, perhaps how many messages are in their folders, how big those folders are, etc., in aggregate."

I'd be very happy with #open folders and agg. size of .msf of open folders :)


and we have WIP Bug 643796 - about:support - indicate folders which may be near or exceed OS/Thunderbird limits. Or over N GB in size

linked to meta bug 492620, which till now has I think only been about logging, but it has diagnostics in the summary title, so WTH
Comment 2 Joshua Cranmer [:jcranmer] 2011-08-23 13:06:02 PDT
For what it's worth, I should point out that mork appears to have methods that let you print out the size in memory of various databases, which may be useful.
Comment 3 Wayne Mery (:wsmwk, use Needinfo for questions) 2012-01-19 10:33:31 PST
*** Bug 697156 has been marked as a duplicate of this bug. ***
Comment 4 Joshua Cranmer [:jcranmer] 2012-10-10 21:39:22 PDT
Created attachment 670241 [details] [diff] [review]
Prototype of database memory reporter

I played around a bit and got some basic memory reporting for databases working. It's not ready for release, since I am undercounting a lot of memory (I need to include more array sizes, and iterate over arrays to collect size of more data).

As probably the resident most-expert on mork around nowadays, I shouldn't be saying this, but I'm not 100% sure that the nsIMdbHeap GetUsedSize method is truly indicative of the memory being used for the mork heap--bienvenu, if you recall, can you confirm or deny this?

I doubt I'll have time to bang on this any more over the next few weeks, but if anyone wants to pick up this patch, the following needs to be done:
1. Override the SizeOfExcludingThis in the NNTP database so you can include the read set (none of the other subclasses have owning references to other objects).
2. Figure out what's going on with the two header cache tables... I don't want to double-count any data here.
3. Fix the reporting for nsMsgDatabase to work properly. This means we need to account for the size of all members we own (which, despite several comments about not having strong references, is nearly all of the members in nsMsgDatabase).
4. Breaking up the reporters to separate out raw database size from other types of objects may be helpful.
5. We probably want to sanitize the database names somehow, since folderURL will contain usernames for IMAP and local folder hierarchies for local folders. Account key + folder path is probably good enough.

As a rough metric for numbers, the message DB on my admittedly puny test profile amount to around 1/3 of heap-uncommitted as it stands right now. A brief test locally (on TB 16) of going from an empty folder to a 7K-message folder appears to induce 10-20MB of heap-unclassified. The message database for a 1700-message folder reported ~2.5MB used by the database.
Comment 5 neil@parkwaycc.co.uk 2013-02-26 06:13:01 PST
Comment on attachment 670241 [details] [diff] [review]
Prototype of database memory reporter

>+  virtual size_t GetUsedSize() = 0;
[How many of these need to be virtual?]

>+  return static_cast<nsMsgHdr*>(entry->mHdr)->
>+    SizeOfExcludingThis(aMallocSizeOf);
Excluding for members, but Including for pointers, no?
Comment 6 Joshua Cranmer [:jcranmer] 2013-03-21 20:28:38 PDT
Created attachment 728032 [details] [diff] [review]
Add a reporter for databases

After some more exploration, I've discovered this slightly undercounts the database (a few things related to the mork factories aren't counted properly). I would run with DMD to figure out what specifically is undercounted, but the DMD is slightly broken with comm-central since it fails to record that the reporter is actually reporting memory (we don't get the MOZ_DMD defined). Once we get bug 846540 fixed, we'll use mozilla-config.h everywhere and we can revisit under/overreporting.

We'll also need reporters for the folder cache and address book databases, and some snooping in DMD indicates that the spam filter and IMAP protocols are other big offenders.
Comment 7 :Irving Reid (No longer working on Firefox) 2013-03-31 13:57:30 PDT
Comment on attachment 728032 [details] [diff] [review]
Add a reporter for databases

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

Joshua, thanks so much for putting this together. My about:memory page already looks way better.

::: db/mork/src/orkinHeap.cpp
@@ +42,4 @@
>  // { ===== begin nsIMdbHeap methods =====
>  /*virtual*/ mdb_err
>  orkinHeap::Alloc(nsIMdbEnv* mev, // allocate a piece of memory
> +  mdb_size inSize,  // requested size of new memory block 

trailing white space

::: db/mork/src/orkinHeap.h
@@ +26,1 @@
>    

existing trailing white space right next to your changed line ;->

::: mailnews/db/msgdb/public/nsDBFolderInfo.h
@@ +58,5 @@
>    nsresult GetUint64PropertyWithToken(mdb_token columnToken,
>                                        uint64_t *propertyValue);
>  
>    nsTArray<nsMsgKey> m_lateredKeys; // list of latered messages
>    

more adjacent old white space

::: mailnews/db/msgdb/public/nsMsgDatabase.h
@@ +427,3 @@
>  private:
>    uint32_t m_cacheSize;
> +  nsRefPtr<mozilla::mailnews::MsgDBReporter> mMemReporter;

Can mMemReporter be a direct member, rather than a refcounted pointer to another heap object? I suppose that might not play well with the mem reporter being an XPCOM object...
Comment 8 Joshua Cranmer [:jcranmer] 2013-03-31 22:00:55 PDT
(In reply to Irving Reid (:irving) from comment #7)
> ::: mailnews/db/msgdb/public/nsMsgDatabase.h
> @@ +427,3 @@
> >  private:
> >    uint32_t m_cacheSize;
> > +  nsRefPtr<mozilla::mailnews::MsgDBReporter> mMemReporter;
> 
> Can mMemReporter be a direct member, rather than a refcounted pointer to
> another heap object? I suppose that might not play well with the mem
> reporter being an XPCOM object...

We might be able to do that by making the refcount of the memory reporter use the database refcount instead or some other trickery like that. But it would be hacky...
Comment 9 Joshua Cranmer [:jcranmer] 2013-03-31 22:06:36 PDT
https://hg.mozilla.org/comm-central/rev/71de34b00fc2

Should make the Aurora merges today. :-)
Comment 10 Ludovic Hirlimann [:Usul] 2013-04-02 04:09:10 PDT
(In reply to Joshua Cranmer [:jcranmer] from comment #9)
> https://hg.mozilla.org/comm-central/rev/71de34b00fc2
> 
> Should make the Aurora merges today. :-)

So this will show up in about:memory ?
Comment 11 Wayne Mery (:wsmwk, use Needinfo for questions) 2013-04-02 04:22:51 PDT
(In reply to Ludovic Hirlimann [:Usul] from comment #10)
> (In reply to Joshua Cranmer [:jcranmer] from comment #9)
> > https://hg.mozilla.org/comm-central/rev/71de34b00fc2
> > 
> > Should make the Aurora merges today. :-)
> 
> So this will show up in about:memory ?

look for maildb. as in the following

├──586,234,752 B (467.33%) -- maildb [?!]
│  ├──315,577,824 B (251.57%) ── database(imap://vseerror@mail.lehigh.edu/Moz) [?!]
│  ├──116,056,240 B (92.52%) ── database(imap://vseerror@mail.lehigh.edu/INBOX)
│  ├──116,046,720 B (92.51%) ── database(imap://wsm0@mail.lehigh.edu/INBOX)
│  ├───25,392,432 B (20.24%) ── database(imap://wayne.mery@imap.gmail.com/INBOX)
│  ├────4,240,672 B (03.38%) ── database(imap://vseerror@mail.lehigh.edu/moz-lxmac/moz-FF)
│  ├────2,500,432 B (01.99%) ── database(imap://vseerror@mail.lehigh.edu/moz-lxmac/moz-filters)
│  ├────1,609,216 B (01.28%) ── database(imap://vseerror@mail.lehigh.edu/moz-lxmac/moz-compact)
│  ├────1,471,296 B (01.17%) ── database(news://news.mozilla.org:119/mozilla.support.thunderbird)
│  ├──────680,448 B (00.54%) ── database(imap://vseerror@mail.lehigh.edu/moz-lxmac/moz-gmNmaild)
│  ├──────477,120 B (00.38%) ── database(news://news.mozilla.org:119/mozilla.dev.apps.calendar)
│  ├──────427,632 B (00.34%) ── database(imap://wayne.mery@imap.gmail.com/%5BGmail%5D/Trash)
│  ├──────405,888 B (00.32%) ── database(imap://vseerror@mail.lehigh.edu/moz-lxmac/gsfn)
│  ├──────300,848 B (00.24%) ── database(imap://vseerror@mail.lehigh.edu/Trash) [2]
│  ├──────165,664 B (00.13%) ── database(imap://wsm0@mail.lehigh.edu/Trash)
│  ├──────138,384 B (00.11%) ── database(imap://wsmerynews%40gmail.com@imap.googlemail.com/INBOX)
│  ├──────114,256 B (00.09%) ── database(imap://concordchambersingers%40gmail.com@imap.googlemail.com/INBOX)
│  ├──────105,472 B (00.08%) ── database(imap://wsm0@mail.lehigh.edu/servers/linux.fld)
│  ├───────88,784 B (00.07%) ── database(mailbox:///C:/Users/Wayne/AppData/Roaming/Thunderbird/Profiles/9a159fc8.default/Mail/smart%20mailboxes/Inbox)
│  ├───────87,344 B (00.07%) ── database(imap://vseerror@mail.lehigh.edu/ageis1)
│  ├───────87,264 B (00.07%) ── database(imap://wsm0@mail.lehigh.edu/INBOX/14-wd)
│  ├───────87,264 B (00.07%) ── database(imap://wsm0@mail.lehigh.edu/INBOX/14d-w)
│  ├───────86,784 B (00.07%) ── database(imap://vseerror@mail.lehigh.edu/14d-v)
│  └───────86,768 B (00.07%) ── database(imap://vseerror@mail.lehigh.edu/rec-unread)
Comment 12 neil@parkwaycc.co.uk 2013-04-06 07:00:09 PDT
Created attachment 734239 [details] [diff] [review]
External API build fix

(Actually this is one of three fixes needed to get external API builds working, sigh...)
Comment 13 neil@parkwaycc.co.uk 2013-04-13 16:20:51 PDT
Comment on attachment 734239 [details] [diff] [review]
External API build fix

Pushed comm-central changeset 737e81a09f30.

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