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

Performance issue of deactivateLegacyPlugin() #380

Open
waviaei opened this issue May 20, 2022 · 0 comments
Open

Performance issue of deactivateLegacyPlugin() #380

waviaei opened this issue May 20, 2022 · 0 comments
Labels
confirmed bug [severity][p4] Minor severity, little to no impact, can be handled later

Comments

@waviaei
Copy link

waviaei commented May 20, 2022

I have been investigating out client site, which seem to have few bottlenecks on the backend PHP processing, resulting in slow TTFB.

On my local site, using Blackfire as profiling tool, I found that deactivateLegacyPlugin() function of this plugin is one of the bottleneck, occupying up to around 15% (~450ms) of the php processing wall time.

I checked this by commeinting out add_action.

add_action('init', [$this, 'deactivateLegacyPlugin']);

To be more exact, it is the core get_plugins() function, called inside deactivateLegacyPlugin() that is taking the time (and memory).

I feel get_plugins() can be improved to better utilize object cache -- it seems retrieved cache is only utilized only when $plugin_folder parameter is specified. And this particular site has around 70 plugins which doesn't help the situation.

Nevertheless, I think it is unnecessary to check legacy "publishpress-content-checklist" in this way, every time the site is accessed, even on the frontend when user is not logged in.

Possible Solution

Idea 1

I think checking the legacy plugin only needs to happen once, when this new plugin is activated. So this can be called once inside register_activation_hook ?

Idea 2

Put it inside is_admin(). Also add a filter something like publishpress_skip_legacy_check, which when return true, skip deactivateLegacyPlugin() to run. So if a developer is sure that this legacy plugin is not installed, we can skip the function.

Your Environment

  • plugin ver 2.5.0 (I am aware this is not the latest, but 2.7.2 has the same code, so assuming same issue)
  • multisite
  • local environment is lando, with redis as persistant cache.
@richaferry richaferry added the [severity][p4] Minor severity, little to no impact, can be handled later label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed bug [severity][p4] Minor severity, little to no impact, can be handled later
Projects
None yet
Development

No branches or pull requests

2 participants