IMAP downloadaing all headers instead of just headers required for d

VERIFIED FIXED

Status

MailNews Core
Networking: IMAP
VERIFIED FIXED
16 years ago
9 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

({perf, regression})

Trunk
perf, regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 RTM])

Attachments

(4 attachments)

(Assignee)

Description

16 years ago
 
(Assignee)

Comment 1

16 years ago
The fix for supporting custom headers accidentally broke the imap code which
determines which headers to download when opening a folder. We are now
downloading all headers instead of just the headers needed for the db when
opening a folder. I think this also hurts web-mail imap server performance since
we're going to ask for all headers. (If you turn on custom headers, everything
works like it used to, but very few people will turn on custom headers). I'll
attach a patch soon.
Status: NEW → ASSIGNED
Keywords: 4xp, nsbeta1, perf, regression
(Assignee)

Comment 2

16 years ago
Created attachment 79673 [details] [diff] [review]
proposed fix

OK, there was some confusion in the code about what arbitrary headers mean, as
opposed to custom headers. I've cleaned that all up and changed the method name
so people won't get confused.
(Assignee)

Comment 3

16 years ago
Navin, can you review? thx.
Comment on attachment 79673 [details] [diff] [review]
proposed fix

sr=sspitzer
Attachment #79673 - Flags: superreview+

Comment 5

16 years ago
Comment on attachment 79673 [details] [diff] [review]
proposed fix

r=naving
Attachment #79673 - Flags: review+

Comment 6

16 years ago
Comment on attachment 79673 [details] [diff] [review]
proposed fix

We know that we have not
implemented filters on
any headers. So why check
it, just take it to be	false


