OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

Projections for content types with Taxonomy Field (performance issue) #8601

Open mwentz opened 2 years ago

mwentz commented 2 years ago

I have a content type with an attached shared part that consists of two Taxonomy fields (each taxonomy has 30-40 items). I created a query of all the items in this content type and added it as a projection element in a layout on a page. I'm seeing three specific places where it's causing slow loads:

I have tried this with ~50 and ~140 content items in the projections, the more items I add the longer it takes (7-10 seconds for 50 and 28-34 seconds for 140) for the page to load.

I tried to use mini profiler to confirm exactly where the slowness is happening, but it's acting up on me and I can't get a good read. I tried removing the shared part with the two Taxonomy fields from the content type and I saw a large immediate improvement in performance that brought it back in line with other projections with more simple content types.

mwentz commented 2 years ago

I came across this issue #8370 and tested out the related pull request #8371. This change resulted in a large performance improvement (down to 7-10 seconds for 140 content items). It looks like there might still be a performance problem with taxonomy fields on a content item in a projection and that it was being compounded by that other issue.

EDIT: On further testing I am not seeing these performance improvements. I must have been seeing some sort of caching, because further testing is back to 30+ second load times.

MatteoPiovanelli-Laser commented 2 years ago

What version of Orchard are you running?

Just to make sure the problem is with how the projection is handled with the TaxonomyFields, did you test whether a contentItem that has a projection of those is just as slow as the one with the projection element? Just making sure the issue isn't somewhere with how layouts are interacting with the thing, because I don't use layouts much (or at all really).

If you were running 1.10.x, it would be useful, I think, if you could export the contents and their definition (as long as there is nothing sensitive or proprietary) to have a minimal case to test.

MatteoPiovanelli-Laser commented 2 years ago

On the TaxonomyField side oth things, I recommend looking at how many times these lines are fired

https://github.com/OrchardCMS/Orchard/blob/c38e6814a90bc6a999334c3e3ae06028eb8a623a/src/Orchard.Web/Modules/Orchard.Taxonomies/Handlers/TermsPartHandler.cs#L68

https://github.com/OrchardCMS/Orchard/blob/c38e6814a90bc6a999334c3e3ae06028eb8a623a/src/Orchard.Web/Modules/Orchard.Taxonomies/Handlers/TermsPartHandler.cs#L76

Those may be ripe for optimization, but not trivial.

mwentz commented 2 years ago

Thank you for looking at this, and my apologies for not including version number. The site is running Orchard 1.10.3.

I tested this projection as a Projection content type as well as a page with a projection widget and I am still seeing very large load times. Also note that in my first comment I said I saw a reduced load time with the change from another ticket. Since a couple first tests, I have not seen these improvements - I am seeing 30+ second load times in all cases.

Adding break points on those lines you mentioned I see the second point being hit about 200 times when loading about 80 content items.

MatteoPiovanelli-Laser commented 2 years ago

https://github.com/OrchardCMS/Orchard/blob/c38e6814a90bc6a999334c3e3ae06028eb8a623a/src/Orchard.Web/Modules/Orchard.Taxonomies/Shapes.cs#L80

could you check whether it's going through there next?

I don't have a chance to recreate your conditions and test this, but from a quick browse through the code that stands out as a likely place where the Loader() method I highlighted earlier might be invoked. If it's going there, you may wish to try and comment out the whole method that loop is in to see how that affects your performances. (getting rid of the method would really be the fix, but if we see a big improvement, if would point us in the right direction)

mwentz commented 2 years ago

@MatteoPiovanelli-Laser I sanitized and simplified an export which I'm attaching (I changed the extension from .xml to .txt so that it could be uploaded). Steps to reproduce:

I'm seeing a page load of 10+ seconds in localhost.

blog-export.txt