276716 - Virtual Folder/Saved Search Folder does not cache the message list for subsequent accesses to the folder

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Colin]

When I enter a Virtual Folder/Saved Search Folder for the first time it 
populates the the message list with all of the messages that match, and this 
works fine and takes about 5 minutes. The issue comes when you exit the 
folder, Thunderbird does not seem to cache the results so when you re-enter 
the folder it is empty and Thuderbird has to re-populate the message list 
which take another 5 minutes and is far too slow for day to day usage.

Comment 1

[sammo]

5 minutes is a long time. My virtual folder searches usually take around 30
seconds at most, which is still too long.

I agree that it would be great if virtual folder / saved search folders were
cached ..

Comment 2

[David :Bienvenu]

Attachment: work in progress

work in progress - caching for single folder saved searches is basically working and caching for cross-folder saved searches is coming along.

Comment 3

[David :Bienvenu]

Attachment: more work in progress

getting fairly close now - this gets cross-folder virtual folder hit caching working, and updating hdr caches when the underlying folders change.

Comment 4

[David :Bienvenu]

Attachment: work in progress

this patch seems to work pretty well, and should be more efficient about checking if we have a cached hit or not. I've also added some assertions to tell if we're not sorting the cached hits correctly, because we do count on that.

Comment 5

[David :Bienvenu]

Attachment: proposed fix

I'd like to get this some baking on the trunk. This implements cache tables in each db that is a search scope of a virtual folder - a table with the search hits for that folder and virtual folder combination. The tables will be very compact since they're just the row ids of the corresponding header. When the folder is compacted, the tables will get blown away and the search will have to be re-done - this avoids the problem of having stale hits when msg keys change because a folder is compacted.

This also fixes a bug whereby newly created cross-folder virtual folders weren't having their counts updated until you rebooted.

Comment 6

[Scott MacGregor]

Comment on attachment 230040 [details] [diff] [review]
proposed fix

I'm assumingnsMsgFlatFolderDataSource::OnItemRemoved isn't part of the patch, do you want to check that in? It might conflict with the other flat datasource changes we landed yesterday

1) Typo in nsMsgQuickSearchDBView::OnNewSearch
+  // from the db, and set a flag s saying that we're using cached values.

2) In nsMsgXFVirtualFolderDBView::UpdateCacheAndViewForPrevSearchedFolders
nsMsgKey key gets declared inside the for loop. Should we declare that outside the for loop or will the compiler optimize that out for us? I saw a couple of for loops with a similar pattern.

3) I didn't understand the change in nsMsgLineBuffer.cpp, was that for this patch? 

4) brackets around the if clause in nsMsgDatabase::GetSearchResultsTable can probably be removed. 

This is going to be a great feature.

Comment 7

[David :Bienvenu]

thx, Scott. the nsMsgLineBuffer.cpp change is unrelated, as is the folder data source change.

nsMsgKey is just a PRUint32, so there's no constructor, just space allocated on the stack.  I don't think any code is required for that (I would hope that would be optimized to be in a register anyway)

Comment 8

[David :Bienvenu]

Attachment: patch for checkin

this is the patch I intend to check in when the tree greens up a bit. It fixes two of Scott's nits, and an off by one error in the binary search of cached search results, iirc.

Comment 9

[David :Bienvenu]

fixed on trunk - I'll get ready to land on branch.

Comment 10

[David :Bienvenu]

fixed on 2.0 branch as well. 

Comment 11

[Wolfgang Rosenauer [:wolfiR]]

Please change the nsMsgAccountManager::AddVFListenersForVF() to return something because it's of type nsresult and you don't return anything. Thanks.