Mass delete of messages resulted in massive memory usage

RESOLVED FIXED in Thunderbird 3.0rc1

Status

defect
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: allan, Assigned: Bienvenu)

Tracking

Thunderbird 3.0rc1
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact])

Attachments

(1 attachment)

1. clicked on an IMAP folder
2. searched by subject
3. selected all messages that matched (16000+)
4. clicked the delete button
Result: tb3.0b4 started eating memory like there was no tomorrow. Around 100MB/sec or so of virtual memory, so I killed it.

I submitted an Apple bug report (or whatever the correct name is :) ), but I cannot remember if/how I can link that in here.
probably a dup of bug 519226
Easily reproducible btw, I just tried again and hit 3GB of virtual mem in an instant, before I nuked the process.
OK, I've figured this out. I have a simple fix for tb 3.0, but a better fix will have to wait for 3.1.
Assignee: nobody → bienvenu
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: blocking-thunderbird3+
Posted patch proposed fixSplinter Review
We were creating messageIds strings for all 4000 messages for each undo object, but with a single message key. Needless to say, memory usage got crazy. We really should be trying to create a single undo object for the batch delete, but it's a bit scary (actually, several undo objects because there are several sub-operations in the undo queue for a single delete).
Attachment #410691 - Flags: superreview?(bugzilla)
Attachment #410691 - Flags: review?(bugzilla)
Target Milestone: --- → Thunderbird 3.0rc1
Attachment #410691 - Flags: superreview?(bugzilla)
Attachment #410691 - Flags: superreview+
Attachment #410691 - Flags: review?(bugzilla)
Attachment #410691 - Flags: review+
Comment on attachment 410691 [details] [diff] [review]
proposed fix

>-  rv = GetChildWithURI(uri, PR_FALSE/*deep*/, PR_FALSE /*case Insensitive*/, getter_AddRefs(msgFolder));
>+  PRBool caseInsensitive = mIsServer && name.EqualsLiteral("INBOX");
>+  rv = GetChildWithURI(uri, PR_FALSE/*deep*/, caseInsensitive /*case Insensitive*/, getter_AddRefs(msgFolder));

This looks like part of a different patch and shouldn't be here?
 
>+      nsCString messageId;

I think messageIds is now obsolete - so that should be removed.

r+sr=Standard8 with those fixed.
Whiteboard: [no l10n impact][ready to land]
OS: Mac OS X → All
Hardware: x86 → All
fixed on trunk and branch with review comments addressed.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact][ready to land] → [no l10n impact]
You need to log in before you can comment on or make changes to this bug.