I experience significant lags in the address bar (basically it doesn’t react to the Return/Enter key, I have to use the mouse and the "Go" and even then it’s not working correclty). The cause are probably long URLs saved in the history. mak in #places told me a limit should be implemented. My places.sqlite is 1.4 GB. Tried to delete the history of March, Firefox then didn’t respond anymore. SELECT COUNT(*) FROM moz_places WHERE LENGTH(url) > 5000 > 15136 SELECT COUNT(*) FROM moz_places WHERE LENGTH(url) > 10000 > 13886 SELECT COUNT(*) FROM moz_places WHERE LENGTH(url) > 30000 > 8885 Steps to reproduce: 1. Disable JavaScript 2. Go to https://clkde.tradedoubler.com/click?p(195105)a(1662290)g(21792436)url(http://www.yves-rocher.de/control/category/~category_id=1506WNBEST)&f=0&f=0&f=0 , it redirects to itself with an added &f=0 everytime

The "forget this site" command took about 80 minutes (on an Intel i5-2500K). places.sqlite-wal grew up to about 2.3 GB. places.sqlite was then still 1.4 GB. The Places Maintenance add-on did a VACUUM and now places.sqlite is about 20 MB. Everything’s working fine again. Still, a limit for large history entries seems to be reasonable.

One of the possible causes of the awesomebar slowness may be long urls, partly due to bug 907001. But this may also be problematic from many other points of view. Big urls make the db grow a lot, that causes any scan queries like the awesomebar to be slower. It also makes any query take longer and increases I/O and memory usage. Finally it confuses expiration, if it notices the db is too big, it may decide to expire much more than needed to try to shrink it. We should decide a threshold, over which urls won't be stored in history at all. Bookmarks will ignore it, so we can bookmark any url. We may also want an expiration rule to expire long uris before, when they have only one visit. But this may be a follow-up. Note: versions 46 on should be faster at removing history (bug 1250363).

Review commit: https://reviewboard.mozilla.org/r/48595/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48595/

This does a couple additional things: - removes some old unused prefs (we usually do this along with another migration) - expires long urls and downloads after 60 days

Comment on attachment 8744509 [details] MozReview Request: Bug 1262887 - Long URLs stored in history cause slow address bar. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48595/diff/1-2/

Comment on attachment 8744509 [details] MozReview Request: Bug 1262887 - Long URLs stored in history cause slow address bar. r=adw https://reviewboard.mozilla.org/r/48595/#review45333 Looks OK to me, but I have one question below about the migration. ::: toolkit/components/places/Database.h:314 (Diff revision 2) > */ > RefPtr<ClientsShutdownBlocker> mClientsShutdown; > RefPtr<ConnectionShutdownBlocker> mConnectionShutdown; > + > + // Maximum length of a stored url. > + // Big URLS are usually not for humans consumption, so there's not much I would flip this comment around so that it's more obvious that the primary reason (only reason?) for having a maximum length is performance. Then at the end say something like "big URLs are not usually for human consumption anyway", but I'm not sure that's really necessary. ::: toolkit/components/places/Database.cpp:50 (Diff revision 2) > #define PREF_FORCE_DATABASE_REPLACEMENT "places.database.replaceOnStartup" > > // Set to specify the size of the places database growth increments in kibibytes > #define PREF_GROWTH_INCREMENT_KIB "places.database.growthIncrementKiB" > > +// The maximum url length we can store for history. More info like "We do not add URLs to history longer than this value" would be helpful. ::: toolkit/components/places/Database.cpp:1676 (Diff revision 2) > + NS_ENSURE_SUCCESS(rv, rv); > + mozStorageStatementScoper scoper(stmt); > + rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("maxlen"), > + MaxUrlLength()); > + NS_ENSURE_SUCCESS(rv, rv); > + rv = stmt->Execute(); It's been a while since I looked at the mozstorage APIs, but doesn't Statement::ExecuteStep() execute on the main thread? And this is on startup too. Won't this be a bad experience for people with lots of long URLs? Actually, is this O(number of long URLs), or is it the even worse O(number of rows in moz_places)?

(In reply to Drew Willcoxon :adw from comment #6) > ::: toolkit/components/places/Database.cpp:1676 > (Diff revision 2) > > + NS_ENSURE_SUCCESS(rv, rv); > > + mozStorageStatementScoper scoper(stmt); > > + rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("maxlen"), > > + MaxUrlLength()); > > + NS_ENSURE_SUCCESS(rv, rv); > > + rv = stmt->Execute(); > > It's been a while since I looked at the mozstorage APIs, but doesn't > Statement::ExecuteStep() execute on the main thread? And this is on startup > too. Won't this be a bad experience for people with lots of long URLs? > Actually, is this O(number of long URLs), or is it the even worse O(number > of rows in moz_places)? Yes, schema updates still happen on the main-thread, because we need a consistent db to move on. I looked at some real-life dbs I had around and the query ended up finding 9 or 10 urls over 2k chars. Surely someone like the OP may have far more (thousands indeed), but the DELETE should still be a quick operation overall, so I'm not much worried. And we want to remove those big urls for them, cause otherwise the global experience with the awesomebar sucks, so it sounds critical enough. On the other side, this comment made me recall bug 1250363 (the reason why deletes are now quick enough), that remembered me I must also execute a "DELETE FROM moz_updatehosts_temp". This is slower and not critical for db sanity, so I'll execute it asynchronously.

Comment on attachment 8744509 [details] MozReview Request: Bug 1262887 - Long URLs stored in history cause slow address bar. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48595/diff/2-3/

Comment on attachment 8744509 [details] MozReview Request: Bug 1262887 - Long URLs stored in history cause slow address bar. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48595/diff/3-4/