114700 - Remove PR_Sleep(PR_MicrosecondsToInterval(500UL))in nsMailboxProtocol.cpp

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Navin Gupta]

I have just tried commenting it in my tree and batch deleting/copying local mail
 works ok. Looks like the necko guys have fixed this problem that was
there.

Is there any special case you want me to check?

Comment 1

[Navin Gupta]

This sleep call is on nsMailboxProtocol.cpp:325

Comment 2

[David :Bienvenu]

oh boy, tha's a can of worms you want to open. You need to try copying large
numbers of different sized messages between folders, with various sorts, etc. I
can try it if you want.

Comment 3

[Navin Gupta]

I have just tried copying message of size in range 1KB < size <= 2MB and it 
works ok. I don't know how sorting would play a role here. 

Comment 4

[David :Bienvenu]

I assume you mean multiple messages, not single messages of sizes varying from
1K to 1MB (the code in question is only executed when you copy/move multiple
messages). The problem only occured when you copied multiple messages, a large
number of messages, of varying sizes. I tried reproducing the problem, and
couldn't, after commenting out that line, so if you want to live dangerously,
r=bienvenu

Comment 5

[Navin Gupta]

ya, i did multiple messages. i believe this will speed up batch deleting a lot. 
i will try some more cases. I will back it out if it doesn't give us much gains.
 

Comment 6

[David :Bienvenu]

We're talking about a 1/2 millisecond (less than 1/thousandth of a second) gain
per message, so we know pretty much what the improvement will be. It's great if
we can remove this since it was an ugly hack anyway. Your previous fix for
multiple local message move/copy might have also fixed the problem this silly
hack was put in for in the first place.

Comment 7

[Navin Gupta]

Attachment: proposed fix

tested varying size of messages in the batch deletes. worked ok. I am hoping
for some indirect gains.

Comment 8

[scottputterman]

Is this worth taking?  It sounds like the performance gain isn't going to be
noticeable.  Did we determine that it's not risky to take this out?

Comment 9

[David :Bienvenu]

I think we should take it - I thought we'd checked this in a long time ago :-).

Comment 10

[Navin Gupta]

fixed

Comment 11

[[e-mail] (gone - use [e-mail] instead)]

Performance results for local folder batch delete/copy operations:

2002-01-04 Win32 build:

17.02 seconds to Delete 2001 msgs
24.35 seconds to Copy 2001 msgs 

2002-01-07 Win32 build:

15.64 seconds to Delete 2001 msgs
23.78 seconds to Copy 2001 msgs.

A side note, between the 4th and the 7th build, something broke our 'Copying xxx
of xxx messages to Sent' and 'Deleting xxx of xxx messages' notifications. 
Anybody know if that's filed already or not?

Verified FIXED with the build mentioned above.