Closed Bug 505309 Opened 13 years ago Closed 13 years ago

os search integration should use nsIMsgDatabase::getFilterEnumerator to avoid creating a lot of js garbage nsMsgHdrs


(Thunderbird :: OS Integration, defect)

Windows Vista
Not set


(Not tracked)



(Reporter: Bienvenu, Assigned: Bienvenu)




(1 file)

+++ This bug was initially created as a clone of Bug #505307 +++

indexer.js creates a hdr enumerator and enumerates over the headers of a db looking for non-indexed message. For a folder that's mostly indexed, this generates a lot of garbage nsIMsgDBHdrs that aren't needed, and causes the cycle collector to block for long periods of time. Using the filter enumerator to find hdrs to index should produce a lot less garbage.
The patch in Bug 499806 has a test case that shows how to do this.
I looked at how searchCommon.js iterates over headers, and the filter enumerator won't help here yet, because it lets you create terms that compare msg hdr string properties, not uint32 properties. I'm not sure winsearch is creating as many garbage headers as gloda, but it's possible the two together are what's causing my issues. It shouldn't be too hard to tweak the filter enumerator code to allow int comparisons, he said hopefully.
Depends on: 513102
taking, since this is a big thorn in my side...
Assignee: nobody → bienvenu
Flags: blocking-thunderbird3+
Attached patch proposed fixSplinter Review
This is working for me, and I don't get the long garbage collection pauses if I make the mistake of leaving my computer idle for 30 seconds or so...But I haven't worked with this code in a while so I may have messed something up.
Attachment #397984 - Flags: review?(sid.bugzilla)
Whiteboard: [has patch for review]
Comment on attachment 397984 [details] [diff] [review]
proposed fix

>diff --git a/mail/components/search/content/searchCommon.js b/mail/components/search/content/searchCommon.js

>+        //  we need to create search terms for messages to index
>+        let searchSession = Cc[";1"]
>+                              .createInstance(Ci.nsIMsgSearchSession);
>+        let searchTerms = Cc[";1"].createInstance(Ci.nsIMutableArray);
>+        searchSession.addScopeTerm(Ci.nsMsgSearchScope.offlineMail, this._currentFolderToIndex);

Hm, given that search integration does index non-offline messages right now, I'm a little worried about this. A patch that fixes bug 467111 (even if only to ignore non-offline messages completely) would be fine with me, though =)
Not sure what you mean - the offlineMail scope just means we don't search online for the headers; it doesn't say anything about whether the message bodies are offline or not. So the code is the same as it was before, in effect, though I could make it ignore non-offline messages like we do for gloda. That would benefit users who have imap folders not configured for offline use...
Comment on attachment 397984 [details] [diff] [review]
proposed fix

(In reply to comment #6)
> Not sure what you mean - the offlineMail scope just means we don't search
> online for the headers;

Sorry, I misunderstood what offlineMail meant. The patch looks great.
Attachment #397984 - Flags: review?(sid.bugzilla) → review+
Whiteboard: [has patch for review] → [needs landing]
fix checked in.
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.