When urt.is_oneway <> 0
is used, MySQL is forced to use ALL on the table join, making it an order of magnitude slower. As a rough benchmark, removing that condition makes a 1s query 1ms, and reduces the load time of /relationships by about 4 (!) seconds. On sites with many relationships, but few (or one) relationship type, this could be optimized better.
Here's the EXPLAIN output:
The current query:
EXPLAIN EXTENDED SELECT COUNT(DISTINCT rid) AS count FROM user_relationships ur INNER JOIN user_relationship_types urt USING ( rtid ) WHERE (ur.requester_id = 43 OR ((ur.approved <> 1 OR urt.is_oneway <> 0) AND ur.requestee_id = 43)) AND ur.approved = 1
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE urt ALL PRIMARY NULL NULL NULL 1 100.00
1 SIMPLE ur ref PRIMARY,requester_id,requestee_id,rtid,approved rtid 4 grammy365_drupal_blizzard.urt.rtid 36801 100.00 Using where
And a modified query, assuming a single SELECT is used beforehand to get the value of urt.is_oneway:
EXPLAIN EXTENDED SELECT COUNT(DISTINCT rid) AS count FROM user_relationships ur INNER JOIN user_relationship_types urt USING ( rtid ) WHERE (ur.requester_id = 43 OR ((ur.approved <> 1 OR 0 <> 0) AND ur.requestee_id = 43)) AND ur.approved = 1
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE urt index PRIMARY name 768 NULL 1 100.00 Using index
1 SIMPLE ur ref PRIMARY,requester_id,rtid,approved PRIMARY 4 const 94 100.00 Using where
Comments
Comment #1
deviantintegral CreditAttribution: deviantintegral commentedHere's a patch that fixes the indentation of the switch block, and only checks for oneway relationships that are actually configured. All tests currently pass. I'll post benchmarks soon, but on my localhost this brings loading /relationships down from ~2.4s to around 1.1s for a user with many relationships.
Comment #2
deviantintegral CreditAttribution: deviantintegral commentedResults are a little less impressive for a different user account with a more representative number of relationships, but here's the ab results:
Before the patch:
After the patch:
Comment #3
BerdirCan you provide a patch against 7.x-1.x? I currently only commit patches for 7.x.
Comment #4
deviantintegral CreditAttribution: deviantintegral commentedHere it is!
Comment #5
BerdirThe list of relationship types returned by user_relationships_types_load() is statically cached. Instead of doing another query, it is probably faster to simply get all relationship types from there and then select those which are not oneway is probably faster. Also, some comments would be nice to see why we are doing this.
Powered by Dreditor.
Comment #6
deviantintegral CreditAttribution: deviantintegral commentedHere's a patch that uses user_relationships_types_load() and adds some additional comments.
Comment #7
BerdirThanks, commited.
Two notes:
- Many maintainers don't like multiple commits in a single patch, it quickly gets hard to review (with dreditor) and can be confusing. I merged latest commit into the previous one now.
- Remember to add your username to the commit message (Issue #xxx by deviantintegral: ...)
Marking as patch to be ported and back to 6.x-1.x (Although I can not guarantee you that anything will happen with a 6.x patch..)
Comment #8
mrf CreditAttribution: mrf commentedSo the first patch no longer applied and I manually updated it for 6.x dev. I included the types_load optimization from #6 as well. I am now getting mysql errors when visiting user profile pages and wanted to get another set of eyes on this to see if I'm missing something obvious.
Comment #9
mrf CreditAttribution: mrf commentedWaking up the testbot.
Comment #10
mrf CreditAttribution: mrf commentedDitto.
Comment #11
mrf CreditAttribution: mrf commentedFound the error in the manual patch looking at this fresh. This passes tests locally.
Comment #13
mrf CreditAttribution: mrf commented#11: 1183040.11-optimize-oneway-queries.patch queued for re-testing.
Comment #14
mrf CreditAttribution: mrf commentedCommitted to 6.x-1.x