Closed Bug 533865 Opened 10 years ago Closed 10 years ago

Doing a quick search in a cross-folder saved search clears cached results

Categories

(Thunderbird :: Folder and Message Lists, defect)

x86
All
defect
Not set

Tracking

(blocking-thunderbird3.0 .1+, thunderbird3.0 .1-fixed)

VERIFIED FIXED
Thunderbird 3.1a1
Tracking Status
blocking-thunderbird3.0 --- .1+
thunderbird3.0 --- .1-fixed

People

(Reporter: Bienvenu, Assigned: Bienvenu)

References

Details

(Keywords: perf)

Attachments

(1 file, 3 obsolete files)

If you do a quick search in a  cross-folder saved search (e.g., a smart mailbox), we remove from the cached results the messages that don't match the quick search. This slows down both the quick search, and returning to the saved search. Patch upcoming.
Attached patch proposed fix (obsolete) — Splinter Review
This patch makes cross-folder saved searches compare the base search criteria for the saved search against the current search session's search criteria. If they are different, then we're doing a quick search, and we should not start with the old saved search results, nor should we update the cached results. This is a big performance win for quick search in saved searches, but it doesn't change the correctness of the saved searches, which are already covered by unit tests...

This is such a big win that I'd like to think about putting this in a 3.0x release.
Attachment #416822 - Flags: superreview?(bugzilla)
Attachment #416822 - Flags: review?(bugzilla)
Attachment #416822 - Flags: approval-thunderbird3.0.1?
blocking-thunderbird3.0: --- → ?
Duplicate of this bug: 494984
nice!
Keywords: perf
OS: Mac OS X → All
Whiteboard: [has patch for review]
blocking-thunderbird3.0: ? → .1+
Attachment #416822 - Flags: superreview?(bugzilla)
Attachment #416822 - Flags: superreview+
Attachment #416822 - Flags: review?(bugzilla)
Attachment #416822 - Flags: review+
Comment on attachment 416822 [details] [diff] [review]
proposed fix


>-    { "Message Filter Service", NS_MSGFILTERSERVICE_CID,
>-      NS_MSGFILTERSERVICE_CONTRACTID,
>-      nsMsgFilterServiceConstructor,

Can you do the same in mailnews/build/nsMailModule.cpp please - it has the same problem.

>+    nsCAutoString  stream;

nit: I know this is just copy/paste, but can you make that just one space please.
Whiteboard: [has patch for review]
Target Milestone: --- → Thunderbird 3.1a1
Duplicate of this bug: 533224
Duplicate of this bug: 525598
this is what I'll check in...I found an issue with the code that converts search terms to strings; it was bailing after the ALL term, which doesn't work for the quick search terms in a smart folder, since quick search includes the all term...
Attachment #416822 - Attachment is obsolete: true
Attachment #416822 - Flags: approval-thunderbird3.0.1?
Attachment #420426 - Flags: approval-thunderbird3.0.1?
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I just backed this out due to unit test failures in test_viewWrapper_virtualFolder.js (which exists under mail/!):

http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1262848429.1262850880.14500.gz#err0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 420426 [details] [diff] [review]
fix addressing comments plus a bug fix

Needs some extra test fixes at least ;-)
Attachment #420426 - Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1-
Attached patch fix regression found by test... (obsolete) — Splinter Review
We were skipping the registering of folder listeners when doing quick search in saved searches...this fixes that, and the well-hidden tests pass.
Attachment #420426 - Attachment is obsolete: true
Attachment #420557 - Flags: superreview?(bugzilla)
Attachment #420557 - Flags: review?(bugzilla)
Whiteboard: [needs review for new patch]
Attachment #420557 - Flags: superreview?(bugzilla)
Attachment #420557 - Flags: review?(bugzilla)
Attachment #420557 - Flags: review-
Comment on attachment 420557 [details] [diff] [review]
fix regression found by test...

This patch is exactly the same as the last one :-(
Whiteboard: [needs review for new patch] → [needs correct patch]
Attached patch correct patchSplinter Review
darnit, this should be the right file...
Attachment #420557 - Attachment is obsolete: true
Attachment #420885 - Flags: superreview?(bugzilla)
Attachment #420885 - Flags: review?(bugzilla)
Attachment #420885 - Flags: superreview?(bugzilla)
Attachment #420885 - Flags: superreview+
Attachment #420885 - Flags: review?(bugzilla)
Attachment #420885 - Flags: review+
Attachment #420885 - Flags: approval-thunderbird3.0.1+
Whiteboard: [needs correct patch] → [ready to land]
fixed on trunk and branch.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [ready to land]
Difficult to verify - but the search was very speedy on a very large smart folder. and none of the duped bugs symptoms where observed. Tested on XP.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.