CUNY Academic Commons

Yes, I can confirm that it's loading very slowly.

A significant part of the problem is that images on the page are embedded with URLs that look like this:
https://qcurban.commons.gc.cuny.edu/files/sb-instagram-feed-images/277044118_5271389622917272_422760944997931755_nlow.jpg

This format requires WordPress to load and check whether the file should be read-protected. In the case of the fully-public qcurban.org, this is never the case. Image URLs should be of the format:
https://qcurban.commons.gc.cuny.edu/wp-content/blogs.dir/20623/files/sb-instagram-feed-images/277044118_5271389622917272_422760944997931755_nlow.jpg

Note the /wp-content/blogs.dir/20623/ section in the middle.

I'm unsure at a glance why this is happening. I tested creating a new post on qcurban.org, and inserting an item from the Media Library. In the Media Library and in the Editor, images are loaded from the /wp-content/ URL. But when I view a preview on the front end, they've been changed to the /files/ format. I'll research this - and maybe Ray (CCed here) has some idea why it's happening - but in the meantime, you could go through saved pages, such as 'Front', and change the URL format for images (edit each image block as HTML).

Aside from the image issue, the TTFB is also extremely slow for the site. I enabled Query Monitor but it doesn't show anything egregious at the database level. I'll do some more diagnostics to see if I can narrow things down.

It appears that ml-slider is adding 4 or 5 seconds to each page load. Here's the technical explanation: https://wordpress.org/support/topic/call-to-get_plugins-causing-performance-problems/ It's not obvious that I can work around this problem in ml-slider without forking the plugin.

I have to step away for the day, but first to leave a note for myself and Ray: The blogs.dir problem appears to come from Mercator and domain mapping. When WP builds attachment URLs wp_get_attachment_url(), it uses wp_upload_dir(). The latter function uses get_option( 'siteurl' ) to build its base values. In the case of a mapped domain, get_option( 'siteurl' ) returns the unmapped value - Mercator only filters the site_url() wrapper function. As such, the various str_replace juggling in wp_upload_dir() doesn't work right.

I haven't fully worked out the details or though of a good workaround - maybe there's an additional filter we can apply in our own wp-content/mu-plugins/assets/mercator? - but I won't be able to look further today. Ray, if you have the interest and energy, as our local Mercator expert, I'd welcome your eyes. Otherwise I'll have another look in the upcoming days.

Thanks for all you help. I guess first step is to use a different slider plugin.

Aside from the image issue, the TTFB is also extremely slow for the site.

This is the main issue.

I guess first step is to use a different slider plugin.

Yes, this is my main conclusion as well.

Deactivating MetaSlider speeds up the site by between 4-6 seconds per WordPress asset request (the main document, plus any asset requests to /files/). Deactivating other plugins can reduce the TTFB down to under a second.

I've left MetaSlider deactivated for now.

When WP builds attachment URLs wp_get_attachment_url(), it uses wp_upload_dir(). The latter function uses get_option( 'siteurl' ) to build its base values. In the case of a mapped domain, get_option( 'siteurl' ) returns the unmapped value - Mercator only filters the site_url() wrapper function.

Boone, thanks for your analysis here. I changed Mercator to filter 'option_home' and 'option_siteurl' instead of 'site_url' and 'home_url' on production (haven't committed the change to wp-content/mu-assets/mercator/ yet), but that didn't really do much for the TTFB or asset load time.

Should all attachment URLs for public sites be written as /wp-content/blogs.dir/ instead of /files/? If so, we can look at writing a filter to do this.

Thanks, Ray!

Boone, thanks for your analysis here. I changed Mercator to filter 'option_home' and 'option_siteurl' instead of 'site_url' and 'home_url' on production (haven't committed the change to wp-content/mu-assets/mercator/ yet), but that didn't really do much for the TTFB or asset load time.

With Meta Slider deactivated, the home page doesn't call wp_get_attachment_url() very much. Instead, the URLs are saved in the post content. But to get a sense of load time differences, try these two URLs side by side (uncached especially):

a. https://qcurban.org/files/2022/03/wonderlane-mVnI4UxYxJI-unsplash2.jpg
b. https://qcurban.org/wp-content/blogs.dir/20623/files/2022/03/wonderlane-mVnI4UxYxJI-unsplash2.jpg

The instances that are in the existing post content would have to be swapped out manually. But your filter fix should ensure that, for new content, the URLs inserted into post_content ought to have the longer URL.

Should all attachment URLs for public sites be written as /wp-content/blogs.dir/ instead of /files/? If so, we can look at writing a filter to do this.

I think the question is whether we should (a) go with the broader fix that you've currently done to Mercator (filtering option_home and option_siteurl), (b) filter upload_dir to make the necessary changes, or (c) specifically target the attachment URLs with a filter. Option (a) is the most general but has the greatest likelihood of side effects (though I can't think of any off the top of my head). Option (b) feels like it's the right level for the Mercator filter, but it's not totally straightforward because of the filter placement in WP - I think we'd need to reproduce some of the logic of _wp_upload_dir() to build the paths properly. With (c) we could pretty much just add Mercator's existing filters. What do you think?

I think the question is whether we should (a) go with the broader fix that you've currently done to Mercator (filtering option_home and option_siteurl)

Option (a) is the most general but has the greatest likelihood of side effects (though I can't think of any off the top of my head).

There might have been a reason why Mercator did not filter option_home and option_siteurl in the first place.

While I think it makes sense to filter option_home and option_siteurl in theory, one side effect is since the attachment URLs would now point to the mapped domain and be saved in the post content, if a site ever decides to remove their domain mapping, then those attachment URLs would no longer work on the Commons subdomain site.

or (c) specifically target the attachment URLs with a filter

This sounds the safest, but it's also a pain.

I was looking at some prior code I've written for another site and I had to use a bunch of filters -- 'the_post', 'the_content', 'the_excerpt', 'wp_get_attachment_image_src', 'wp_get_attachment_url', 'wp_calculate_image_srcset', 'theme_mod_header_image', 'theme_mod_background_image'. Code was to replace attachment URLs with those uploaded to Amazon S3, but general approach would be the same here.

Do a regex search for any /files/ URLs and switch them out for the /blogs.dir/ equivalent if that site is public only. I think there is definitely some benefit to doing this.

The option_home and option_siteurl filter change affected mapped domain single sign-on (see #15810). It was causing a redirect loop. I've just removed the change on production.

Looks like filtering the attachment URLs is the way to go.

Eesh, thanks for the quick response, Ray.

'wp_get_attachment_image_src', 'wp_get_attachment_url', 'wp_calculate_image_srcset', 'theme_mod_header_image', 'theme_mod_background_image'

Definitely we should do these. It's possible that 'wp_get_attachment_url' will do most of the work.

'the_post', 'the_content', 'the_excerpt'

These sorts of filters make me more nervous. We'd need to make sure we only look inside of 'src' attributes, not elsewhere in body text, but in any case we need to be very careful when running regular expressions on user-provided text. In fact, my inclination is to ignore post content. The 'wp_get_attachment_url' filter will ensure that future images inserted into posts will be correct. In some cases, like Scott's, it may be beneficial to go back and do the swapouts, but it should be done manually by the admin.

Boone, while debugging #16346, I found that the ACERT site was extremely slow and noticed that ml-slider did not apply your suggested fix to the plugin. So I've reapplied your hotfix for ml-slider again: https://github.com/cuny-academic-commons/cac/commit/bb69429fc4a13cbecb4ab44c9fe3374b79d697da

Boone, while debugging #16659, I found that the ILETC site was using ml-slider and again noticed that ml-slider did not apply your suggested fix from before. So I've reapplied your hotfix for ml-slider once more: https://github.com/cuny-academic-commons/cac/commit/8fb42bfa205a106702807d26bfd5cd8c0ae66323

I've replied on the wordpress.org forum thread that you created to hopefully make them aware of the problem: https://wordpress.org/support/topic/call-to-get_plugins-causing-performance-problems/

Thanks for updating the thread, Ray - I was pleased to see they responded right away. I'll try to remember to look the next time ml-slider is updated.

I also had questions about the ml-slider on another issue, but I looked at the their update log and it looked like took Boone advice and fixed the issue.

Boone, while looking into the ILETC site again, I noticed that the MetaSlider team has not addressed the get_plugins() issue from before. So I've re-applied the hotfix from before and have replied to the wordpress.org thread once more: https://wordpress.org/support/topic/call-to-get_plugins-causing-performance-problems/. Hopefully they will address this issue soon.