Closed
Bug 505309
Opened 15 years ago
Closed 15 years ago
os search integration should use nsIMsgDatabase::getFilterEnumerator to avoid creating a lot of js garbage nsMsgHdrs
Categories
(Thunderbird :: OS Integration, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Bienvenu, Assigned: Bienvenu)
References
Details
Attachments
(1 file)
(deleted),
patch
|
rain1
:
review+
|
Details | Diff | Splinter Review |
+++ 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.
Assignee | ||
Comment 1•15 years ago
|
||
The patch in Bug 499806 has a test case that shows how to do this.
Assignee | ||
Comment 2•15 years ago
|
||
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.
Assignee | ||
Comment 3•15 years ago
|
||
taking, since this is a big thorn in my side...
Assignee: nobody → bienvenu
Flags: blocking-thunderbird3+
Assignee | ||
Comment 4•15 years ago
|
||
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)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [has patch for review]
Comment 5•15 years ago
|
||
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["@mozilla.org/messenger/searchSession;1"]
>+ .createInstance(Ci.nsIMsgSearchSession);
>+ let searchTerms = Cc["@mozilla.org/array;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 =)
Assignee | ||
Comment 6•15 years ago
|
||
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 7•15 years ago
|
||
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+
Updated•15 years ago
|
Whiteboard: [has patch for review] → [needs landing]
Assignee | ||
Comment 8•15 years ago
|
||
fix checked in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in
before you can comment on or make changes to this bug.
Description
•