nystudio107 / craft-seomatic

SEOmatic facilitates modern SEO best practices & implementation for Craft CMS 3. It is a turnkey SEO system that is comprehensive, powerful, and flexible.
https://nystudio107.com/plugins/seomatic
Other
166 stars 71 forks source link

Slow response from plugin #1494

Closed ghost closed 4 months ago

ghost commented 4 months ago

Describe the bug

I have tested my site on dev and production with different caching mechanisms but I am unable to workout why the plugin is struggling to respond quickly.

I have found a few blog pages on optimising SEOmatic and have found similar sounding issues:

These appear to be resolved so I'm not sure if there is a regression or if this is a new issue with my specific site.

The performance profiling section on homepage of the site appears as follows:

Time Duration Category Info
17:23:00.540 349.1 ms nystudio107\seomatic\services\MetaContainers::loadMetaContainers MetaContainers::loadMetaContainers
17:23:00.708 180.8 ms nystudio107\seomatic\helpers\DynamicMeta::addDynamicMetaToContainers →DynamicMeta::addDynamicMetaToContainers
17:23:00.595 110.7 ms nystudio107\seomatic\services\MetaContainers::loadFieldMetaContainers →MetaContainers::loadFieldMetaContainers
17:23:00.708 104.2 ms nystudio107\seomatic\helpers\DynamicMeta::addMetaJsonLdBreadCrumbs →→DynamicMeta::addMetaJsonLdBreadCrumbs
17:23:00.812 75.6 ms nystudio107\seomatic\helpers\DynamicMeta::addMetaLinkHrefLang →→DynamicMeta::addMetaLinkHrefLang
17:23:00.812 74.6 ms nystudio107\seomatic\helpers\DynamicMeta::getLocalizedUrls →→→DynamicMeta::getLocalizedUrls
17:23:02.022 42.5 ms nystudio107\seomatic\services\MetaContainers::includeMetaContainers MetaContainers::includeMetaContainers
17:23:00.560 18.9 ms nystudio107\seomatic\services\MetaContainers::loadGlobalMetaContainers →MetaContainers::loadGlobalMetaContainers

I have created a new section within Craft and used blank twig template other than head() and endBody() which has no content fields assigned to it and it appears as the following:

Time Duration Category Info
17:08:37.565 317.2 ms nystudio107\seomatic\services\MetaContainers::includeMetaContainers MetaContainers::includeMetaContainers
17:08:37.316 230.9 ms nystudio107\seomatic\services\MetaContainers::loadMetaContainers MetaContainers::loadMetaContainers
17:08:37.410 136.8 ms nystudio107\seomatic\helpers\DynamicMeta::addDynamicMetaToContainers →DynamicMeta::addDynamicMetaToContainers
17:08:37.609 109.9 ms nystudio107\seomatic\models\MetaTagContainer::includeMetaData →MetaTagContainer::includeMetaData
17:08:37.410 100.3 ms nystudio107\seomatic\helpers\DynamicMeta::addMetaJsonLdBreadCrumbs →→DynamicMeta::addMetaJsonLdBreadCrumbs
17:08:37.719 71.8 ms nystudio107\seomatic\models\MetaTagContainer::includeMetaData →MetaTagContainer::includeMetaData
17:08:37.339 52.6 ms nystudio107\seomatic\services\MetaContainers::loadGlobalMetaContainers →MetaContainers::loadGlobalMetaContainers
17:08:37.835 41.1 ms nystudio107\seomatic\models\MetaJsonLdContainer::includeMetaData →MetaJsonLdContainer::includeMetaData
17:08:37.794 36.2 ms nystudio107\seomatic\models\MetaLinkContainer::includeMetaData →MetaLinkContainer::includeMetaData
17:08:37.511 36.2 ms nystudio107\seomatic\helpers\DynamicMeta::addMetaLinkHrefLang →→DynamicMeta::addMetaLinkHrefLang
17:08:37.511 35.9 ms nystudio107\seomatic\helpers\DynamicMeta::getLocalizedUrls →→→DynamicMeta::getLocalizedUrls
17:08:37.585 24.5 ms nystudio107\seomatic\models\MetaTagContainer::includeMetaData →MetaTagContainer::includeMetaData

With the plugin disabled this new page changes from a 1s page load to 600ms which is still not amazing but nothing shows on the Yii timeline page as lasting longer than 0ms. I know there is a lot of DB queries being made on this site.

Further, using the "Social Media Preview" view and dev tools the page loads in 2.3s - 3.0s. The default image is not transformed either it is just a static asset. I did see on a previous issue that volumes / transforms may be a factor but I cannot see signs of this with the typical page generation.

Sorry for the flood of info, hope it provides something to go on. The load times are from my local machine but issues are consistent with the production server. The plugin load times are very similar with nystudio107\seomatic\services\MetaContainers::loadMetaContainers taking 335.4 ms.

Screenshots

Screenshot 2024-07-16 at 17 45 52

