OrchardCMS/Orchard

Perf: GetMany doesn't use ContentManagerSession

MatteoPiovanelli-Laser opened this issue · 7 comments

public IEnumerable<T> GetMany<T>(IEnumerable<int> ids, VersionOptions options, QueryHints hints) where T : class, IContent {

I was checking the performances of some test pages. In one of them I have a bunch of MediaLibraryPickerFields. Since this is only a test tenant on a dev box, all those fields have the same Media items selected.

I noticed that the query corresponding to the LazyField fetching the information for the Media from the db (

localField._contentItems = new Lazy<IEnumerable<MediaPart>>(() => _contentManager.GetMany<MediaPart>(localField.Ids, VersionOptions.Published, QueryHints.Empty).ToList());
) is fired for every field, always with the same parameters.

This is unlike fetching ContentItems one by one using the Get method, where the ContentManagerSession prevents refiring the same query multiple times.

Looking at the code, I think ContentPickerFields have the same issue.

I think GetMany should only be doing the query if any of the Ids provided to it corresponds to a ContentItem that IS NOT available in ContentManagerSession. It should still, of course, not change the order of returned items.

I think GetMany should only be doing the query if any of the Ids provided to it corresponds to a ContentItem that IS NOT available in ContentManagerSession

Correct. I am pretty sure we do that in OC.

And that's important for eager loading techniques (pre-loading a bunch of items knowing that there will be a SELECT N+1 to come)

Further testing: TaxonomyField seems to be affected by the same issue.

Has anyone looked more into this yet maybe with a PoC change? I would love to have this fixed!

I haven't had a chance yet. Hopefully over the next few weeks.

I did some test during a couple meetings.
I think I have a working version, but I'm not sure about its performances.
I'll draft a PR early next week.

Sounds great! I'll test it with an application that uses Taxonomies extensively once you've got the PR submitted.