Closed Bug 480843 Opened 15 years ago Closed 11 years ago

Implement memory reporter for mailnews

Categories

(MailNews Core :: Backend, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 22.0

People

(Reporter: wsmwk, Assigned: jcranmer)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Implement memory reporter for mailnews.
xref Bug 472209 which begins implementing reporters for core
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
Blocks: tb-logging
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.
Severity: normal → enhancement
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.
Attachment #670241 - Flags: feedback?(mozilla)
Blocks: 799535
Attachment #670241 - Flags: feedback?(mozilla) → feedback?(neil)
Blocks: 844937
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?
Attachment #670241 - Flags: feedback?(neil) → feedback+
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.
Assignee: nobody → Pidgeot18
Attachment #670241 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #728032 - Flags: review?(irving)
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...
Attachment #728032 - Flags: review?(irving) → review+
(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...
https://hg.mozilla.org/comm-central/rev/71de34b00fc2

Should make the Aurora merges today. :-)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Version: Trunk → 22
(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 ?
(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)
Depends on: 857036
Depends on: 805023
Depends on: 857373
(Actually this is one of three fixes needed to get external API builds working, sigh...)
Attachment #734239 - Flags: review?(irving)
Attachment #734239 - Flags: review?(irving) → review+
Target Milestone: --- → Thunderbird 22.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: