ElementalArea gridfield performance issues

[mfendeksilverstripe]

Version: ^3.0.0 (5334f5709878a3502e1cee431c58a28c36f9a843)

It seems that elemental area gridfield does not scale well with the number of blocks contained inside it. I tested this on a page with 56 blocks. Here are the results:

• 28.4s CMS page load with ElementalArea field removed
• 51.6s CMS page load with ElementalArea field removed

That's quite a difference. It's probably worth investigating why is the difference so large.

[robbieaverill]

For reference, I'm testing this (in CWP 2.1) on a page with 100 blocks in one ElementalArea.

Response times: 11.2s, 11.5s, 11.18s

It's a long way off what you're reporting, but that could be because you're using container blocks like layout blocks/the list block.

It's still too slow.

[robbieaverill]

Ok so 50% of the request time when loading the CMS page edit form is calling ElementalArea::getOwnerPage() - I've made a PR at #376 to use a cacheable call to do this which reduces that by 50% - a good start.

My baseline is 2.69s with ElementalAreasExtension disabled, and 8.82s with it enabled and the fix in #376 in place. There's still 6.13 seconds to render 100 blocks on a page in the CMS - still some room for improvement.

[robbieaverill]

Ok I've tried a few other things:

• Partial caching on the editor preview's icon
• Internal instance caching for:
  • BaseElement::getPage()
  • ElementalArea::getOwnerPage()
  • BaseElement::getAreaRelationName()
  • BaseElement::CMSEditLink()

None of these have made a very significant change in response time at all.

The bulk of the remaining time is in BaseElement::getEditorPreview(), which generates the GridField UI for each element in the CMS (icon, title, type, summary). All of this is unique to each element, and aside from trying to cache the icon I don't think we can do much more with it.

My last profiling shows the following notable changes:

• BaseElement::getPage(): 606 calls -> 404 (drop in 300ms)
• BaseElement::Parent(): 909 calls -> 202 calls (drop in 14ms)

Profiles:

• With versioned caching
• With versioned caching and internal property caching
• Baseline
• Baseline to latest PR changes comparison

Most relevant:

I can make a PR for the getPage() calls being cached, 300ms is 300ms... everything else is irrelevant. Edit: pushed into #376

@mfendeksilverstripe thoughts?

FYI I actually have 101 blocks on this page, not 100 😉

[mfendeksilverstripe]

@robbieaverill Nice work! I see you PR is already merged. I will pull the latest elemental version and will test it out. Many thanks :)

[robbieaverill]

Ok thank you!

[mfendeksilverstripe]

@robbieaverill Tried to test it out. I upgraded to 3.0.x-dev (0e3c17aa866b7d954fdd369f9c5681a4e95956a3), however I couldn't get the page loaded in CMS because of this issue:

[Emergency] Uncaught SilverStripe\ORM\Connect\DatabaseException: Couldn't run query:

SELECT DISTINCT "SiteTree"."ClassName", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."LastEdited" ELSE "SiteTree"."LastEdited" END AS "LastEdited", CASE WHEN
"SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."Created"
ELSE "SiteTree"."Created" END AS "Created", "SiteTree"."Priority", "SiteTree"."HideFromExternalSearch",
CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."NextReviewDate"
ELSE "SiteTree"."NextReviewDate" END AS "NextReviewDate", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."PublishOnDate" ELSE "SiteTree"."PublishOnDate" END AS "PublishOnDate", CASE WHEN
"SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."UnPublishOnDate" ELSE
"SiteTree"."UnPublishOnDate" END AS "UnPublishOnDate", "SiteTree"."CanViewType", "SiteTree"."CanEditType", "SiteTree"."Version",
CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."URLSegment" ELSE
"SiteTree"."URLSegment" END AS "URLSegment", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."Title" ELSE "SiteTree"."Title" END AS "Title", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."MenuTitle" ELSE "SiteTree"."MenuTitle" END AS "MenuTitle", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."Content" ELSE "SiteTree"."Content" END AS "Content", CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN
"SiteTree_Localised_en_NZ"."MetaDescription" ELSE "SiteTree"."MetaDescription" END AS "MetaDescription",
"SiteTree"."ExtraMeta", "SiteTree"."ShowInMenus", "SiteTree"."ShowInSearch", "SiteTree"."Sort", "SiteTree"."HasBrokenFile", "SiteTree"."HasBrokenLink", CASE WHEN
"SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."ReportClass" ELSE "SiteTree"."ReportClass" END AS "ReportClass",
CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."PublishJobID" ELSE "SiteTree"."PublishJobID" END AS "PublishJobID",
CASE WHEN "SiteTree_Localised_en_NZ"."ID" IS NOT NULL THEN "SiteTree_Localised_en_NZ"."UnPublishJobID"
ELSE "SiteTree"."UnPublishJobID" END AS "UnPublishJobID", "SiteTree"."ParentID", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN
"Page_Localised_en_NZ"."DisplayOnSite" ELSE "Page"."DisplayOnSite" END AS "DisplayOnSite", "Page"."Key", "Page"."Latitude", "Page"."Longitude",
"Page"."MapEnabled", "Page"."MapSort", "Page"."MapZoom", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."MetaTitle" ELSE
"Page"."MetaTitle" END AS "MetaTitle", "Page"."OID", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."OpenGraphTitle" ELSE
"Page"."OpenGraphTitle" END AS "OpenGraphTitle", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."OpenGraphDescription" ELSE
"Page"."OpenGraphDescription" END AS "OpenGraphDescription", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."OpenGraphUrl" ELSE
"Page"."OpenGraphUrl" END AS "OpenGraphUrl", "Page"."ParentArticleOID", CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN
"Page_Localised_en_NZ"."PrimaryImageOID" ELSE "Page"."PrimaryImageOID" END AS "PrimaryImageOID", "Page"."GeographicLocationCityID", "Page"."GeographicLocationRegionID",
CASE WHEN "Page_Localised_en_NZ"."ID" IS NOT NULL THEN "Page_Localised_en_NZ"."PrimaryImageID" ELSE
"Page"."PrimaryImageID" END AS "PrimaryImageID", "Page"."RegionQueuedJobDescriptorID", "BlockPage"."ArticleTypeID", CASE WHEN
"BlockPage_Localised_en_NZ"."ID" IS NOT NULL THEN "BlockPage_Localised_en_NZ"."ElementalAreaID" ELSE "BlockPage"."ElementalAreaID" END AS "ElementalAreaID",
"BlockPage"."ArticleLayoutQueuedJobDescriptorID", "TopicPage"."PrimaryTaxonomyTermOID", "TopicPage"."SecondaryTaxonomyTermOID", "TopicPage"."PrimaryTaxonomyTermID",
"TopicPage"."SecondaryTaxonomyTermID", "SiteTree"."ID", CASE WHEN "SiteTree"."ClassName" IS NOT NULL THEN "SiteTree"."ClassName"
ELSE 'SilverStripe\\CMS\\Model\\SiteTree' END AS "RecordClassName", 'en_NZ' AS "Locale", CASE WHEN "BlockPage_Localised_en_NZ"."ID" IS NOT NULL THEN 'en_NZ'
ELSE NULL END AS "SourceLocale" 

FROM "SiteTree" LEFT JOIN "Page" ON "Page"."ID" = "SiteTree"."ID" LEFT JOIN "BlockPage" ON "BlockPage"."ID" = "SiteTree"."ID" LEFT JOIN
"TopicPage" ON "TopicPage"."ID" = "SiteTree"."ID" LEFT JOIN "SiteTree_Localised" AS "SiteTree_Localised_en_NZ" ON "SiteTree"."ID" = "SiteTree_Localised_en_NZ"."RecordID"
AND "SiteTree_Localised_en_NZ"."Locale" = ? LEFT JOIN "Page_Localised" AS "Page_Localised_en_NZ" ON "Page"."ID" = "Page_Localised_en_NZ"."RecordID"
AND "Page_Localised_en_NZ"."Locale" = ? LEFT JOIN "BlockPage_Localised" AS "BlockPage_Localised_en_NZ" ON "BlockPage"."ID" = "BlockPage_Localised_en_NZ"."RecordID"
AND "BlockPage_Localised_en_NZ"."Locale" = ? 

WHERE ("ElementalAreaID" = 534770) AND ("SiteTree"."ClassName" IN (?, ?, ?, ?))

ORDER BY "SiteTree"."Sort" ASC LIMIT 1

23000-1052: Column 'ElementalAreaID' in where clause is ambiguous


GET /admin/pages/edit/show/23
Line 64 in /var/www/mysite/www/vendor/silverstripe/framework/src/ORM/Connect/DBConnector.php

To be fair, I don't think this is related to your changeset, but I suspect something changed in Elemental code because this is more about Fluent and Elemental working together.

It would be great if we could either push your performance improvement to 3.0.0 release branch or if someone could look at this issue as a priority because we will need to have Fluent and Elemental modules working together in the future too.

[mfendeksilverstripe]

@chillu Since the issue is related to Elemental and Fluent modules working together, I'm not sure who should take lead on tis. Any ideas?

[mfendeksilverstripe]

@robbieaverill @chillu

I did some further investigation into the query issue. The problem is how the page gets loaded from the database. Also, the query issue was introduced in Robbie's code.

I made following changes which remedies the query issue:

silverstripe-terraformers@dc42675

I was finally able to test the actual performance improvements :). My test page load went from 50s down to 32s. Nice work!

[ScopeyNZ]

Thanks @mfendeksilverstripe for the real-world example testing. Using get_one_by_stage should hopefully be a further performance improvement as DataObject::get_one is cached by default.

I might get a chance to look into it a bit this afternoon - if not I guess @robbieaverill will be able to come up with a solution.

[mfendeksilverstripe]

@ScopeyNZ We had a little discussion about this issue within the team. Our guess is that the WHERE clause doesn't get picked up by the Fluent module in case of get_one_by_stage as it's passed as a string. This is different in get_by_stage where the values are passed via the filter($areaID, $this->ID).

This issue may be completely unrelated to elemental - it can be related to Core and Fluent interaction.

it's worth noting that if we currently upgrade to latest elemental, it will break our project.

[ScopeyNZ]

Yes I think I've seen this issue with get_one_by_stage before - I think I should be able to find an example where it has been fixed while still using get_one_by_stage.

If you're talking about elemental master branch - this is in heavy development and is very unstable right now. The team is currently working on adding in-line editing to elemental (for elemental 4.0).

[robbieaverill]

Oh I see, we need to add the model name prefix - my fault for being lazy =(

[robbieaverill]

@mfendeksilverstripe #377 should fix it, sorry about that

[ScopeyNZ]

@robbieaverill Do you think there's more to action on in this issue? If not - feel free to close it. PRs have been merged 🙂

[robbieaverill]

I think we've mostly done all that we can. I think that #347 would help too, but we've squeezed most of the low hanging fruit out of this ticket now. @mfendeksilverstripe feel free to reopen the issue if you think there's more that can be achieved here.

[mfendeksilverstripe]

Thanks for the help all. The performance improvement is really helpful.

[chillu]

@mfendeksilverstripe How much is this improving performance for you? Are you still dependant on the RFC: Restructure data models from Robbie?

[chillu]

@robbieaverill @unclecheese Has either of you tested this on the codebase @mfendeksilverstripe is working on? Has it fixed the 30-50s load times which he described in the issue originally?

[mfendeksilverstripe]

@chillu My test page load went from 50s down to 32s so it was definitely a big improvement. There is probably still room for improvement, though. Not sure how much of that is located in the Elemental module.

[chillu]

OK, reopening - 32s is still bad.

[robbieaverill]

I noted that these changes were shaving off pretty much all that could be done easily. I think the remaining time could be a combination of things - CMS, versioned, fluent, subsites, elemental, etc etc. We can reinvestigate this anyway though and see where improvements can be made.

cc @brynwhyman

[chillu]

The thing is that "edit forms are slow" is tracked through this issue for the original reporter. And that is not fixed. If there's diminishing returns on elemental, then we need to identify other means to research, and highlight them to the reporter. But we can't have it drop off the radar completely. Maybe it'd be more efficient to have private issues with the highlevel problem statement in an area ("edit forms are slow"), and then branch out to specific issues. Note that we already have a "cms performance" epic on the Open Sourcerers board: https://app.zenhub.com/workspace/o/silverstripe/silverstripe-admin/issues/571

[robbieaverill]

We have made a few caching PRs, including a piece of pseudo-wizardry from @unclecheese which optimistically caches elemental relationships to speed all of this up. Everything has been merged, but I'll leave this open for @mfendeksilverstripe and his team to validate.

[robbieaverill]

Fixed in the project code