682676 - urlbar performance regression with SQLite 3.7.7.1

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Stefan]

User Agent:  

Steps to reproduce:

0. Have a profile with 40 M data (dating back to march).
1. Enter urlbar.
2. enter some "trigraphs" like "goo".


Actual results:

After a few 100 ms one entry appears (google) some 100 ms later six visible entries of a longer list are shown.


Expected results:

Show the list immediately (whithout noticeable delay) in one.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

https://addons.mozilla.org/en-US/firefox/addon/places-maintenance/ reports a sane Places Database File?

Comment 2

[Stefan]

(In reply to XtC4UaLL [:xtc4uall] from comment #1)
> https://addons.mozilla.org/en-US/firefox/addon/places-maintenance/ reports a
> sane Places Database File?

I have trouble getting this plugin installed. Installing from downloaded xpi fails without any information. Downloading/install hangs with a "veryfiyng" button.

Comment 3

[Stefan]

I have bisected the issue:

hg bisect -b
The first bad revision is:
changeset:   75951:2fd69e4236ea
user:        Marco Bonardo <[e-mail]>
date:        Fri Aug 26 13:38:28 2011 +0200
summary:     Bug 666420 - Upgrade to SQLite 3.7.7.1 - SQLite changes.

Comment 4

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to Stefan from comment #2)
Does running Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");PlacesDBUtils.checkAndFixDatabase(); in the Error Console work?

Comment 5

[Stefan]

(In reply to XtC4UaLL [:xtc4uall] from comment #4)
> (In reply to Stefan from comment #2)
> Does running
> Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");
> PlacesDBUtils.checkAndFixDatabase(); in the Error Console work?

No.

[15:04:00.351] Error: Permission denied for <moz-safe-about:blank> to get property XPCComponents.utils

Comment 6

[Stefan]

(In reply to XtC4UaLL [:xtc4uall] from comment #1)
> https://addons.mozilla.org/en-US/firefox/addon/places-maintenance/ reports a
> sane Places Database File?

I have increased maxVersion to 9. The plugin reports:

> Integrity check
+ The database is sane

Comment 7

[(mostly gone) XtC4UaLL [:xtc4uall]]

Ok. CCing Marco.

Comment 8

[Stefan]

SQLite 3.7.6 ist the first which shows the behavior.

Comment 9

[Marco Bonardo [:mak]]

interesting, my db is clearly much larger than yours (about three times) and I've not noticed any kind of slowdown in the location bar. I suppose I may need to get a compressed copy of your database by some private way (either mail or a secure link sent by mail) to try running the ac queries on it.

I guess if you maybe didn't have a recent analyze, did you notice if the issue got better after a day of normal use?

Comment 10

[Matthias Versen [:Matti]]

I can confirm a massive performance regression with Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110828 Firefox/9.0a1 SeaMonkey/2.6a1
After typing i get the results in SM after 2s and not nearly instant.

Comment 11

[Marco Bonardo [:mak]]

also, did you change any setting regarding history expiration? Any other special setting in your setup we should be aware of?

Comment 12

[Marco Bonardo [:mak]]

Something you may try (please save a copy of the db apart before, for debugging purposes I would like to get your original db), run each of these queries on the db, check if the problem is solved after each one.
ANALYZE moz_places
ANALYZE moz_historyvisits
ANALYZE moz_inputhistory
ANALYZE moz_bookmarks

May even be the new query planner relies more deeply on sqlite_stat2 (bug 646278)

Comment 13

[Stefan]

(In reply to Marco Bonardo [:mak] from comment #11)
> also, did you change any setting regarding history expiration? Any other
> special setting in your setup we should be aware of?

Using my "big" DB with a fresh profile does not show the issue, when I copy the prefs.js into the new profile the error shows up.

I will now bisect my prefs.js

Comment 14

[Stefan]

This setting causes the delay with the new SQLite >= 3.7.6:

browser.urlbar.default.behavior;1

"The default of this preference in SeaMonkey 2 is 1, meaning that only History items are shown." (http://kb.mozillazine.org/Browser.urlbar.default.behavior)

Comment 15

[Marco Bonardo [:mak]]

that may mean the history query is slower, and with that behavior we run only that one.
Could you investigate (through SQLite manager for example) comment 12?

Comment 16

[Stefan]

(In reply to Marco Bonardo [:mak] from comment #12)
> run each of these queries on the db, check if the problem is solved after each one.

No change after the ANALYZE queries.

Comment 17

[Marco Bonardo [:mak]]

well this needs an owner, I will be away in a week from now, so I have to investigate this week.

Comment 18

[Stefan]

(In reply to Stefan from comment #14)
> "The default of this preference in SeaMonkey 2 is 1, meaning that only
> History items are shown."
> (http://kb.mozillazine.org/Browser.urlbar.default.behavior)

On my machine Fx shows results from bookmarks when the pref's value is 0, 1, 2, 8, 16, 64 (with 128 max. checked value). Is this expected? Fx shows the result most rapidly with pref = 2 (only bookmarks).

Comment 19

[Marco Bonardo [:mak]]

So, the new query planner takes a completely different path to optimize the query, that is wrong (uses the visit_count index rather than the frecency index).

But here running "ANALYZE moz_places" completely solves the problem, are you sure it didn't make any difference in your case?

Btw, for added safety we can enforce it taking the right path using the magic '+' on the wrong index.

Comment 20

[Marco Bonardo [:mak]]

Attachment: patch v1.0

Comment 21

[Stefan]

(In reply to Marco Bonardo [:mak] from comment #19)
> But here running "ANALYZE moz_places" completely solves the problem, are you
> sure it didn't make any difference in your case?

Yes. Just re-checked "ANALYZE moz_places;" with a fresh sqlite3 version 3.7.7.1.

Comment 22

[Marco Bonardo [:mak]]

I have no idea why it doesn't help in your case, here it moved things exactly as they were in 3.7.4.
Could you compile a build with my patch and check if it solves your problem? Otherwise I may give you a trybuild.

Comment 23

[Stefan]

(In reply to Marco Bonardo [:mak] from comment #22)
> Could you compile a build with my patch and check if it solves your problem?

I will check it immediately.

Comment 24

[Stefan]

Marco, your patch works! Shall this issue be reported upstream to the SQLite folks?

Comment 25

[Marco Bonardo [:mak]]

(In reply to Stefan from comment #23)
> (In reply to Marco Bonardo [:mak] from comment #22)
> > Could you compile a build with my patch and check if it solves your problem?
> 
> I will check it immediately.

Yes, we may ask them to check the planner. Btw, based on what I saw locally, it's really just matter of having a fresh analyze result, without a fresh analyze the query planner has no way to know which index will give better results and guesses one that may reduce the number of results. I have other tickets to report upstream, so I'll also report this.

Comment 26

[Marco Bonardo [:mak]]

I quoted the wrong comment :) Btw, thanks for confirming that the patch works for you as well!

Comment 27

[Shawn Wilsher :sdwilsh]

(In reply to Marco Bonardo [:mak] from comment #25)
> Yes, we may ask them to check the planner. Btw, based on what I saw locally,
> it's really just matter of having a fresh analyze result, without a fresh
> analyze the query planner has no way to know which index will give better
> results and guesses one that may reduce the number of results. I have other
> tickets to report upstream, so I'll also report this.
Please make sure you let drh know about this please!

Comment 28

[Shawn Wilsher :sdwilsh]

Comment on attachment 556693 [details] [diff] [review]
patch v1.0

Review of attachment 556693 [details] [diff] [review]:
-----------------------------------------------------------------

r=sdwilsh

Comment 29

[Marco Bonardo [:mak]]

(In reply to Shawn Wilsher :sdwilsh from comment #27)
> Please make sure you let drh know about this please!

I sent a mail yesterday (cc-ed you) reporting the threee issues we found when upgrading to 3.7.7.1, I got a reply from him that our issues will be investigated for 3.7.8, so he is aware of this.

Comment 30

[Marco Bonardo [:mak]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1cbac1b072d5

Comment 31

[Ed Morley [:emorley]]

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