108077 - Optimization in nsMsgCopyService::CopyMessages()

Status: RESOLVED INVALID

(MailNews Core :: Backend, defect)

Description

[Håkan Waara]

In CopyMessages, there's a for-loop that:

* iterates through all elements in a nsISupportsArray argument
* splits it up into nsISupports objects
* QIs every object into a nsIMsgDBHdr*
* pass every object to nsCopySource::AddMessage()

Then AddMessage() does the exact opposite. It QIs back into a nsISupports* and 
adds it into another nsISupportsArray.  My optimization is to assign the 
nsISupportsArray argument directly into the last nsISupportsArray, to skip all 
this stuff.

Comment 1

[Håkan Waara]

Attachment: Fix -- Add nsCopySource::AddMessages()

Comment 2

[Navin Gupta]

Comment on attachment 56145 [details] [diff] [review]
Fix -- Add nsCopySource::AddMessages()

r=naving, Could you 
try deleting multiple 
local messages and see 
everything works ok. 
Also look out for other
code where we go through
copyService. I know deleting local messages
does.

Comment 3

[David :Bienvenu]

this will have no appreciable performance impact (sorry, a couple QI's compared
to the time it takes to move/copy an imap or local message is just not going to
matter) and I don't really have the time to make sure you're doing the right thing.

Comment 4

[Håkan Waara]

Very nice

Comment 5

[Navin Gupta]

I totally agree w/ bienvenu - there will be no appreciable performance gain. 

Comment 6

[(not reading, please use [e-mail] instead)]

saving a few QIs isn't going to amount to much in this scenario.

hwaara, convince yourself that it's not.

I haven't looked carefully, but I'm not sure that patch is correct either.

Comment 7

[Håkan Waara]

I think it's good cleanup... but as you say perhaps it doesn't make much
different in footprint/performance world -- just good old code cleanup.

I'm fine with landing it post-lockup.

Comment 8

[Håkan Waara]

Due to internal problems with my computer, I don't know when I'll get back to
this bug. Feel free to attach a patch -- I'll review it. 

Changing target milestone for now.

Comment 9

[Håkan Waara]

This bug expired a long, long time ago.

Comment 10

[MDN Team (:mdn-dev)]

Commit pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/93d35d34746e88fdb13141d01f4dfc22229d4825
bug 108077 - Updates to demo detail control buttons and listing title colors