Closed Bug 659740 Opened 9 years ago Closed 9 years ago
Frecency update causes SQL sort warning
WARNING: 1 sort operation has occurred for the SQL statement '0x10bb8b578'. See https://developer.mozilla.org/En/Storage/Warnings details.: file /Users/rnewman/moz/git/alder/storage/src/mozStoragePrivateHelpers.cpp, line 144 Every. Damn. Record.
Marco + sdwilsh: is there a way for us to see what query might be triggering this warning?
NSPR logging is generally the way to go. You can also change https://hg.mozilla.org/mozilla-central/annotate/5e1ee7f192ad/storage/src/mozStoragePrivateHelpers.cpp#l118 to print the sql string instead of a pointer.
The query culprit is SELECT ROUND((strftime('%s','now','localtime','utc') - v.visit_date/1000000)/86400), COALESCE( (SELECT r.visit_type FROM moz_historyvisits r WHERE v.visit_type IN (5,6) AND r.id = v.from_visit), visit_type), visit_date FROM moz_historyvisits v WHERE v.place_id = :page_id ORDER BY visit_date DESC LIMIT 10 from toolkit/components/places/nsNavHistory.cpp:1295 That query gets called for each history record, presumably via a Places API call.
(In reply to comment #3) > That query gets called for each history record, presumably via a Places API > call. Shouldn't be hard to figure out which. Getting a backtrace is the way to move forward here.
You are adding a visit and that causes us to recalculate frecency for that page, this happens on the async thread though. I touched this code today, so I could look why this warns, the problem may be minimal though, like caused by the subquery.
(In reply to comment #5) > I touched this code today That should be "I'm working on a patch that touches this code", but my patch is mostly removing some supporting code, not fixing any query or such.
(In reply to comment #5) > I touched this code today, so I could look why this warns, the problem may > be minimal though, like caused by the subquery. I'll just say: we'd all love to see this go away :D Thanks mak!
Component: Firefox Sync: Backend → Places
Product: Mozilla Services → Toolkit
QA Contact: sync-backend → places
Summary: Incoming history record processing causes SQL sort warning → Frecency update causes SQL sort warning
I think we are going to see an increase in warnings, looks like current SQLite sometimes ignores existing indices, that's because if there are a few entries (like 2 or 3) doing sorting using a scan is faster than doing sorting using a b-tree, indeed in this case if I force it to use the index (through INDEXED BY moz_historyvisits_placedateindex) the warning goes away. All of this uses sqlite_stat1 (and, if present, sqlite_stat2) table, so ANALYZE is really important. Based on this, I've added a /* do not warn */ since the indices exist but are ignored by the optimizer. Apart this, the patch slightly improves the queries, and introduces 2 stataments caches for them, removing all the crazy stuff that was introduced to init those statements. In a separate bug we can remove GetStatementByID and GetStatement and directly use these caches for all statements.
Attachment #536268 - Flags: review?(sdwilsh)
Comment on attachment 536268 [details] [diff] [review] patch v1.0 r=sdwilsh
Attachment #536268 - Flags: review?(sdwilsh) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
6 years ago
You need to log in before you can comment on or make changes to this bug.