possible improvement: Blog Archives could be cached

[MatteoPiovanelli]

Orchard/src/Orchard.Web/Modules/Orchard.Blogs/Services/BlogPostService.cs

Line 86 in 9644ced

public IEnumerable<KeyValuePair<ArchiveData, int>> GetArchives(BlogPart blogPart) {

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]

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

[MatteoPiovanelli]

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]

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]

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.