Improve optimizations of get_site_url by not using switch_to_blog

[PerS]

I belive a better approach would be to use the WP_Site_Query query, as in:

<?php
function get_site_url( $blog_id = null, $path = '', $scheme = null ) {
        if ( empty( $blog_id ) || ! is_multisite() ) {
                $url = get_option( 'siteurl' );
        } else {

                // Use WP_Site_Query to get the site URL.
                $site = get_site( $blog_id );

                $url = $site->domain . $site->path;

                // If the network has a custom domain, use that instead.
                if ( $site->domain !== \get_network()->domain ) {
                        $url = get_network()->domain . $site->path;
                }
        }

        $url = set_url_scheme( $url, $scheme );

        if ( $path && is_string( $path ) ) {
                $url .= '/' . ltrim( $path, '/' );
        }

        /**
         * Filters the site URL.
         *
         * @since 2.7.0
         *
         * @param string      $url     The complete site URL including scheme and path.
         * @param string      $path    Path relative to the site URL. Blank string if no path is specified.
         * @param string|null $scheme  Scheme to give the site URL context. Accepts 'http', 'https', 'login',
         *                             'login_post', 'admin', 'relative' or null.
         * @param int|null    $blog_id Site ID, or null for the current site.
         */
        return apply_filters( 'site_url', $url, $path, $scheme, $blog_id );
}

Improve optimisations of get_site_url() by not using switch_to_blog, instead use the WP_Site_Query query

[johnbillion]

Thanks for the ticket and PR @PerS. Some questions that will need answering before this change can be made:

• Does get_site_url() have sufficient test coverage to make this change safe?
• This change means the filters in get_option() will no longer fire when fetching the site URL, is that a concern? For example domain mapping plugins might break.
• What sort of performance improvement does this result in?

[PerS]

Hi John,

The existing unit tests should be sufficience, and I've added a domain mapping check.

Performance wise, my main reason for this patch is to avoid switch_to_blog, even with the latest fixes it's a slow function. And for a super admin it's triggered a lot (ref query monitor).

[manfcarlo]

I strongly recommend not to accept this patch. The reasons should be obvious. It replaces a query of one field in one table to a totally different field in a totally different table. The effects would be significant and wide-reaching, and to describe it as a "performance" tune-up is misleading.

I'm no fan of switch_to_blog and like to see it avoided, but you can't do it by throwing the baby out with the bathwater.

[jeremyfelt]

I don't think wp_blogs.domain + wp_blogs.path is a one to one replacement for the siteurl option. It's more aligned with the home option instead.

I don't have a specific example where this would break, but I'd be very wary of changing the existing option call.

One particular case that would need testing that is probably not covered in our existing tests is WordPress installed in a subdirectory and configured for multisite.