-
Notifications
You must be signed in to change notification settings - Fork 91
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
Comments
❤️ |
I have been working on some filters that would help provide detailed stats |
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. |
should we define the thresholds? |
750KB for warning (yellow background), 1MB+ for error (red background) |
From WPEngine
From Kinsta
|
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. |
@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:
|
@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! |
@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. |
For reference, VIP GO limits to 1MB work of autoloaded options. See alloptions-limit.php |
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. |
I've started working on implementing a preliminary version of how I envision these checks.
|
@manuelRod: Should we point to a developer.wordpress.org article by default? |
@tillkruss indeed, I was thinking about that, but couldn't find any relevant article. Is there any? |
@manuelRod: No, it probably needs to be added to https://wordpress.org/support/article/optimization/ |
I put together a PR for our initial iteration if anyone wants to take a look and give some feedback. |
@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 :) |
@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 ) |
Also see https://github.com/wp-cli/doctor-command/blob/main/inc/checks/class-autoload-options-size.php (Didn't see "doctor" mentioned yet.) |
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 |
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:
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. |
Hello @felixarntz thanks for the review, I commented in the PR, but maybe here is easier to follow the discussion.
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. |
@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
This can be overwritten by the hosts to adjust the autoloaded recommended size for the warnings, and also the message / recommended link, etc. |
That's a good suggestion @felixarntz , having something actionable out of it. |
Please vote for an approach with a thumbs-up, thumbs-down, or heart emoji. Voting will close on Friday, March 18 at 5pm UTC.
|
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.
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. |
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?) |
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/ |
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. |
@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. |
@felixarntz follow up issue created 234. |
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. |
Fixed via #124. |
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.
What do you think?
@spacedmonkey @pbearne
The text was updated successfully, but these errors were encountered: