707954 - Index exclusion for tags

Status: VERIFIED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

with lots of tags the query doesn't take the best path, we should enforce it
JOIN moz_bookmarks t ON t.id = +b.parent
this reduces time on a profile I have locally (with 1600 tags) to 30ms, from about 5 seconds.

Comment 1

[Marco Bonardo [:mak]]

Attachment: patch v1.0

Simple and powerful

Comment 2

[Marco Bonardo [:mak]]

The problem here happens because SQLite cannot know that tags are "special" entries in the bookmarks table, indeed it's a nonsense thing. It then just do an optimization thinking all bookmarks are equal, and not that some is more equal than others.

Comment 3

[Dietrich Ayala (:dietrich)]

Comment on attachment 579501 [details] [diff] [review]
patch v1.0

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

r=me. is there a way to detect suboptimal execution choice like this? or do we need to do a manual examination of queries to find others that need column exclusion like this?

Comment 4

[Marco Bonardo [:mak]]

I think we need to work on a test harness where to collect all of our queries, build avg and max dbs based on telemetry data, and all queries on those detecting variations. This would be far better than the Dirty tests we have now. I'll file a bug to track this.

In the meanwhile I think bug 699051 will greatly help us.

Comment 5

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8db3e759e1f3

Comment 6

[Marco Bonardo [:mak]]

filed bug 708413

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8db3e759e1f3

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 579501 [details] [diff] [review]
patch v1.0

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

I would like to port this simple queries fix to the stabilization branches, the results are pretty good, as I said on some query it may save seconds, someone noticed improvements to the awesome bar responsiveness in the last days. Will especially help users with many tags, that right now have a really bad responsiveness experience.

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 579501 [details] [diff] [review]
patch v1.0

[Triage Comment]
We're now at the point in the 9 cycle where we're only taking fixes that may require us to chemspill if left unresolved. Approving for Aurora 10 though.

Comment 10

[Marco Bonardo [:mak]]

Thanks, I understand the will to stabilize.  Landed to Aurora.
https://hg.mozilla.org/releases/mozilla-aurora/rev/0c3f20e0594b

Comment 11

[u279076]

I assume this might be something QA can verify given access to the profile mentioned in comment 0. Would this be possible?

Comment 12

[Marco Bonardo [:mak]]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11)
> I assume this might be something QA can verify given access to the profile
> mentioned in comment 0. Would this be possible?

The profile is not mine, has been sent to me by the reporter of bug 701145 and I can't share it without his consent. We may ask him there, or I could make a test profile with many tags. Will do the former for now.

Comment 13

[u279076]

Thanks Marco for providing your profile off-line. Here are the results of our testing:

* in Firefox 10 beta 3 the search took about 2 seconds maybe less
* in Firefox 9.0.1 the same search took about 15 seconds
* in Firefox 8.0.1 the search took about 15 seconds
* in Firefox 7.0.1 the search took about 15 seconds

Given this and discussions offline with Marco, marking this verified.