Open
Bug 664796
Opened 14 years ago
Updated 2 years ago
Avoid opening databases when getting new messages
Categories
(MailNews Core :: Networking: NNTP, enhancement)
MailNews Core
Networking: NNTP
Tracking
(Not tracked)
NEW
People
(Reporter: jcranmer, Unassigned)
Details
(Keywords: perf)
Attachments
(1 file)
7.12 KB,
application/octet-stream
|
Details |
Right now, getting new messages for a group requires us to open up the database. We should avoid doing so until we're absolutely sure that the database needs to be opened--i.e., the readsets confirm that we have a new message.
Reporter | ||
Comment 1•14 years ago
|
||
Some notes:
The first part the database is opened is probably nsNNTPNewsgroupList::Init; it's not needed for sure until GetRangeOfArtsToDownload. nsNewsFolder::UpdateSummaryFromNNTPInfo may indirectly cause it to be opened, but that should only happen if there are actually new messages.
The last part of the puzzle is making sure nsNNTPNewsgroupList::GetRangeOfArtsToDownload doesn't open up the database if it finds it shouldn't have to, i.e., m_firstArticle would equal m_lastArticle.
It shouldn't be too hard to write a test to make sure that the database isn't opened when we don't need to (nsIDbChangeListener::onEvent with the event being DBOpened).
Comment 2•13 years ago
|
||
I took these while working with some gurus in the #maildev IRC channel. The files were taken with a loop during most of the time that my CPU was being throttled by Thunderbird. It took a few seconds to stop the loop once utilization calmed down resulting in the last few files. The loop follows:
while [ 1 ]; do gstack 7542 > /tmp/tb/7542.`date +%s`; sleep 1; done
Comment 3•13 years ago
|
||
Looking through the gstack output seven (of twenty-five) appear to come down through nsNNTPNewsgroupList::Initialize and eleven of them come through nsNNTPNewsgroupList::CleanUp though how helpful this is I am not sure. Focusing on these (or close) functions would seem to be where an optimization could be added (if CleanUp could somehow realize nothing changed, then do not write anything assuming it is doing so currently). Anyway, just noticing...
Reporter | ||
Comment 4•13 years ago
|
||
::Initialize is opening up the database. Interestingly enough, ::CleanUp is causing the database to be indirectly reopened:
#15 0x00007f9f3a204b79 in nsMsgDBService::OpenFolderDB(nsIMsgFolder*, int, nsIMsgDatabase**) () from /home/aburgemeister/code/thunderbird/comm-release/mozilla/dist/bin/libxul.so
#16 0x00007f9f3a2a9d57 in nsMsgNewsFolder::GetDatabase() () from /home/aburgemeister/code/thunderbird/comm-release/mozilla/dist/bin/libxul.so
#17 0x00007f9f3a2da333 in nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*) () from /home/aburgemeister/code/thunderbird/comm-release/mozilla/dist/bin/libxul.so
#18 0x00007f9f3a2a8b8f in nsMsgNewsFolder::NotifyFinishedDownloadinghdrs() () from /home/aburgemeister/code/thunderbird/comm-release/mozilla/dist/bin/libxul.so
#19 0x00007f9f3a29e919 in nsNNTPNewsgroupList::CleanUp() () from /home/aburgemeister/code/thunderbird/comm-release/mozilla/dist/bin/libxul.so
Judging from code, it looks like CallFilterPlugins grabs its database before it determines it needs to use it. Moving that block of code after the point at which it determines that the spam filters need to be run (which shouldn't be necessary for news barring uncommon circumstances) should cut out some of the blocks.
Certainly, we can move the db initialization code from ::Initialize to ::GetRangeOfArtsToDownload, where it can be done solely on the condition that the number of articles to download is definitely non-zero. That does seem to be much harder though.
Comment 5•13 years ago
|
||
If what I'm proposing is against any rules or anything, please let me know.
I'd like this to get fixed so that Thunderbird is not wasting my time as often as it does. It sounds like this is something that could help the product as a whole through some rework. I am not talented enough to fix this, but if somebody could do it for me in a way that Mozilla will accept (not looking for a one-off just for me that I'll need to re-implement every time Thunderbird is updated) I'd be willing to compensate for time. If there are any good "bounty" sites for this, please let me know. Otherwise, the first person to get patch file(s) to me for this which are acceptable to the project maintainers will get $50 on release of the fixes with TB (too low? justify it and we'll talk). In doing this of course you'll need to follow the current TB license.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•