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.37k stars 1.12k forks source link

possible improvement: Blog Archives could be cached #8739

Open MatteoPiovanelli-Laser opened 10 months ago

MatteoPiovanelli-Laser commented 10 months ago

https://github.com/OrchardCMS/Orchard/blob/9644ceda1f9b077d44aaa4ffaab103c19a59ddba/src/Orchard.Web/Modules/Orchard.Blogs/Services/BlogPostService.cs#L86

In the current implementation, the query for the archive data is performed on every invocation of the method. For long-lived blogs this query fetches thousands of records from the database every time it's performed, and I don't think there's any need for that.

I propose it would be beneficial to cache its result, evicting those whenever a change to the archives is made (the methods invoked for that are in the BlogPartArchiveHandler). Bonus, the "choice" between caching or not this could be controlled by a flag injectable from config file, so it's possible to decide about the tradeoff between doing the query more often vs storing stuff in memory on each tenant.

sebastienros commented 10 months ago

Can it be done at the UI layer instead? This way you can configure it in the view.

MatteoPiovanelli-Laser commented 10 months ago

I'm not sure how simple that would be. As is, we can of course already cache the pages where archives are displayed. The performance hit arises where the archive is displayed on different pages, perhaps with authenticated/customized bits in them: a donut cache would alleviate this.

Perhaps you meant whether the flag "should we cache archivedata" could be put in (admin) UI? In that case, yes, of course.

sebastienros commented 9 months ago

Doesn't Orchard 1 have that? We have that in Orchard Core, maybe something to migrate. Note that it shouldn't be hard to implement. See the Distributed Cache Tag helpers in dotnet core. It is really about have some sort of wrapper in Razor that will either render its content or get it from a cache. Then you can add a custom field that contains something that varies the cache entry (per user, per role, per locale) and some TTL (sliding or not, to invalidate the entry automatically. And there should always be a default TTL value such that entries eventually die).

Orchard Core's feature adds the notion of russian dolls eviction, such that if a cached fragment renders another cached fragment, it evicts the outer elements too. Not hard to do either.

MatteoPiovanelli-Laser commented 9 months ago

donut caching isn't a feature here, no. I think IDeliverable had open sourced a module for that. We fiddled with it a while ago, but I don't think we are really using it anywhere right now because it needed some rethinking of how things were set up in it, and we didn't have the bandwidth to work on it.