Skip to:
Content

Opened 2 months ago

Last modified 12 days ago

#7442 new enhancement

Use JOIN rather than Subquery on user search

Reported by: brandonliles Owned by:
Milestone: 2.9 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch dev-feedback
Cc:

Description

The user search relies on two subqueries. This is a known weakness of MySQL performance. Using joins will improve performance.

Attachments (3)

bp-search-use-joins.patch (2.2 KB) - added by brandonliles 2 months ago.
bp-search-use-joins2.patch (2.0 KB) - added by brandonliles 7 weeks ago.
bp-search-optimize.patch (2.3 KB) - added by brandonliles 13 days ago.
Subquery Materialization Approach

Download all attachments as: .zip

Change History (9)

#1 @r-a-y
2 months ago

Thanks for creating this ticket and for the patch, @brandonliles!

However, the patch is breaking multiple unit tests.

If you are able to, can you set up WordPress for PHP unit testing?
https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/

Once you have done so, navigate to the buddypress directory and run phpunit in a command prompt. Then, you will be able to see what testcases are breaking.

If you need further help in setting up PHPUnit, let us know.

#2 @brandonliles
7 weeks ago

Thanks for the feedback @r-a-y just uploaded a second version of the patch that doesn't break stuff :)

#3 @hnla
13 days ago

  • Keywords has-patch dev-feedback added
  • Milestone changed from Awaiting Review to 2.9

@brandonliles
13 days ago

Subquery Materialization Approach

#4 @brandonliles
13 days ago

After seeing the query still perform poorly on a site I took another look, although joins can improve performance, MySQL still tends to look at too many rows in the bp_activity table. Determining the matching user_ids first and plugging those values into the query is a major performance improvement.

New patch bp-search-optimize.patch added.

Last edited 13 days ago by brandonliles (previous) (diff)

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


12 days ago

#6 @r-a-y
12 days ago

Thanks for the new patch, @brandonliles.

bp-search-optimize.patch passes existing unit tests. The rationale behind the "query for IDs first" approach makes sense to me.

Looks good, but needs sign off from a lead dev. We might need some new unit tests to cover this change though.

Last edited 12 days ago by r-a-y (previous) (diff)
Note: See TracTickets for help on using tickets.