Improve ShapeTableManager performance

src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/DefaultShapeTableManager.cs

@@ -3,7 +3,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
-using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 using OrchardCore.DisplayManagement.Extensions;
@@ -19,109 +19,119 @@ namespace OrchardCore.DisplayManagement.Descriptors
     /// </summary>
     public class DefaultShapeTableManager : IShapeTableManager
     {
+        private const string DefaultThemeIdKey = "_ShapeTable";
+
+        // FeatureShapeDescriptors are identical across tenants so they can be reused statically. Each shape table will
+        // create a unique list of these per tenant.
         private static readonly ConcurrentDictionary<string, FeatureShapeDescriptor> _shapeDescriptors = new();
+
         private static readonly object _syncLock = new();
 
-        private readonly IHostEnvironment _hostingEnvironment;
-        private readonly IEnumerable<IShapeTableProvider> _bindingStrategies;
-        private readonly IShellFeaturesManager _shellFeaturesManager;
-        private readonly IExtensionManager _extensionManager;
-        private readonly ITypeFeatureProvider _typeFeatureProvider;
-        private readonly IMemoryCache _memoryCache;
+        // Singleton cache to hold a tenant's theme ShapeTable 
+        private readonly Dictionary<string, ShapeTable> _shapeTableCache;
+
+        private readonly IServiceProvider _serviceProvider;
         private readonly ILogger _logger;
 
         public DefaultShapeTableManager(
-            IHostEnvironment hostingEnvironment,
-            IEnumerable<IShapeTableProvider> bindingStrategies,
-            IShellFeaturesManager shellFeaturesManager,
-            IExtensionManager extensionManager,
-            ITypeFeatureProvider typeFeatureProvider,
-            IMemoryCache memoryCache,
+            [FromKeyedServices(nameof(DefaultShapeTableManager))] Dictionary<string, ShapeTable> shapeTableCache,
+            IServiceProvider serviceProvider,
             ILogger<DefaultShapeTableManager> logger)
         {
-            _hostingEnvironment = hostingEnvironment;
-            _bindingStrategies = bindingStrategies;
-            _shellFeaturesManager = shellFeaturesManager;
-            _extensionManager = extensionManager;
-            _typeFeatureProvider = typeFeatureProvider;
-            _memoryCache = memoryCache;
+            _shapeTableCache = shapeTableCache;
+            _serviceProvider = serviceProvider;
             _logger = logger;
         }
 
-        public ShapeTable GetShapeTable(string themeId)
-            => GetShapeTableAsync(themeId).GetAwaiter().GetResult();
-
-        public async Task<ShapeTable> GetShapeTableAsync(string themeId)
+        public Task<ShapeTable> GetShapeTableAsync(string themeId)
         {
-            var cacheKey = $"ShapeTable:{themeId}";
+            // This method is intentionally kept non-async since most calls
+            // are from cache.
 
-            if (!_memoryCache.TryGetValue(cacheKey, out ShapeTable shapeTable))
+            if (_shapeTableCache.TryGetValue(themeId ?? DefaultThemeIdKey, out var shapeTable))
             {
-                _logger.LogInformation("Start building shape table");
+                return Task.FromResult(shapeTable);
+            }
 
-                HashSet<string> excludedFeatures;
+            return BuildShapeTableAsync(themeId);
+        }
 
-                // Here we don't use a lock for thread safety but for atomicity.
-                lock (_syncLock)
-                {
-                    excludedFeatures = new HashSet<string>(_shapeDescriptors.Select(kv => kv.Value.Feature.Id));
-                }
+        private async Task<ShapeTable> BuildShapeTableAsync(string themeId)
+        {
+            _logger.LogInformation("Start building shape table");
 
-                var shapeDescriptors = new Dictionary<string, FeatureShapeDescriptor>();
+            // These services are resolved lazily since they are only required when initializing the shape tables
+            // during the first request. And binding strategies would be expensive to build since this service is called many times
+            // per request.
 
-                foreach (var bindingStrategy in _bindingStrategies)
-                {
-                    var strategyFeature = _typeFeatureProvider.GetFeatureForDependency(bindingStrategy.GetType());
+            var hostingEnvironment = _serviceProvider.GetRequiredService<IHostEnvironment>();
+            var bindingStrategies = _serviceProvider.GetRequiredService<IEnumerable<IShapeTableProvider>>();
+            var shellFeaturesManager = _serviceProvider.GetRequiredService<IShellFeaturesManager>();
+            var extensionManager = _serviceProvider.GetRequiredService<IExtensionManager>();
+            var typeFeatureProvider = _serviceProvider.GetRequiredService<ITypeFeatureProvider>();
 
-                    var builder = new ShapeTableBuilder(strategyFeature, excludedFeatures);
-                    await bindingStrategy.DiscoverAsync(builder);
-                    var builtAlterations = builder.BuildAlterations();
+            HashSet<string> excludedFeatures;
 
-                    BuildDescriptors(bindingStrategy, builtAlterations, shapeDescriptors);
-                }
+            // Here we don't use a lock for thread safety but for atomicity.
+            lock (_syncLock)
+            {
+                excludedFeatures = new HashSet<string>(_shapeDescriptors.Select(kv => kv.Value.Feature.Id));
+            }
 
-                // Here we don't use a lock for thread safety but for atomicity.
-                lock (_syncLock)
-                {
-                    foreach (var kv in shapeDescriptors)
-                    {
-                        _shapeDescriptors[kv.Key] = kv.Value;
-                    }
-                }
+            var shapeDescriptors = new Dictionary<string, FeatureShapeDescriptor>();
 
-                var enabledAndOrderedFeatureIds = (await _shellFeaturesManager.GetEnabledFeaturesAsync())
-                    .Select(f => f.Id)
-                    .ToList();
+            foreach (var bindingStrategy in bindingStrategies)
+            {
+                var strategyFeature = typeFeatureProvider.GetFeatureForDependency(bindingStrategy.GetType());
+
+                var builder = new ShapeTableBuilder(strategyFeature, excludedFeatures);
+                await bindingStrategy.DiscoverAsync(builder);
+                var builtAlterations = builder.BuildAlterations();
+
+                BuildDescriptors(bindingStrategy, builtAlterations, shapeDescriptors);
+            }
 
-                // let the application acting as a super theme for shapes rendering.
-                if (enabledAndOrderedFeatureIds.Remove(_hostingEnvironment.ApplicationName))
+            // Here we don't use a lock for thread safety but for atomicity.
+            lock (_syncLock)
+            {
+                foreach (var kv in shapeDescriptors)
                 {
-                    enabledAndOrderedFeatureIds.Add(_hostingEnvironment.ApplicationName);
+                    _shapeDescriptors[kv.Key] = kv.Value;
                 }
+            }
 
-                var descriptors = _shapeDescriptors
-                    .Where(sd => enabledAndOrderedFeatureIds.Contains(sd.Value.Feature.Id))
-                    .Where(sd => IsModuleOrRequestedTheme(sd.Value.Feature, themeId))
-                    .OrderBy(sd => enabledAndOrderedFeatureIds.IndexOf(sd.Value.Feature.Id))
-                    .GroupBy(sd => sd.Value.ShapeType, StringComparer.OrdinalIgnoreCase)
-                    .Select(group => new ShapeDescriptorIndex
-                    (
-                        shapeType: group.Key,
-                        alterationKeys: group.Select(kv => kv.Key),
-                        descriptors: _shapeDescriptors
-                    ))
-                    .ToList();
+            var enabledAndOrderedFeatureIds = (await shellFeaturesManager.GetEnabledFeaturesAsync())
+                .Select(f => f.Id)
+                .ToList();
+
+            // let the application acting as a super theme for shapes rendering.
+            if (enabledAndOrderedFeatureIds.Remove(hostingEnvironment.ApplicationName))
+            {
+                enabledAndOrderedFeatureIds.Add(hostingEnvironment.ApplicationName);
+            }
 
-                shapeTable = new ShapeTable
+            var descriptors = _shapeDescriptors
+                .Where(sd => enabledAndOrderedFeatureIds.Contains(sd.Value.Feature.Id))
+                .Where(sd => IsModuleOrRequestedTheme(extensionManager, sd.Value.Feature, themeId))
+                .OrderBy(sd => enabledAndOrderedFeatureIds.IndexOf(sd.Value.Feature.Id))
+                .GroupBy(sd => sd.Value.ShapeType, StringComparer.OrdinalIgnoreCase)
+                .Select(group => new ShapeDescriptorIndex
                 (
-                    descriptors: descriptors.ToDictionary(sd => sd.ShapeType, x => (ShapeDescriptor)x, StringComparer.OrdinalIgnoreCase),
-                    bindings: descriptors.SelectMany(sd => sd.Bindings).ToDictionary(kv => kv.Key, kv => kv.Value, StringComparer.OrdinalIgnoreCase)
-                );
+                    shapeType: group.Key,
+                    alterationKeys: group.Select(kv => kv.Key),
+                    descriptors: _shapeDescriptors
+                ))
+                .ToList();
 
-                _logger.LogInformation("Done building shape table");
+            var shapeTable = new ShapeTable
+            (
+                descriptors: descriptors.ToDictionary(sd => sd.ShapeType, x => (ShapeDescriptor)x, StringComparer.OrdinalIgnoreCase),
+                bindings: descriptors.SelectMany(sd => sd.Bindings).ToDictionary(kv => kv.Key, kv => kv.Value, StringComparer.OrdinalIgnoreCase)
+            );
 
-                _memoryCache.Set(cacheKey, shapeTable, new MemoryCacheEntryOptions { Priority = CacheItemPriority.NeverRemove });
-            }
+            _logger.LogInformation("Done building shape table");
+
+            _shapeTableCache[themeId ?? DefaultThemeIdKey] = shapeTable;
 
             return shapeTable;
         }
@@ -159,7 +169,7 @@ private static void BuildDescriptors(
             }
         }
 
-        private bool IsModuleOrRequestedTheme(IFeatureInfo feature, string themeId)
+        private static bool IsModuleOrRequestedTheme(IExtensionManager extensionManager, IFeatureInfo feature, string themeId)
         {
             if (!feature.IsTheme())
             {
@@ -172,13 +182,13 @@ private bool IsModuleOrRequestedTheme(IFeatureInfo feature, string themeId)
             }
 
             return feature.Id == themeId || IsBaseTheme(feature.Id, themeId);
-        }
 
-        private bool IsBaseTheme(string themeFeatureId, string themeId)
-        {
-            return _extensionManager
-                .GetFeatureDependencies(themeId)
-                .Any(f => f.Id == themeFeatureId);
+            bool IsBaseTheme(string themeFeatureId, string themeId)
+            {
+                return extensionManager
+                    .GetFeatureDependencies(themeId)
+                    .Any(f => f.Id == themeFeatureId);
+            }
         }
     }
 }

src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/IShapeTableManager.cs

@@ -1,12 +1,8 @@
-using System;
 using System.Threading.Tasks;
 
 namespace OrchardCore.DisplayManagement.Descriptors;
 
 public interface IShapeTableManager
 {
-    [Obsolete($"Instead, utilize the {nameof(GetShapeTableAsync)} method. This current method is slated for removal in upcoming releases.")]
-    ShapeTable GetShapeTable(string themeId);
-
     Task<ShapeTable> GetShapeTableAsync(string themeId);
 }

src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs

@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.ApplicationParts;
 using Microsoft.AspNetCore.Mvc.Razor.Compilation;
@@ -57,7 +58,8 @@ public static OrchardCoreBuilder AddTheming(this OrchardCoreBuilder builder)
                     services.AddScoped<IViewLocationExpanderProvider, ThemeViewLocationExpanderProvider>();
 
                     services.AddScoped<IShapeTemplateHarvester, BasicShapeTemplateHarvester>();
-                    services.AddTransient<IShapeTableManager, DefaultShapeTableManager>();
+                    services.AddKeyedSingleton<Dictionary<string, ShapeTable>>(nameof(DefaultShapeTableManager));
+                    services.AddScoped<IShapeTableManager, DefaultShapeTableManager>();
 
                     services.AddScoped<IShapeTableProvider, ShapeAttributeBindingStrategy>();
                     services.AddScoped<IShapeTableProvider, ShapePlacementParsingStrategy>();

test/OrchardCore.Tests/DisplayManagement/Decriptors/DefaultShapeTableManagerTests.cs

@@ -1,3 +1,4 @@
+using DocumentFormat.OpenXml.Office2016.Drawing.ChartDrawing;
 using OrchardCore.DisplayManagement.Descriptors;
 using OrchardCore.DisplayManagement.Extensions;
 using OrchardCore.DisplayManagement.Implementation;
@@ -118,6 +119,7 @@ public DefaultShapeTableManagerTests()
             serviceCollection.AddMemoryCache();
             serviceCollection.AddScoped<IShellFeaturesManager, TestShellFeaturesManager>();
             serviceCollection.AddScoped<IShapeTableManager, DefaultShapeTableManager>();
+            serviceCollection.AddKeyedSingleton<Dictionary<string, ShapeTable>>(nameof(DefaultShapeTableManager));
             serviceCollection.AddSingleton<ITypeFeatureProvider, TypeFeatureProvider>();
             serviceCollection.AddSingleton<IHostEnvironment>(new StubHostingEnvironment());