We had to disable it for Bug 341137, but it needs to be re-enabled, and preferably done upstream or in mozStorage code (not sqlite).

Vlad, can you triage this? I don't know enough.

So this was done originally to get back some startup performance; do we still have a startup perf issue?

from brett's original work (bug #331158), he added this so that url bar autocomplete would not be slow the first time. I don't know if this blocks 1.9, but we care about it for firefox 3. > do we still have a startup perf issue? There might still be some "first time autocomplete performance issues", but I'll have to confirm that.

Places has a 2% Ts penalty as of the end of June: https://bugzilla.mozilla.org/show_bug.cgi?id=374532#c7

Another issue to consider here is do we really want to patch our own sqlite? It makes upgrading *a lot* harder to do (which is why it was disabled when we upgraded initially).

to solve the "first time autocomplete performance issue", without patching sqlite and making upgrade harder, what about doing an autocomplete query that would (in effect) pre-flight the db cache? in other words, get the "first time autocomplete performace issue" out of the way by doing an autocomplete (in delayed startup, so we don't affect Ts?) so that the first time a user does one, we've payed the price. thoughts?

(In reply to comment #6) > to solve the "first time autocomplete performance issue", without patching > sqlite and making upgrade harder, what about doing an autocomplete query that > would (in effect) pre-flight the db cache? > > in other words, get the "first time autocomplete performace issue" out of the > way by doing an autocomplete (in delayed startup, so we don't affect Ts?) so > that the first time a user does one, we've payed the price. This seems worth trying.

Not clear what the gain is here. If there's a Ts win, it would qualify for wanted-1.9.

(In reply to comment #6) > ... > thoughts? This isn't nearly enough. The problem is that sqlite brings pages in on-demand and in many cases, in random order. This hits *very* hard on disk seeks. Darin had a loptop that was particularly bad. Firefox would start up quickly, but the first time he pressed a key, it took 14 *seconds* for the results to come back. The preload patch had almost no impact on startup time, and made this result come back almost instantly. You will still need to pay that 14 seconds, even if you don't do it on startup, as long as you bring in the pages by doing queries. Plus, you don't really know which queries to do to bring in the database, since most queries won't hit most of the pages. I spent many weeks on this. I do not think there is another way.

(In reply to comment #9) > You will still need to pay that 14 seconds, even if you don't do it on startup, > as long as you bring in the pages by doing queries. Plus, you don't really know > which queries to do to bring in the database, since most queries won't hit most > of the pages. Would it be possible to construct a specific query that retrieves all relevant data from the history table and execute that query in a background thread on delayed startup so that all relevant pages get loaded into the cache within 14 seconds without delaying startup or hacking sqlite?

That will still give you 14 seconds of solid disk activity which is not OK, even on a background thread.

Plus you still wouldn't get autocomplete for that time, and people want to navigate after starting their browser.

(In reply to comment #10) > Would it be possible to construct a specific query that retrieves all relevant > data from the history table and execute that query in a background thread on > delayed startup so that all relevant pages get loaded into the cache within 14 > seconds without delaying startup or hacking sqlite? Also, cache's are thread specific, so if you run it on a background thread, it's completely useless. Brett - is there any reason this change wasn't pushed back up to sqlite? The reason why this wasn't kept when we upgraded was because the patch didn't apply cleanly, and thunder and I didn't understand what was going on well enough to try and manually patch it. As it stands, I'm rather against patching sqlite in only our tree now, but if we can preload without touching sqlite3.c, I'm all for it.

(In reply to comment #9) > > I spent many weeks on this. I do not think there is another way. Hey Brett, thanks for the background information. It's really helpful.

I brought this up with Richard Hipp, the sqlite author, but it didn't really go anywhere (not that he was necessarily against it). Maybe you guys should submit it to him. Assuming he doesn't take it, why wouldn't you patch it? It takes < 30 minutes to fix the patch to match the renamed functions and moved code that happened in the last year.

This is an updated patch for sqlite I put together last night after the discussion to see if it was as easy as I said. It took me about 40 minutes because I found a bug in the old version! When I loaded a page, the old version would always load page 1, whereas it was supposed to load all the pages. The old patch sped up the system just by priming the OS cache, not sqlite's. This patch is for sqlite 3.4.2 because that's what I checked out, it looks like Mozilla has 3.4.1. This isn't a patch for Mozilla, as I don't have a very recent checkout, I did this on a plain sqlite checkout. Maybe somebody can make a patch for Mozilla that checks this in and applies it.

(In reply to comment #15) > I brought this up with Richard Hipp, the sqlite author, but it didn't really go > anywhere (not that he was necessarily against it). Maybe you guys should submit > it to him. > > Assuming he doesn't take it, why wouldn't you patch it? Given that there don't seem to be better options, this indeed seems like something we should patch locally and try again to push up.

Actually, since my old patch was basically bogus and only primed the OS cache. I wonder if this would be sufficient? It would not require modifications to sqlite. I think there will have to be some benchmarks done to decide what to do here.

here's brett's preload change, patched into the moz amalgamated sqlite source. i tested this patch on a profile w/ a large places.sqlite (50+mb), and startup hangs for about 5 seconds while preloading.

Re-requesting blocking. We need one of the two things decided here: 1) enable preloading (which appears to have a Ts perf impact) 2) remove preloading from our API

We should remove preloading; there's no need to patch our sqlite any more, and the significant Ts impact makes this not worth it. We should file another bug to resolve the first-time autocomplete hit, but that's a separate thing (in any case, such a thing should be done in a timer, and not at app startup).

Why I we removing this capability altogether instead of just disabling it for Places? Perhaps other consumers, including extensions, could still find it useful.

Because I'm not inclined to patch sqlite just for addons.

Checking in toolkit/components/satchel/src/nsStorageFormHistory.cpp; /cvsroot/mozilla/toolkit/components/satchel/src/nsStorageFormHistory.cpp,v <-- nsStorageFormHistory.cpp new revision: 1.20; previous revision: 1.19 done Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.214; previous revision: 1.213 done Checking in db/sqlite3/README.MOZILLA; /cvsroot/mozilla/db/sqlite3/README.MOZILLA,v <-- README.MOZILLA new revision: 1.16; previous revision: 1.15 done Removing db/sqlite3/preload-cache.patch; /cvsroot/mozilla/db/sqlite3/preload-cache.patch,v <-- preload-cache.patch new revision: delete; previous revision: 1.2 done Checking in storage/public/mozIStorageConnection.idl; /cvsroot/mozilla/storage/public/mozIStorageConnection.idl,v <-- mozIStorageConnection.idl new revision: 1.14; previous revision: 1.13 done Checking in storage/src/mozStorageConnection.cpp; /cvsroot/mozilla/storage/src/mozStorageConnection.cpp,v <-- mozStorageConnection.cpp new revision: 1.30; previous revision: 1.29 done

Also, it's questionable at best whether this really helps in today's sqlite world -- it was originally done to fix a specific performance issue that's no longer present to anywhere the same degree.

Why is this marked as requiring a doc update? This appears to be an internal functionality change rather than something that impacts docs. What am I missing?

Never mind... didn't scroll down quite far enough. I've marked the preload() function as obsolete and no longer available in Firefox 3 in the documentation for mozIStorageConnection.