+	 PRBool downloadAllHeaders = PR_FALSE;
+	 GetShouldDownloadAllHeaders(&downloadAllHeaders); // checks if we're
filtering on "any header"
+	 if (!downloadAllHeaders)  // if it's ok -- no filters on any header,
etc.
	 {
	   char *headersToDL = nsnull;
	   char *what = nsnull;
(Assignee)

Comment 7

16 years ago
the filters code knows that, but the imap code doesn't know that - I can check
it in that way with a big comment, I guess, but it basically saves a function
call. But that doesn't affect the computation of the custom headers to download...

Comment 8

16 years ago
yes, I want to avoid that function call. you can comment that function call and 
say not implemented or something.

Comment 9

16 years ago
adding adt1.0.0 [adt2] for the performance hit for a very common mail operation.
Keywords: nsbeta1 → adt1.0.0, nsbeta1+
Whiteboard: [adt2]
(Assignee)

Comment 10

16 years ago
Karen, can you verify this fix today on the trunk, for normal imap servers
(e.g., the netscape server) and for webmail servers? We would like to check this
in soon since it's a rather serious regression from a performance point of view.
I'll attach two protocol logs, one before the fix, that shows us downloading all
the headers, and one after the fix, showing us just downloading the headers we
need for our db, and if you can just verify that your logs show the same thing,
that would be great.
(Assignee)

Comment 11

16 years ago
Created attachment 80004 [details]
this shows the old, bad behaviour
(Assignee)

Comment 12

16 years ago
Created attachment 80006 [details]
this shows the correct behaviour after the fix

this shows us just downloading the headers we need instead of all the headers.

Comment 13

16 years ago
Huang - Pls verify that we get a performance win on the trunk, and that this
does not cause any regressions.
(Assignee)

Comment 14

16 years ago
fixed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 15

16 years ago
adt1.0.0-/adt2 RTM. This is good bug to take for RTM. Let's pick it up, after
MachV beta ships.
Keywords: adt1.0.0 → adt1.0.0-
Whiteboard: [adt2] → [adt2 RTM]
(Assignee)

Comment 16

16 years ago
karen, any luck verifying this? this has a very large performance impact on imap
servers, and on users with slow connections.

Comment 17

16 years ago
Helping Karen & David out, trying to qa this on trunk.

I ran into problem with customized headers as I get a error
mesg about FETCH BODY.PEEK[HEADER.FIELDS every time i do a GetMsgs. 
See bug 155496.

Adding dependency.
Depends on: 155496

Comment 18

16 years ago
Using commercial trunk
2002-07-01-08-trunk/ NT 4.0
2002-07-02-08-trunk/ mac 9.2.2
2002-07-02-04-trunk/ linux 2.2

Looking at the protocol logs for am imap mail account.

I have confirmed the headers that we were downloading
are indeed smaller now when I go do a 'getmsg' to receive
a new mesg.

For comparisons sake:

Old way we downloaded all headers:
-INBOX:SendData: 15 UID fetch 257:258 (UID RFC822.SIZE BODY.PEEK[HEADER] FLAGS)
-INBOX:CreateNewLineFromSocket: * 64 FETCH (FLAGS (\Recent) UID 257 RFC822.SIZE 
708 BODY[HEADER] {700}
-INBOX:STREAM:OPEN Size: 708: Begin Message Download Stream
-INBOX:CreateNewLineFromSocket: Return-Path: <gchan6@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: Received: from linzilla.mcom.com 
([10.169.111.205]) by
-INBOX:CreateNewLineFromSocket:           linzilla.mcom.com (Netscape Messaging 
Server 4.15) with ESMTP id
-INBOX:CreateNewLineFromSocket:           GYLLAZ00.C11 for 
<gchan6@linzilla.mcom.com>; Mon, 1 Jul 2002
-INBOX:CreateNewLineFromSocket:           18:25:47 -0700 
-INBOX:CreateNewLineFromSocket: Message-ID: <3D21011C.2090101@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: Date: Mon, 01 Jul 2002 18:25:48 -0700
-INBOX:CreateNewLineFromSocket: From: "g chan6" <gchan6@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; 
en-US; rv:1.0.1) Gecko/20020701 Netscape/7.0
-INBOX:CreateNewLineFromSocket: X-Accept-Language: en-us, en
-INBOX:CreateNewLineFromSocket: MIME-Version: 1.0
-INBOX:CreateNewLineFromSocket: To: g chan6 <gchan6@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: Subject: superboy
-INBOX:CreateNewLineFromSocket: Content-Type: text/plain; charset=us-ascii; 
format=flowed
-INBOX:CreateNewLineFromSocket: Content-Transfer-Encoding: 7bit
-INBOX:CreateNewLineFromSocket: 
-INBOX:CreateNewLineFromSocket: )
-INBOX:STREAM:CLOSE: Normal Message End Download Stream

With the fix, the new way we download headers (notice less lines):


-INBOX:SendData: 14 UID fetch 264 (UID RFC822.SIZE FLAGS BODY.PEEK[HEADER.FIELDS 
(From To Cc Subject Date Message-ID Priority X-Priority References Newsgroups)])
-INBOX:CreateNewLineFromSocket: * 71 FETCH (FLAGS (\Recent) UID 264 RFC822.SIZE 
709 BODY[HEADER.FIELDS (From To Cc Subject Date Message-ID Priority X-Priority 
References Newsgroups)] {194}
-INBOX:STREAM:OPEN Size: 709: Begin Message Download Stream
-INBOX:CreateNewLineFromSocket: Message-ID: <3D21F41A.1040909@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: Date: Tue, 02 Jul 2002 11:42:34 -0700
-INBOX:CreateNewLineFromSocket: From: "g chan6" <gchan6@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: To: "g chan6" <gchan6@linzilla.mcom.com>
-INBOX:CreateNewLineFromSocket: Subject: shaggy
-INBOX:CreateNewLineFromSocket: 
-INBOX:CreateNewLineFromSocket: )
-INBOX:STREAM:CLOSE: Normal Message End Download Stream

Still not sure about custom headers and how it deals with webmail
(as I didn't see the 'peek' lines in protocol log as it relates
to webmail accounts)

Will leave it as Resolved for now and will do more testing
as instructed.
(Assignee)

Comment 19

16 years ago
for web mail, we use a different command to get the envelope information.

Comment 20

16 years ago
so would log for webmail be different between current trunk & branch?
Anything I need to look for?


Comment 21

16 years ago
I am investigating this bug right now.....

Comment 22

16 years ago
Removing dependency bug 155496 since that bug should be different issue.

Verified on all the platforms for Netscape Messaging Server. Results are same as 
David & Gray described on above comments.
Veriifed on Windows 07-09-08-trunk build Messaging Server IMAP
Verified on Linux 07-09-07-trunk build Messaging Server IMAP
Verified on Mac 07-09-08-trunk build Messaging Server IMAP

Verified on all the platforms for Webmail, got the expected results as following 
Webmail IMAP log.
Veriifed on Windows 07-09-08-trunk build Webmail
Verified on Linux 07-09-07-trunk build Webmail
Verified on Mac 07-09-08-trunk build Webmail
No longer depends on: 155496

Comment 23

16 years ago
Created attachment 90674 [details]
Webmail old behaviour

Comment 24

16 years ago
Marking as verified based on the confirmation with developer.
Status: RESOLVED → VERIFIED

Comment 25

16 years ago
Ccing Scott.
Should this bug change keywords from "adt 1.0.0-" to "adt1.0.1"?
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.