Closed
Bug 482276
Opened 15 years ago
Closed 15 years ago
mDBVisitsForFrecency query doesn't use indices for sorting (we can be 7x faster!)
Categories
(Toolkit :: Places, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: sdwilsh, Assigned: mak)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file)
3.63 KB,
patch
|
sdwilsh
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
I'm seeing this a lot in my console with the patch in bug 481261 applied. We have a likely inefficient query here: WARNING: 1 sort operation has occurred for the SQL statement 'SELECT COALESCE(r_t.visit_date, r.visit_date, v.visit_date) date, COALESCE(r_t.visit_type, r.visit_type, v.visit_type) FROM ( SELECT visit_date, visit_type, from_visit FROM moz_historyvisits_temp WHERE place_id = ?1 UNION ALL SELECT visit_date, visit_type, from_visit FROM moz_historyvisits WHERE id NOT IN (SELECT id FROM moz_historyvisits_temp) AND place_id = ?1 ) AS v LEFT JOIN moz_historyvisits r ON r.id = v.from_visit AND v.visit_type IN (5,6) LEFT JOIN moz_historyvisits_temp r_t ON r_t.id = v.from_visit AND v.visit_type IN (5,6) ORDER BY date DESC LIMIT 10'. This may indicate an opportunity to improve performance through the careful use of indexes.: file /Code/moz/central/mozilla/storage/src/mozStoragePrivateHelpers.cpp, line 104 I can't land bug 481261 until this gets fixed because it's pretty darn noisy.
Assignee | ||
Comment 1•15 years ago
|
||
it's mDBVisitsForFrecency used by calculatefrecencyInternal, it's the query that samples the visits to calculate frecency. Number of returned results should be the number of visits on a place_id, won't be so high, but probably the query can be improved.
Assignee | ||
Comment 2•15 years ago
|
||
this query is quite (7x) faster here and does not warn, but this is another case where i see UNION ALL quite faster then UNION. Btw this can use indices on both tables for order by, using the original visit_date instead of the redirected one does not change a lot for us, they are added at ms distance. SELECT v.visit_date, COALESCE( (SELECT r.visit_type FROM moz_historyvisits_temp r WHERE v.visit_type IN (5,6) AND r.id = v.from_visit), (SELECT r.visit_type FROM moz_historyvisits r WHERE v.visit_type IN (5,6) AND r.id = v.from_visit), visit_type) FROM moz_historyvisits_temp v WHERE v.place_id = ?1 UNION ALL SELECT v.visit_date, COALESCE( (SELECT r.visit_type FROM moz_historyvisits_temp r WHERE v.visit_type IN (5,6) AND r.id = v.from_visit), (SELECT r.visit_type FROM moz_historyvisits r WHERE v.visit_type IN (5,6) AND r.id = v.from_visit), visit_type) FROM moz_historyvisits v WHERE v.place_id = ?1 AND v.id NOT IN (SELECT id FROM moz_historyvisits_temp) ORDER BY 1 DESC LIMIT ?2 ; taking for now.
Assignee | ||
Updated•15 years ago
|
Summary: WARNING: We have an inefficient query! → mDBVisitsForFrecency query can't use indices for sorting
Reporter | ||
Comment 3•15 years ago
|
||
(In reply to comment #2) > this query is quite (7x) faster here and does not warn, but this is another > case where i see UNION ALL quite faster then UNION. That makes sense since UNION is going to look to filter out any duplicates, whereas UNION ALL doesn't.
Reporter | ||
Updated•15 years ago
|
Attachment #366379 -
Flags: review?(sdwilsh) → review+
Reporter | ||
Comment 4•15 years ago
|
||
Comment on attachment 366379 [details] [diff] [review] patch v1.0 r=sdwilsh I like perf wins that are 7x faster
Reporter | ||
Updated•15 years ago
|
Summary: mDBVisitsForFrecency query can't use indices for sorting → mDBVisitsForFrecency query doesn't use indices for sorting (we can be 7x faster!)
Assignee | ||
Comment 5•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/863c5c397e3b
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Updated•15 years ago
|
Flags: wanted1.9.1?
Target Milestone: mozilla1.9.2a1 → ---
Reporter | ||
Comment 6•15 years ago
|
||
Comment on attachment 366379 [details] [diff] [review] patch v1.0 requesting approval for a nice performance win (7x faster) with minimal risk.
Attachment #366379 -
Flags: approval1.9.1?
Updated•15 years ago
|
Attachment #366379 -
Flags: approval1.9.1? → approval1.9.1+
Comment 7•15 years ago
|
||
Comment on attachment 366379 [details] [diff] [review] patch v1.0 a191=beltzner
Assignee | ||
Comment 8•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8ec9a419d943
Keywords: fixed1.9.1
Assignee | ||
Updated•15 years ago
|
Flags: wanted1.9.1?
You need to log in
before you can comment on or make changes to this bug.
Description
•