677805 - Bump page size and cache size for Gloda

Status: RESOLVED FIXED

(MailNews Core :: Database, enhancement)

Description

[Jonathan Protzenko [:protz]]

Attachment: Patch v1

As discussed, here's a patch that will bump the schema number, nuke the database, and create a fresh one with the new page size and cache size. These are computed exactly like places for Firefox.

We might want a slightly bigger cache, but on my machine, that makes it 64M which seems sufficient enough.

I used to have NS_ERROR_BUSY while trying to do the synchronous mozIStorageConnection::Close in _migrate from datastore.js, but these seem to have gone, so it might just be my getting confused in my tests.

The patch is not slated for landing now, but rather will sit at the bottom of my queue while others patches pile up, and I can batch them all together for the big schema change.

Comment 1

[Wayne Mery (:wsmwk)]

> on my machine, that makes it 64M 
what was your cache prior to patch, and with how much memory installed?

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

64 MiB is a lot of space to burn on cache, even on a 4gig machine.  Every MiB we claim is one less that the OS can use, and the OS might do a better job of using it than us (and it can still benefit us.) I believe Places is aggressive with cache because their database schema was not particularly efficient and they did a lot of synchronous queries, so every little bit helps.  In contrast, gloda is purely async and non-fulltext operations are unlikely to overflow the cache.

Which is to say, I would feel better if:
1) We have a good performance reason for how we're sizing our cache.
2) We claw back some of the memory from places when doing this.  Specifically, I am assuming that Places may already be claiming 32 MiB or 64 MiB in Thunderbird, most of which is massive waste, since Thunderbird is not a web browser and is going to have much less data or need for the data.

As previously noted, the main performance guideline I used when picking the cache size was trying to avoid exceeding our cache size between commits when modifying the database, because as soon as we have more dirty pages than cache, we need to potentially perform a lot more random disk I/O and maybe some fsyncs.  The increase in page size may exceed the cache size more quickly, but the I/O costs in terms of seeks may also be much lower since we will ideally have less random I/O.

This may, in large part, depend on the other potential gloda db operations that happen.  Specifically, with the column change stuff, and if we get some stop-word support in the database, we might see massive reductions in the amount of disk space required by gloda and drastically reduce the cache overflow potential.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 551964 [details] [diff] [review]
Patch v1

So, you also missed the other spot we set the size (where we open the database and consider invoking migration).

Thanks for removing the TRUE ASYNC stuff, it should indeed be moot now.  As far as I can immediately see, your close explosions should indeed have been transient weirdness.  Having said that, you might want to try increasing the logging statements inside the catch handlers from debug to warn so that it triggers test failures in case we are doing something dumb.  I'm not sure we'd want to ship that, although since we should only teardown during shutdown, it would ideally not complicate debugging other issues.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

offline discussion summary:

Let's use strict a strict lower bound of 8 MiB (our current cache size), strict upper bound of 64 MiB, and use 1% of system memory to pick the cache size.  This leaves us starting to use more memory on systems with 800 MiB, and capping out on systems with 6400 MiB.

It is important to note that SQLite does not allocate all the cache at once ahead of time, but only allocates its cache memory on demand.

On a separate bug, protz will propose we change the Thunderbird preference for the Places cache size.  Places currently sets the max cache size to 6% of system memory, which is sorta ridiculous, but mitigated by the places database being pretty small on Thunderbird.

Comment 5

[Jonathan Protzenko [:protz]]

https://bugzilla.mozilla.org/show_bug.cgi?id=677954

Comment 6

[Jonathan Protzenko [:protz]]

Attachment: Patch v2

Implements the strategy we discussed, adds a gloda-specific pref to control the cache size, and fixes my completely messing up between Math.min and Math.max.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 552138 [details] [diff] [review]
Patch v2

This looks great.  Two things:

1) Let's change the preference to be in tenths of a percentage, so the pref would be "mailnews.database.global.datastore.cache_to_memory_percentage_tenths", the default would be 10, and we divide by 1000 instead of 100  My rationale is that having a preference whose expressive range is basically 0 or 1 is not super useful if it's supposed to be tunable (although being able to disable our growth is useful.)

Let's change the preference comment to be something like "rate of growth of the gloda cache, whose maximum value is 8 MiB and max is 64 MiB.  See more: https://developer.mozilla.org/en/Thunderbird/gloda#Cache_Size"

(I just wrote up the stuff at the target of the link.  Feel free to augment, etc.)

2) Please post to tb-planning about this change so everyone knows what's happening and in case there are useful suggestions about different defaults or bounds.  A link to https://developer.mozilla.org/en/Thunderbird/gloda#SQLite_Page.2fCache_Sizes and this bug plus a quick overview should be sufficient.

Comment 8

[Jim Porter (:squib)]

(In reply to Andrew Sutherland (:asuth) from comment #7)
> 1) Let's change the preference to be in tenths of a percentage, so the pref
> would be
> "mailnews.database.global.datastore.cache_to_memory_percentage_tenths"

"permil"? <http://en.wikipedia.org/wiki/Permil>

Comment 9

[Jonathan Protzenko [:protz]]

Attachment: Patch v3

With review comments fixed. I went for permillage.

Comment 10

[Jonathan Protzenko [:protz]]

Andrew, I'm still getting NS_ERROR_STORAGE_BUSY when running .remove() on the nsIFile for global-message-db.sqlite, right after the call to mozIStorageConnection.close(). Any ideas?

Comment 11

[Jonathan Protzenko [:protz]]

Attachment: Patch v4

With the extra finalize, which makes the upgrade go smoothly.

Comment 12

[Jonathan Protzenko [:protz]]

http://hg.mozilla.org/comm-central/rev/f2e38e7e432e