616019 - 10mb places.sqlite database due to aggressive file growth.

Status: RESOLVED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Doug Turner (:dougt)]

On desktop, we worry alot about fragmention.  On mobile, we care a bit more about footprint.  Our sqlite files tend to frag a bunch on desktop, so we aggresively bump their side.  For example, the places file grows in increments of 10mbs.

http://mxr.mozilla.org/mozilla-central/search?string=SetGrowthIncrement

On mobile, I am not sure that this value is correct.  For a very small profile, I already have a 10mb file I need to read in (or parts of it).

If I compare with and without the call to SetGrowthIncrement:

1) clean profile
2) launch go to people.mozilla.org/~dougt
3) exit
4) launch
5) record data

with call:
places.sqlite file size: 10mb
total memory used:  84mb res

without call:
places.sqlite file size: 992k
total memory used:  83mb res

I am not sure what the ideal value for SetGrowthIncrement, but I think on mobile we can do better assuming that there isn't much of a startup cost.

Taras, do you have any input on changing these values for mobile?

Comment 1

[(dormant account)]

See bug 597215

Comment 2

[Doug Turner (:dougt)]

yeah, that isn't the best way to fix that.  what about meego, maemo, iphone, palm, desktop, blah.

would anyone mind if we added prefs for this or a new configure options similar to MOZ_GFX_OPTIMIZE_MOBILE but for non-gfx code?

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> would anyone mind if we added prefs for this or a new configure options similar
> to MOZ_GFX_OPTIMIZE_MOBILE but for non-gfx code?
Prefs can be difficult; we'd have to be careful when we lookup the value since database connections can be opened off of the main thread.

Comment 4

[(dormant account)]

I think a pref would be best.

Comment 5

[Doug Turner (:dougt)]

Attachment: patch v.1

i'll patch mobile-browser separately.

Comment 6

[Tuukka Tolvanen (sp3000)]

did you consider a numeric pref, say, in case I'd prefer a 1MB increment?

Comment 7

[Tuukka Tolvanen (sp3000)]

...the wording then being something like, toolkit.storage.maxGrowthIncrement, to cap the callers' suggestions

Comment 8

[Tuukka Tolvanen (sp3000)]

Attachment: dueling patches (central)

something like so

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 495011 [details] [diff] [review]
dueling patches (central)

i think 10 was chosen to avoid fragmentation.  not sure of the value of having that value dynamic.  But, either approach will work for me.  Taras?

Comment 10

[(dormant account)]

(In reply to comment #9)
> Comment on attachment 495011 [details] [diff] [review]
> dueling patches (central)
> 
> i think 10 was chosen to avoid fragmentation.  not sure of the value of having
> that value dynamic.  But, either approach will work for me.  Taras?

I like the dynamic approach, 1mb will still reduce fragmentation. 10 was chosen based on firefox places io pattern.

My only concern is that for best perf this number should be tweaked for each individual db, but short of adding a pref for every growth-increment user,  i think this is a good tradeoff.

Comment 11

[Doug Turner (:dougt)]

Comment on attachment 495011 [details] [diff] [review]
dueling patches (central)

PR_INT32_MAX shouldn't be the default fallback. :)

Comment 12

[Shawn Wilsher :sdwilsh]

Both of these patches suffer from the same threadsafety issues.  This should be built on top of the work in bug 580790.

Comment 13

[Doug Turner (:dougt)]

lets add an #ifdef in the short term so that maemo doesn't get killed by this bug.

Comment 14

[Shawn Wilsher :sdwilsh]

How are we changing the ifdef exactly?

Comment 15

[Doug Turner (:dougt)]

Attachment: work around until we fix the other bugs.

Comment 16

[Shawn Wilsher :sdwilsh]

Comment on attachment 495122 [details] [diff] [review]
work around until we fix the other bugs.

r=sdwilsh

Comment 17

[Shawn Wilsher :sdwilsh]

Comment on attachment 494757 [details] [diff] [review]
patch v.1

Per comment 12

Comment 18

[Shawn Wilsher :sdwilsh]

Comment on attachment 495011 [details] [diff] [review]
dueling patches (central)

Per comment 12

Comment 19

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/1ad47fdb9195


if someone cares, they can file a new bug about making this a preference.  or maybe we will the next time we bring up a new platform.