Closed Bug 682676 Opened 9 years ago Closed 9 years ago

urlbar performance regression with SQLite 3.7.7.1

Categories

(Toolkit :: Storage, defect)

Other
Other
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9
Tracking Status
firefox9 - ---

People

(Reporter: kdevel, Assigned: mak)

References

Details

(Keywords: perf, regression)

Attachments

(1 file)

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.
(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.
I have bisected the issue:

hg bisect -b
The first bad revision is:
changeset:   75951:2fd69e4236ea
user:        Marco Bonardo <mbonardo@mozilla.com>
date:        Fri Aug 26 13:38:28 2011 +0200
summary:     Bug 666420 - Upgrade to SQLite 3.7.7.1 - SQLite changes.
(In reply to Stefan from comment #2)
Does running Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");PlacesDBUtils.checkAndFixDatabase(); in the Error Console work?
(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
(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
Ok. CCing Marco.
Component: General → Storage
Keywords: regression
Product: Firefox → Toolkit
QA Contact: general → storage
SQLite 3.7.6 ist the first which shows the behavior.
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?
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
also, did you change any setting regarding history expiration? Any other special setting in your setup we should be aware of?
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)
(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
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)
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?
(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.
well this needs an owner, I will be away in a week from now, so I have to investigate this week.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
(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).
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.
Keywords: perf
Summary: urlbar performance regression → urlbar performance regression with SQLite 3.7.7.1
Attached patch patch v1.0Splinter Review
Attachment #556693 - Flags: review?(sdwilsh)
(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.
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.
(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.
Marco, your patch works! Shall this issue be reported upstream to the SQLite folks?
(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.
I quoted the wrong comment :) Btw, thanks for confirming that the patch works for you as well!
(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 on attachment 556693 [details] [diff] [review]
patch v1.0

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

r=sdwilsh
Attachment #556693 - Flags: review?(sdwilsh) → review+
(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.
http://hg.mozilla.org/mozilla-central/rev/1cbac1b072d5
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.