386986 - Cleanup SELECT queries in nsAnnotationService

Status: RESOLVED INVALID

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Don't know if this could be useful to you, but i have quickly reviewed select queries in nsAnnotationService, and they could be cleaned up a bit. 

I'm CC-ing Dietrich because i've seen that he has worked (is working) on this file recently for expire, so maybe he could check and eventually use these for his work. 
I have not made a patch for the same reason, these files are changing too much in these days and annotation support is in early stage.

Feel free to INVALID this if you find it useless.

Here is my report:


mDBGetAnnotation
================
Use a JOIN instead of a subselect

before
  SELECT *
  FROM moz_annos
  WHERE place_id = ?1 AND anno_attribute_id =
  (SELECT id FROM moz_anno_attributes WHERE name = ?2)

after
  SELECT a.*
  FROM moz_annos a
  JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
  WHERE a.place_id = ?1 AND n.name = ?2


mDBGetItemAnnotation
====================
Use a JOIN instead of a subselect

before
  SELECT *
  FROM moz_items_annos
  WHERE place_id = ?1 AND anno_attribute_id =
  (SELECT id FROM moz_anno_attributes WHERE name = ?2)

after
  SELECT a.*
  FROM moz_items_annos a
  JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
  WHERE a.place_id = ?1 AND n.name = ?2


mDBGetAnnotationNames
=====================
Cleanup, LEFT JOIN goto next line

before
  SELECT n.name
  FROM moz_items_annos a LEFT JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
  WHERE a.item_id = ?1

after
  SELECT n.name
  FROM moz_items_annos a
  LEFT JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
  WHERE a.item_id = ?1


mDBGetAnnotationFromURI
=======================
Use a JOIN instead of a subselect, so bindparam not needed in fields list

before
  SELECT a.id, a.place_id, ?2, a.mime_type, a.content, a.flags,
    a.expiration, a.type
  FROM moz_places h JOIN moz_annos a ON h.id = a.place_id
  WHERE h.url = ?1 AND a.anno_attribute_id =
  (SELECT id FROM moz_anno_attributes WHERE name = ?2)

after
  SELECT a.id, a.place_id, n.name, a.mime_type, a.content, a.flags,
    a.expiration, a.type
  FROM moz_places h
  JOIN moz_annos a ON h.id = a.place_id
  JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id
  WHERE h.url = ?1 AND  n.name = ?2


mDBGetAnnotationFromItemId
==========================
Use a JOIN instead of a subselect, so bindparam not needed in fields list

before
  SELECT a.id, a.item_id, ?2, a.mime_type, a.content, a.flags,
    a.expiration, a.type
  FROM moz_items_annos a
  WHERE a.item_id = ?1 AND a.anno_attribute_id =
  (SELECT id FROM moz_anno_attributes WHERE name = ?2)

after
  SELECT a.id, a.item_id, n.name, a.mime_type, a.content, a.flags,
    a.expiration, a.type
  FROM moz_items_annos a
  JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id
  WHERE a.item_id = ?1 AND n.name = ?2

Comment 1

[Dietrich Ayala (:dietrich)]

thanks for looking into this. this should not affect my expiration patch, so please go ahead and make a patch for this change. i'd recommend doing some basic profiling for this. my gut says these joins should be faster than subselect, but it'd be good to have at least a primitive estimate of the performance delta before making the change.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i checked all queries with sqlite.exe with timer... on most of them i don't get advantages, on some the SUBSELECTs have advantage on JOINs!

it feels like in sqlite subselects are always faster than joins, or at least better optimized.

So, after some test, in sqlite it feels valid the following rule: 
subselect is faster than join that is faster than left join

now, feel free to invalid this report cause there is no preformance advantage in using joins

Comment 3

[Dietrich Ayala (:dietrich)]

Marco, thanks for looking into this, it's very helpful information. It would be interesting to see the EXPLAIN output for the instances where a subselect is faster than a join.

If we have a "SQLite Performance Best-practices" wiki page on devmo, it'd be great to catalog this there.

Comment 4

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv