Last Comment Bug 741027 - Downloading new emails from pop3 server rapidly slows down with each downloaded email when quarantining turned on
: Downloading new emails from pop3 server rapidly slows down with each download...
Status: RESOLVED FIXED
: perf, regression
Product: MailNews Core
Classification: Components
Component: Networking: POP (show other bugs)
: 13
: All All
: -- major (vote)
: Thunderbird 14.0
Assigned To: David :Bienvenu
:
Mentors:
Depends on:
Blocks: 402392
  Show dependency treegraph
 
Reported: 2012-03-30 18:22 PDT by Honza Bambas (:mayhemer)
Modified: 2012-04-03 12:49 PDT (History)
5 users (show)
mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
+
fixed


Attachments
proposed fix with unit test (4.11 KB, patch)
2012-04-02 15:27 PDT, David :Bienvenu
standard8: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Honza Bambas (:mayhemer) 2012-03-30 18:22:05 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/5c4189019bc1 (I don't know what comm-central revision it is)

I recently lost my TB profile, so I'm redownloading all messages from a pop3 
server.  There is over some 36 thousands emails.

I noticed that after some 5000 email aprox. had been downloaded, the process slowed rapidly down.  At some 15000 email each mail takes about 1 second to download.

nsPop3Sink::IncorporateBegin is the culprit.  It call CreateUnique to create a temp download file.  Problem is that from some reason the prior temp files are not deleted.

I have no idea how nsPop3Sink exactly works, but both places where m_tmpDownloadFile->Remove(false) is called are not hit.
Comment 1 Honza Bambas (:mayhemer) 2012-03-30 18:26:04 PDT
Looks like if (m_pop3ConData->last_accessed_msg >= m_pop3ConData->number_of_messages) at nsPop3Protocol::GetMsg() never passes...

I get back to this, if this is not known or someone else doesn't take, what I would prefer, later next week maybe.

Till that time I'll delete the temp files manually....
Comment 2 David :Bienvenu 2012-03-30 21:11:46 PDT
oh, interesting. I should fix that.
Comment 3 :aceman 2012-03-31 04:35:09 PDT
Can you tell us what are the names of the temp files? What is the pattern?
There are still several of these "not cleaning temp files"-bugs open but they  did presumably happen at other actions of TB (not POP3 download).
Comment 4 Honza Bambas (:mayhemer) 2012-03-31 11:24:36 PDT
(In reply to :aceman from comment #3)
> Can you tell us what are the names of the temp files? What is the pattern?

newmsg, and as CreateUnique creates, followings are newmsg-1, newmsg-2 ... newmsg-999.

> There are still several of these "not cleaning temp files"-bugs open but
> they  did presumably happen at other actions of TB (not POP3 download).

This is specific to POP3 as I understood the code.  It still happens, I still have to delete the files manually.

Just a reminder: this happens on a fresh profile.  I use global inbox for the account.  At the times I was able to reproduce this there were just a single pop3 account.
Comment 5 :aceman 2012-03-31 11:35:56 PDT
Good, so it is another type of temp files and are not covered by bug 299655.
Comment 6 David :Bienvenu 2012-04-02 15:27:13 PDT
Created attachment 611620 [details] [diff] [review]
proposed fix with unit test

Ghe quarantining stuff works by creating one temp file per get new mail session and re-using it by truncating it for every message downloaded, but we were creating a tmp file for every message. I probably messed this up during the pluggable store work. This fixes the leak, and adds a unit test.
Comment 7 David :Bienvenu 2012-04-02 15:28:52 PDT
we'll want this fix landed pretty soon on all branches.
Comment 8 David :Bienvenu 2012-04-03 12:33:38 PDT
fixed on trunk - http://hg.mozilla.org/comm-central/rev/27aac7b2e676

I'll transplant to branches in a minute.

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