Make WordPress Core

Opened 5 years ago

Last modified 2 years ago

#45640 assigned enhancement

get_blogs_of_user improvements

Reported by: maniu's profile maniu Owned by: alexstine's profile alexstine
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 3.0
Component: Networks and Sites Keywords: needs-unit-tests has-patch needs-refresh
Focuses: multisite Cc:

Description

get_blogs_of_user function is often pulling more data than it needs. This often won't cause any issues but in some cases when user is attached to big number of blogs, it may lead to out of memory error.

Attached patch has changes that can lead to more improvements like pagination for "My Sites" admin page, limitation of listed sites in admin bar and on "access denied" page.

Attachments (2)

get_blogs_of_users.diff (16.0 KB) - added by maniu 5 years ago.
get_blogs_of_user improvements
get_blogs_of_users_new.patch (15.7 KB) - added by maniu 5 years ago.
modified version that does not include tweaks to $all parameters as they are confusing and not crucial for performance improvement.

Download all attachments as: .zip

Change History (7)

@maniu
5 years ago

get_blogs_of_user improvements

#1 @earnjam
5 years ago

  • Keywords needs-unit-tests added
  • Version changed from trunk to 3.0

Hi @maniu, thanks for your ticket! There are definitely improvements to be made around querying sites across a network and improvements to get_blogs_of_user() are likely part of the overall multisite roadmap.

We'll probably pick up more of those discussions as we resume our weekly meetings in Slack in the new year and post 5.0.

Regarding the specific patch here, I wonder if it might make more sense to keep the second parameter as the current boolean value, and instead checking for some type of all_ids value in the $args array.

Mixing the type (bool, string) and usage (manipulating the returned list of sites vs returned fields) of a single parameter feels like it could easily lead to confusion.

#2 @maniu
5 years ago

  • Keywords changed from has-patch needs-unit-tests to needs-unit-tests has-patch

Hey @earnjam, yeah I think you are right. I could have avoided all_ids as having $all = true together with $args = array( 'fields' => 'ids' ) can cover it. I will work on another patch. Thanks!

@maniu
5 years ago

modified version that does not include tweaks to $all parameters as they are confusing and not crucial for performance improvement.

#3 @alexstine
3 years ago

  • Owner set to alexstine
  • Status changed from new to assigned

#4 @alexstine
2 years ago

  • Keywords needs-refresh added

At this point, this one probably badly needs a refreshed patch.

@maniu, is this still a problem?

#5 @maniu
2 years ago

@alexstine Sorry for late response! I believe it is still a problem and now I have also realized that there is potential to improve get_active_blog_for_user performance as well. I will submit updated diff or pull request once its ready. Thanks:)!

Note: See TracTickets for help on using tickets.