Versions

PHP: 8.2.20 PHP memory limit: 1G DB: MySQL 8.0.37

/cc @WilliamIsted

dlindberg commented 4 months ago

Pretty sure I'm seeing the same issue in Craft 5.2.8 with SEOmatic 5.1.0. Whatever bug this is it makes entries functionally un-savable on our staging environment (running Servd Pro), back tracking to 5.0.4 also seems non-trivial, giving an error: Setting unknown property: nystudio107\seomatic\models\MetaSitemapVars::sitemapPageSize when attempting to pin 5.04. in composer.

dlindberg commented 4 months ago

Figured out how to walk the update back, it is a significant pain. You need to:

You will probably want to do all that in a non production environment, several parts of the site will become nonfunctional while you do this, then replace production with the results.

khalwat commented 4 months ago

@WilliamIstedMG 349ms doesn't sound unfathomably slow to me for an uncached route; have you tested subsequent routes NOT in local dev? Because in local dev, the caches only last 60 seconds, and will generally be slower due to debug information, etc.

Also the SEOmatic debug toolbar pane itself adds overhead; you can disable it in Plugin Settings if you like.

If you're doing performance profiling, you can keep the Yii2 Debug Toolbar enabled, but disable the SEOmatic panel via Plugin Settings → Advanced → SEOmatic Debug Toolbar Panel.

https://nystudio107.com/docs/seomatic/advanced.html#debug-toolbar

I'm going to close this issue because I think this is expected performance in local dev with the SEOmatic debug toolbar pane enabled.

If you are seeing performance issues in production without the SEOmatic debug toolbar pane enabled, then I can re-open it and we can try to figure out what's going on here.

khalwat commented 4 months ago

@dlindberg I could be wrong, but I don't think your issue is the same as this one.

I'm unable to replicate a performance issue related to the 5.1.0 update (which largely had to do with sitemaps, and is unrelated to saving entries).

Are you saying that downgrading to the previous version then saw the performance problem go away? What if you then upgrade to 5.1.0 again in local dev, does it re-appear?

Feel free to open a new issue if you like, and we can try to figure out what's going on.

dlindberg commented 4 months ago

@khalwat Yes, downgrading to 5.0.4 resolves the problem, upgrading again to 5.1.0 does re-introduce it. Saves will complete in time for me on local dev, but pushing it into staging and you can't save anything, it will either time out or throw a DB lock error. Not certain what is different on the on save event, but it does seem to be trying to do too much. And that is with the 5.2.8 update.

dlindberg commented 4 months ago

note that the primary effected sections have between 1,000 entries and 6,000 entries which seems reasonable.

khalwat commented 4 months ago

@dlindberg Ironically, with the sitemap changes, it should be doing even less on save. It does almost nothing now on save.

Your issue, whatever it is, ie entirely separate from the issue filed by @WilliamIstedMG (which is I think things working as expected, he just needs to disable the SEOmatic debug toolbar panel before doing performance timings).

Any tips you can give me to help reproduce would be great, I've tested just going in and modifying entries and saving them, and am not seeing any additional time added to this happening.

khalwat commented 4 months ago

The number of entries in the section should be irrelevant, all SEOmatic does when you save an entry is it invalidates a cached sitemaps for that item. It doesn't even try to regenerate it.

Are you using Blitz or something else that might be at play here? Or can give me steps to reproduce?

khalwat commented 4 months ago

@dlindberg my apologies, by doing a code review, I think I have found the problem, and it is indeed a regression of sorts.

invalidateSitemapCache() still has vestiges of the old code in it, so it does this:

        $sitemapTemplate = SitemapTemplate::create();
        $xml = $sitemapTemplate->render(
            [
                'groupId' => $groupId,
                'type' => $type,
                'handle' => $handle,
                'siteId' => $siteId,
                'throwException' => false,
            ]
        );

...so when you save an entry, it's trying to regenerate the sitemap for it. Which it should no longer be doing, it should just be invalidating the cache.

And I also had to add this tag:

                    'tags' => [
                        self::GLOBAL_SITEMAP_CACHE_TAG,
                        self::SITEMAP_CACHE_TAG . $handle . $siteId,
                        self::SITEMAP_CACHE_TAG . $handle . $siteId . $pageCacheSuffix,
                    ],

...so that it would be able to invalidate the caches for all of the paginated sitemaps.

khalwat commented 4 months ago

@dlindberg your issue was indeed something else entirely, and has been addressed in the above commits.

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop as 3.5.1”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop-v4 as 4.1.1”,

Then do a composer clear-cache && composer update

…..

Craft CMS 5:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop-v5 as 5.1.1”,

Then do a composer clear-cache && composer update

khalwat commented 4 months ago

Released as SEOmatic 3.5.1, 4.1.1, and 5.1.1 for Craft 3, 4, & 5 respectively

dlindberg commented 4 months ago

Thanks, this does indeed solve the issue.