Improve ShapeTableManager performance #15620
Conversation
| { | ||
| var strategyFeature = _typeFeatureProvider.GetFeatureForDependency(bindingStrategy.GetType()); | ||
| var hostingEnvironment = _serviceProvider.GetRequiredService<IHostEnvironment>(); | ||
| var bindingStrategies = _serviceProvider.GetRequiredService<IEnumerable<IShapeTableProvider>>(); |
There was a problem hiding this comment.
This one specifically creates a list of IShapeTableProvider which themselves have other collections resolved. And this was done multiple times per request since it was transient. Now the collections are materialized (with resolved elements) only when the shape is actually built.
|
|
||
| HashSet<string> excludedFeatures; | ||
| return BuildShapeTableAsync(themeId); |
There was a problem hiding this comment.
Extracting the async path allows dotnet to optimize the call for the most common case which is when we read the shape table from cache.
| private readonly ITypeFeatureProvider _typeFeatureProvider; | ||
| private readonly IMemoryCache _memoryCache; | ||
| // Singleton cache to hold a tenant's theme ShapeTable | ||
| private readonly Dictionary<string, ShapeTable> _shapeTableCache; |
There was a problem hiding this comment.
Using a singleton dictionary as the cache, like IMemoryCache, with the difference that it is indexed by something we don't need to make unique with a prefix since it is only used here.
The dependency is keyed just in case some other component might want to also register the same type.
sebastienros
force-pushed
the
sebros/shapetable
branch
from
7835a84
to
b4bc8cb
Compare
| @@ -1,3 +1,4 @@ | |||
| using DocumentFormat.OpenXml.Office2016.Drawing.ChartDrawing; | |||
There was a problem hiding this comment.
It seems this has been added accidentally?!!
test/OrchardCore.Tests/DisplayManagement/Decriptors/DefaultShapeTableManagerTests.cs
Outdated
Show resolved
Hide resolved
| return _extensionManager | ||
| .GetFeatureDependencies(themeId) | ||
| .Any(f => f.Id == themeFeatureId); | ||
| bool IsBaseTheme(string themeFeatureId, string themeId) |
There was a problem hiding this comment.
I don't think it's right to use it as a local function, but no problem while you make IsModuleOrRequestedTheme static
| )) | ||
| .ToList(); | ||
| var enabledAndOrderedFeatureIds = (await shellFeaturesManager.GetEnabledFeaturesAsync()) | ||
| .Select(f => f.Id) |
There was a problem hiding this comment.
| .Select(f => f.Id) | |
| .OrderBy(f => f.Id) |
There was a problem hiding this comment.
Why change the previous code?
There was a problem hiding this comment.
I'm not sure how these Ids are ordered without OrderBy?
…peTableManagerTests.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
|
Hi @sebastienros ,Is this issue realted to this PR #15715 ？ |
No description provided.