431758 - places.sqlite should use memory for storing temporary data

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Dietrich Ayala (:dietrich)]

Dave Camp noticed that many small temporary files were being created by SQLite, and correlated it with places.sqlite. For example, he said each time he typed a letter in the location bar, a new file was created.

SQLite stores temporary tables in these files, and temporary tables are created any time you have a sub-select in a query... which Places has a lot of.

We should try using PRAGMA temp_store = MEMORY to see how it affects performance.

Comment 1

[Marco Bonardo [:mak]]

Attachment: patch

it would be really interesting check how much this makes the UI more responsive.

Comment 2

[Dietrich Ayala (:dietrich)]

Comment on attachment 319409 [details] [diff] [review]
patch

let's flip this on for a few Talos runs.

drivers: requesting temporary approval for measuring the performance effects of using memory instead of disk for sqlite temporary storage for places.

Comment 3

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.301; previous revision: 1.300
done
Checking in toolkit/components/places/src/nsNavHistory.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v  <--  nsNavHistory.h
new revision: 1.158; previous revision: 1.157
done

Comment 4

[Dietrich Ayala (:dietrich)]

Initial results from qm-pmac-fast01 show a Ts improvement of about 1%. There are a couple of other check-ins during this time, but this is a likely candidate. I'm keeping this in until a windows talos run finishes, so we can see the memset results.

Comment 5

[Dietrich Ayala (:dietrich)]

Ts suffered on qm-pmac-trunk06 by almost 3%, but that box fluctuates wildly, so we should give it a couple of runs.

Comment 6

[Marco Bonardo [:mak]]

"The rollback journal, master journal, and statement journal files are always written to disk. But the other kinds of temporary files might be stored in memory only and never written to disk."

it would be interesting understanding if this affects VACUUM timings.

However from what i'm reading in groups if the sqlite process has enough ram to store files in memory the difference between MEMORY or FILE could be not noticeable. 

temp_store benefits will however be totally negated if the temporary file is big enough to fill available memory and the OS start swapping to disk.

Also we should take a look at memory usage for those users having a large places.sqlite

Comment 7

[Rick Stockton]

Marco, my places.sqlite is now 4.5MB (on Linux). I've already got this patch in place (running a Tinderbox from over 10 hours after the checkin).

If you care to define a "verification" process for an end-user to follow, I can try it. With some extra work, I can go back and grab a Build from between the landings of 430530 and this one, re-do the procedure and report back.

I'm on Mandriva 2008.1 "Spring", Athlon-64 3500+, 2 Gigs PC2-5300, Seagate 160Mb/8MB. I'll turn off my Compiz-Emerald stuff, run just a "native" KDE desktop to make it a bit more reliable. 

Comment 8

[Marco Bonardo [:mak]]

a verification for the end user is not so easy to define, we probably need some internal PRInterval dump to measure this, and could largely vary based on the ram type, on the size of the db, on the memory free space, and so on...

Comment 9

[Dietrich Ayala (:dietrich)]

(In reply to comment #6)
> However from what i'm reading in groups if the sqlite process has enough ram to
> store files in memory the difference between MEMORY or FILE could be not
> noticeable. 
> 
> temp_store benefits will however be totally negated if the temporary file is
> big enough to fill available memory and the OS start swapping to disk.

Dave:

- how much ram on that machine you saw this file-creation happening?
- and how big is the places.sqlite file?

> Also we should take a look at memory usage for those users having a large
> places.sqlite
> 

The memory usage Talos tests show no change either way from this checkin. This is not surprising given that Talos doesn't test places UI actions such as awesomebar, sidebar, toolbar, or any kind of UI open/close.

Given that there's no reported regression (I've been watching the build forum for comment also) and the quantity of disk i/o involved, I'm going to leave this in for potential help for bug 421482, and mark this fixed.

Comment 10

[Seth Spitzer]

I'm investigating a similar trick, to do PRAGMA temp_store = MEMORY to avoid creating these temp files.  (You guys are always a step ahead!)

Something to note, the TEMP_STORE pragma is per connection, not per database (according to http://marc.info/?l=sqlite-users&m=117250134711685&w=2).  You can observe this also if you ever use the SQLite Manager extension (as it always comes up with the compiled default of 0 (for DEFAULT, which ends up being FILE)

a question for dietrch and sdwilsh:  how many connections do you guys open to the places sqlite database?  If more than one, you might need the PRAGMA temp_store = MEMORY" in more parts of the code than just nsNavHistory::InitDB()

backing up marco from comment #6, see http://www.sqlite.org/tempfiles.html#tempstore

for completeness, see also the conversation between sdwilsh and drhipp (http://www.mail-archive.com/[e-mail]/msg35129.html)

Comment 11

[Shawn Wilsher :sdwilsh]

(In reply to comment #10)
> a question for dietrch and sdwilsh:  how many connections do you guys open to
> the places sqlite database?  If more than one, you might need the PRAGMA
> temp_store = MEMORY" in more parts of the code than just nsNavHistory::InitDB()
We only use one and only one database connection accessible through nsPIPlacesDatabase that is opened with an exclusive lock.

Comment 12

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv