Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Site Health Audit for Autoloaded options #92

Closed
manuelRod opened this issue Jan 17, 2022 · 35 comments · Fixed by #124
Closed

Create Site Health Audit for Autoloaded options #92

manuelRod opened this issue Jan 17, 2022 · 35 comments · Fixed by #124

Comments

@manuelRod
Copy link
Contributor

@manuelRod manuelRod commented Jan 17, 2022

Create a new module for site health to audit and measure heavy autoloaded data usage from the options table.

We could discuss different approaches for this or we can create and evolve the module further in different steps.

  1. Informative module, we just gather the information and warn the user if needed (ex, you have more than X autoloaded options), we show them useful information of what it means or how it can be solved (exmple )
  2. Functional module, we can go a step further and keep track of used/unused options, and then give the ability to users to clean up the table from the site health screen itself.

What do you think?

@spacedmonkey @pbearne

@mehigh
Copy link

@mehigh mehigh commented Jan 17, 2022

❤️
If you don't know about the problem, you won't be lifting a finger to fix it. This is a great idea!

@pbearne
Copy link

@pbearne pbearne commented Jan 17, 2022

I have been working on some filters that would help provide detailed stats

@yogeshbeniwal
Copy link

@yogeshbeniwal yogeshbeniwal commented Jan 17, 2022

Nice idea, may be rather than X autoload options, we can keep it as more than x MB size of autoload.

  1. you have more than X autoloaded options

@tillkruss
Copy link
Member

@tillkruss tillkruss commented Jan 17, 2022

Nice idea, may be rather than X autoload options, we can keep it as more than x MB size of autoload.

Yeah, byte size and total count would be very useful.

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 18, 2022

should we define the thresholds?

@mehigh
Copy link

@mehigh mehigh commented Jan 18, 2022

750KB for warning (yellow background), 1MB+ for error (red background)

@eclarke1 eclarke1 moved this from Backlog to To do in [Focus] Site Health Jan 18, 2022
@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 18, 2022

From WPEngine

While autoloaded data is useful in some cases, it is simply not necessary for most information to be load on every single request. It’s recommended to keep your total autoloaded data below 800,000 bytes (0.8Mb) for optimal performance.

From Kinsta

How much is too much-autoloaded data? This can vary of course, but ideally, you want this to be between 300 KB to 1MB. Once you start approaching the 3-5 MB range or more, there are most likely things that can be optimized or removed from being autoloaded. And anything above 10 MB should be addressed right away. This doesn’t always mean it’s going to cause an issue, but it’s a good place to start.

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Jan 18, 2022

I think whatever values we have for sizes would need a filter. It is made clear by these quotes, different hosts have different levels of acceptable autoload sizes.

@tillkruss
Copy link
Member

@tillkruss tillkruss commented Jan 18, 2022

@manuelRod: I've got a PR nearly ready for #5 that almost identical to this. What would you suggest to the user if the autoloaded options are too big, an object cache, or to reduce it?

From @spacedmonkey:

It can be, storing more that 1MB of autoloaded options in a simple cache object is an issue.
In fact VIP GO hosting, will error out at 1MB+ of options in that cache key
VIP GO it will return a WP_DIE as I understand
We can’t do it in core of course, but a warning to users in site health might be useful

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 19, 2022

@tillkruss as we discussed briefly on slack yesterday, the idea of this audit will be purely informative (at least on the first version) and everything should have filters ( from the information we expose to links on how to solve the problem ) those filters can be used by hosting companies to propose their solutions, see: Kinsta example)

@spacedmonkey as we talked about yesterday, could you bring this ticket to the hosting channel and see if we can get some feedback from their side?

@eclarke1 can u assign this one to me? thanks!

@eclarke1
Copy link

@eclarke1 eclarke1 commented Jan 19, 2022

@manuelRod sure thing, assigned to you and once development begins, it would be great to move to 'In Progress' and remove the 'Needs Discussion' label (once we're agreed on an approach). Thanks so much.

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Jan 19, 2022

For reference, VIP GO limits to 1MB work of autoloaded options. See alloptions-limit.php

@getsource
Copy link
Member

@getsource getsource commented Jan 19, 2022

I love this idea!

Would it make sense to profile for the user the time it takes to query the options in some way, too, since what is impactful for a user might be different depending on the host?

I'm also wondering if there might be some way to build in or recommend optimizing the table when necessary, which especially with MyISAM, can completely fix a performance issue with options.

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 19, 2022

I've started working on implementing a preliminary version of how I envision these checks.
There are 3 filters that can be used by 3rd parties:

  1. Warning limit ( we must define one by default, 800kb maybe?), but different hosting companies seem to recommend different max sizes ( from 700kb to 1MB as I can see )
  2. Description will be filterable too, so messaging can be modified.
  3. Actions will be empty by default, but it has a filter for 3rd parties to hook and link to their internal guides (ex, hosting guides on how to clean the table ).

To make it a bit more visual, attached a screenshot.
Screen Shot 2022-01-19 at 4 20 02 PM

@tillkruss
Copy link
Member

@tillkruss tillkruss commented Jan 19, 2022

@manuelRod: Should we point to a developer.wordpress.org article by default?

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 20, 2022

@tillkruss indeed, I was thinking about that, but couldn't find any relevant article. Is there any?

@tillkruss
Copy link
Member

@tillkruss tillkruss commented Jan 20, 2022

@manuelRod: No, it probably needs to be added to https://wordpress.org/support/article/optimization/

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 24, 2022

I put together a PR for our initial iteration if anyone wants to take a look and give some feedback.

@eclarke1 eclarke1 moved this from To do to In progress in [Focus] Site Health Jan 24, 2022
@eclarke1
Copy link

@eclarke1 eclarke1 commented Jan 24, 2022

@manuelRod thanks, I've moved this to 'In Progress' and will also add the 'Needs Review' label so it prompts folks that a review is needed. I've kept as 'In Progress' as I know this is an initial iteration :)

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Jan 26, 2022

@eclarke1 can you also add labels to the PR? Or let me know how to do so? ( not sure if I have permission for that )

@lkraav
Copy link

@lkraav lkraav commented Jan 29, 2022

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Feb 15, 2022

Thanks for the PR review @spacedmonkey anyone else want to chime in? This would be a good candidate to be merged in 1.0.0
cc: @felixarntz @aristath

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 14, 2022

I just reviewed the PR #124 (review), and I have some concerns about the module in its current state. While I agree it may be helpful to surface it as a problem, we also need to find ways to make warnings here actionable.

To make some comparisons here:

  • For the Audit Enqueued Assets module, the end user can check in the frontend which CSS/JS assets are being enqueued and from there get an idea which plugins or themes are primarily responsible. They can then decide if they can get rid of some or file a support request or something along those lines.
  • For the WebP support module, the user can reach out to their hosting provider to see if that can be enabled. They could even change their hosting provider otherwise.
  • For the object cache module, the user can change hosting or switch to a better tier that includes object cache.

The module here is different in that it has little to nothing that the end user can do to improve the situation, since it's not even possible to view the autoloaded options for them right now.

I think we should explore this for how we can provide better guidance here since as of now I think this is not actionable, and we should therefore give this more time before shipping to users. If most of you think this should still be merged as is, I'd say we should at least put the experimental label on it for now.

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Mar 15, 2022

Hello @felixarntz thanks for the review, I commented in the PR, but maybe here is easier to follow the discussion.

Hello @felixarntz those are valid concerns, my initial idea was to flag this problem so hostings could link to actionable guides on how to solve it ( you can check the original discussion on its issue)

Most, if not all, hosting providers have specific articles pointing to this problem, see Kinsta one.

It happened to me in the past that some clients were having death screens that were caused by this, opening a ticket to the hosting, it ended up sending me a link to its guide on how to solve it. These warnings would allow hosting to tweak its limits and point the client in the right direction. (that's why threshold + actionable text is filterable).

Regarding default behavior, @tillkruss mentioned in the issue that this would be a nice place https://wordpress.org/support/article/optimization/ though it would need a new topic to cover the issue.

As for the next iteration, indeed, we should expand functionality and give more information or even offer some actionable solution, see again point 2 in the issue.

Functional module, we can go a step further and keep track of used/unused options, and then give the ability to users to clean up the table from the site health screen itself.

I'm open to keeping iterating and extending the functionality, but the "actionable" part of this module was thought ( or I thought ) to be used by hostings, so they can point out to their guides ( every hosting has an article on the topic ). I'm not sure though if we can get hostings onboard if this is not in the core.

@mehigh
Copy link

@mehigh mehigh commented Mar 15, 2022

@manuelRod this is the top search result for the "clean up autoload" query, and is a useful and well-written article (used it myself too), which can serve as a default for a default message of

To improve performance, a clean-up of the options table can help:
https://kinsta.com/knowledgebase/wp-options-autoloaded-data/

This can be overwritten by the hosts to adjust the autoloaded recommended size for the warnings, and also the message / recommended link, etc.

@mehigh
Copy link

@mehigh mehigh commented Mar 15, 2022

That's a good suggestion @felixarntz , having something actionable out of it.
Would the above satisfy your suggestion for action decently well to move this forward without the addition of an experimental label?

@bethanylang
Copy link

@bethanylang bethanylang commented Mar 15, 2022

Please vote for an approach with a thumbs-up, thumbs-down, or heart emoji. Voting will close on Friday, March 18 at 5pm UTC.

  • 👍 for merge this module as is and including in the next release
  • 👎 for merge as experimental for now and include in this release, and address the concerns in a follow-up release
  • ❤️ for delay merging until concerns have been addressed

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 15, 2022

@manuelRod

I'm open to keeping iterating and extending the functionality, but the "actionable" part of this module was thought ( or I thought ) to be used by hostings, so they can point out to their guides ( every hosting has an article on the topic ). I'm not sure though if we can get hostings onboard if this is not in the core.

That makes sense, however the out-of-the-box core experience should also provide some level of actionable guidance. I agree the hosting providers here can help more, but we also need a foundation that works for core for the case that the filters aren't used - which realistically will be the case for a ton of sites, any sites that aren't on one of the major hosts that pay attention to WordPress.

@mehigh

Would the above satisfy your suggestion for action decently well to move this forward without the addition of an experimental label?

The documentation in https://kinsta.com/knowledgebase/wp-options-autoloaded-data/ looks solid, however linking to an article from a specific commercial host is a no-go for WordPress core.

I think the solution here will need to be either to write and publish an article about this in an official WordPress channel (e.g. https://wordpress.org/support/) or at least have a brief explanation on how to address this right in the Site Health check. Of course the latter could only be 2-3 sentences so that may be challenging and I'd see it as more of a temporary solution. But if some guidance like that was there, at least we have something.

If we move this forward as is or with some quick temporary messaging added, it would help this module to also have follow-up issues created for enhancing the guidance, so that we don't forget about it. We need to be careful not just to merge things and let them sit around, there should be a long-term path forward to an eventual WordPress core merge. And I think for that a documentation article on wordpress.org is essential.

@mehigh
Copy link

@mehigh mehigh commented Mar 16, 2022

Then how about linking to several existing online resources from a wordpress.org article? They're public knowledge more than being specific advertisement for a host.

Also I think this should move forward and get merged with a paragraph recommendation without a link, and slot that link in when that reference wordpress.org article comes into existence (who should we bump to get that added to the list of things to write, anyone?)

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Mar 16, 2022

So, what about marking it as experimental, no link to anywhere, just the basic info. But trying to get the right information as a new topic in here? https://wordpress.org/support/article/optimization/
I think this would be a good article to link (when there is a topic on the matter)

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 16, 2022

@manuelRod

So, what about marking it as experimental, no link to anywhere, just the basic info. But trying to get the right information as a new topic in here? https://wordpress.org/support/article/optimization/ I think this would be a good article to link (when there is a topic on the matter)

That sounds great to me. My personal take would be to mark it experimental in the current state until some more guidance, either directly in the check or via a link in there, is available.

@bethanylang bethanylang added this to the 1.0.0-beta.2 milestone Mar 17, 2022
@felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 17, 2022

@manuelRod Would you be interested in creating a follow-up issue to discuss and proceed with the next steps around improving the guidance of this module? Once the current PR gets merged, we should probably close this issue since it is focused on adding the module overall with its foundation.

@manuelRod
Copy link
Contributor Author

@manuelRod manuelRod commented Mar 18, 2022

@felixarntz follow up issue created 234.
Please add proper labels.

@bethanylang
Copy link

@bethanylang bethanylang commented Mar 18, 2022

Voting is now closed.

Based on 4 thumbs up votes, 6 thumbs down votes, and no heart votes, we'll merge the Site Health autoloaded options module as experimental for now and include in Monday's 1.0.0-beta.2 release, and address the concerns regarding user next steps in a follow-up release.

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 18, 2022

Fixed via #124.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment