Closed Bug 583852 Opened 11 years ago Closed 11 years ago
Weave should not be querying on places views
Views are very slow to select on because they have no indexes. Views also consume more memory (table has to be created in memory to query from). Additionally, views are very likely to be going away in bug 552023.
Instead of querying the views we should query the UNION of table + temp table as per https://wiki.mozilla.org/Places/Places_SQL_queries_best_practices. This should also prevent the symptoms of bug 575423. If the temp table doesn't exist, bug 552023 landed and we don't have to worry about this anymore and query the table directly.
Tests for the history store.
Assignee: nobody → philipp
Attachment #463910 - Flags: review?(mconnor)
Query moz_places, moz_historyvisits *and* moz_places_temp, moz_historyvisits_temp instead of the slower views. If the temp tables aren't available (bug 552023), just query the regular tables. Also fixed some style and syntax nits.
Harmonize spelling of table + temp table queries with places: * query moz_*_temp before moz_* * use IFNULL rather than IN operator in WHERE clause (potential speedup) * use UNION ALL instead of UNION in _urlStm() and add LIMIT 1 since there's at most only 1 result row (potential speedup)
Make tests work when running as part of xpcshell-tests on mozilla-central. Was missing the UHist directory alias again, moved it into head_helpers.js so that all tests will have it available.
Attachment #463922 - Flags: review?(mconnor) → review+
Attachment #463977 - Flags: review?(mconnor) → review+
1.4.x: http://hg.mozilla.org/services/fx-sync/rev/7f4053acded9 http://hg.mozilla.org/services/fx-sync/rev/7deb15c50eaf default: http://hg.mozilla.org/services/fx-sync/rev/dbc4c91f1344 http://hg.mozilla.org/services/fx-sync/rev/e482668c5fca
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
can anyone shed some light on how this can be verified manually? Otherwise, i just assume from the bug that it sounds like unit tests here that fail against m-c will suffice when sounding the alarm.
The decision to query views was made to fix bug 575423. However, querying views is slow and they're going to go away with bug 552023 (hasn't permanently landed on m-c yet, but there's there's mak's tryserver build I used to verify and run my unit tests against: http://email@example.com/tryserver-macosx/) So essentially this bug is an alternate (better performing) way to fix bug 575423 and at the same time it prepares us for nightlies once bug 552023 lands.
Tarek, can you please verify if this is fixed with the 1.4.4 extension?
(In reply to comment #10) > Tarek, can you please verify if this is fixed with the 1.4.4 extension? sorry, wrong bug comment. disregard.
Running Fennec with Weave, while syncing, I am seeing warnings of this type: WARNING: 2 sort operations have occurred for the SQL statement '0xac05b118'. See https://developer.mozilla.org/En/Storage/Warnings details.: file /home/crowder/mobilla/storage/src/mozStoragePrivateHelpers.cpp, line 138 Executing this statement: (gdb) call sqlite3_sql(mStatements[i].mStatement) [Thread 0xb02fdb70 (LWP 13115) exited] $8 = 0xaba1bd68 "SELECT visit_type type, visit_date date FROM moz_historyvisits_temp WHERE place_id = IFNULL( (SELECT id FROM moz_places_temp WHERE url = :url), (SELECT id FROM moz_places WHERE url = :url) ) UNION SEL"... I'm wondering how seriously I should take this warning, but also how we're getting behavior that according to the comment (http://hg.mozilla.org/mozilla-central/annotate/97949d240701/services/sync/modules/engines/history.js#l143) should only happen in Gecko < 2.0
You need to log in before you can comment on or make changes to this bug.