From 76f65f9df3d1016e1cf5dc240f3c417e826ba9c5 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 11:52:11 -0700 Subject: [PATCH 1/9] Improve ShapeBindingStrategyResolver performance --- .../DefaultContentDefinitionDisplayManager.cs | 3 -- .../AdminTemplatesShapeBindingResolver.cs | 13 +++--- .../Services/TemplatesShapeBindingResolver.cs | 25 +++++------ .../Implementation/DefaultHtmlDisplay.cs | 43 +++++++++++-------- 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.ContentTypes/Editors/DefaultContentDefinitionDisplayManager.cs b/src/OrchardCore.Modules/OrchardCore.ContentTypes/Editors/DefaultContentDefinitionDisplayManager.cs index 680ffbe3a6b..bc7045bc6bd 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentTypes/Editors/DefaultContentDefinitionDisplayManager.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentTypes/Editors/DefaultContentDefinitionDisplayManager.cs @@ -16,7 +16,6 @@ namespace OrchardCore.ContentTypes.Editors public class DefaultContentDefinitionDisplayManager : BaseDisplayManager, IContentDefinitionDisplayManager { private readonly IEnumerable _handlers; - private readonly IShapeTableManager _shapeTableManager; private readonly IContentDefinitionManager _contentDefinitionManager; private readonly IShapeFactory _shapeFactory; private readonly ILayoutAccessor _layoutAccessor; @@ -24,7 +23,6 @@ public class DefaultContentDefinitionDisplayManager : BaseDisplayManager, IConte public DefaultContentDefinitionDisplayManager( IEnumerable handlers, - IShapeTableManager shapeTableManager, IContentDefinitionManager contentDefinitionManager, IShapeFactory shapeFactory, IEnumerable placementProviders, @@ -33,7 +31,6 @@ ILayoutAccessor layoutAccessor ) : base(shapeFactory, placementProviders) { _handlers = handlers; - _shapeTableManager = shapeTableManager; _contentDefinitionManager = contentDefinitionManager; _shapeFactory = shapeFactory; _layoutAccessor = layoutAccessor; diff --git a/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs b/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs index b04c6690c07..af4eb91d3f5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs @@ -17,6 +17,7 @@ public class AdminTemplatesShapeBindingResolver : IShapeBindingResolver private readonly AdminPreviewTemplatesProvider _previewTemplatesProvider; private readonly IHttpContextAccessor _httpContextAccessor; private readonly HtmlEncoder _htmlEncoder; + private bool? isAdmin; public AdminTemplatesShapeBindingResolver( AdminTemplatesManager templatesManager, @@ -34,7 +35,11 @@ public AdminTemplatesShapeBindingResolver( public async Task GetShapeBindingAsync(string shapeType) { - if (!AdminAttribute.IsApplied(_httpContextAccessor.HttpContext)) + // Cache this value since the service is scoped and this method is invoked for every + // alternate of every shape. + isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); + + if (!isAdmin.Value) { return null; } @@ -67,11 +72,7 @@ private ShapeBinding BuildShapeBinding(string shapeType, Template template) { BindingName = shapeType, BindingSource = shapeType, - BindingAsync = async displayContext => - { - var content = await _liquidTemplateManager.RenderHtmlContentAsync(template.Content, _htmlEncoder, displayContext.Value); - return content; - } + BindingAsync = displayContext => _liquidTemplateManager.RenderHtmlContentAsync(template.Content, _htmlEncoder, displayContext.Value) }; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs b/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs index d4788f2009f..92afbec5830 100644 --- a/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs @@ -1,3 +1,4 @@ +using System.Linq; using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -12,11 +13,13 @@ namespace OrchardCore.Templates.Services public class TemplatesShapeBindingResolver : IShapeBindingResolver { private TemplatesDocument _templatesDocument; + private readonly TemplatesDocument _localTemplates; + private readonly TemplatesManager _templatesManager; private readonly ILiquidTemplateManager _liquidTemplateManager; - private readonly PreviewTemplatesProvider _previewTemplatesProvider; private readonly IHttpContextAccessor _httpContextAccessor; private readonly HtmlEncoder _htmlEncoder; + private bool? isAdmin; public TemplatesShapeBindingResolver( TemplatesManager templatesManager, @@ -27,23 +30,25 @@ public TemplatesShapeBindingResolver( { _templatesManager = templatesManager; _liquidTemplateManager = liquidTemplateManager; - _previewTemplatesProvider = previewTemplatesProvider; _httpContextAccessor = httpContextAccessor; _htmlEncoder = htmlEncoder; + _localTemplates = previewTemplatesProvider.GetTemplates(); } public async Task GetShapeBindingAsync(string shapeType) { - if (AdminAttribute.IsApplied(_httpContextAccessor.HttpContext)) + // Cache this value since the service is scoped and this method is invoked for every + // alternate of every shape. + isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); + + if (isAdmin.Value) { return null; } - var localTemplates = _previewTemplatesProvider.GetTemplates(); - - if (localTemplates != null) + if (_localTemplates != null && _localTemplates.Templates.Any()) { - if (localTemplates.Templates.TryGetValue(shapeType, out var localTemplate)) + if (_localTemplates.Templates.TryGetValue(shapeType, out var localTemplate)) { return BuildShapeBinding(shapeType, localTemplate); } @@ -67,11 +72,7 @@ private ShapeBinding BuildShapeBinding(string shapeType, Template template) { BindingName = shapeType, BindingSource = shapeType, - BindingAsync = async displayContext => - { - var content = await _liquidTemplateManager.RenderHtmlContentAsync(template.Content, _htmlEncoder, displayContext.Value); - return content; - } + BindingAsync = displayContext => _liquidTemplateManager.RenderHtmlContentAsync(template.Content, _htmlEncoder, displayContext.Value) }; } } diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs index c067db5a492..b6a9bf08e31 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -14,6 +15,8 @@ namespace OrchardCore.DisplayManagement.Implementation { public class DefaultHtmlDisplay : IHtmlDisplay { + private static ConcurrentDictionary _alternateShapeTypes = []; + private readonly IShapeTableManager _shapeTableManager; private readonly IEnumerable _shapeDisplayEvents; private readonly IEnumerable _shapeBindingResolvers; @@ -243,13 +246,32 @@ private async Task GetShapeBindingAsync(string shapeType, Alternat // When no alternates matches, the shapeType is used to find the longest matching binding, // the shapetype name can break itself into shorter fallbacks at double-underscore marks, // so the shapetype itself may contain a longer alternate forms that falls back to a shorter one. - var shapeTypeScan = shapeType; - do + // Build a cache of such values + var alternateShapeTypes = _alternateShapeTypes.GetOrAdd(shapeType, shapeType => { + var segments = shapeType.Split("__"); + + if (segments.Length == 1) + { + return segments; + } + + for (var i = 1; i < segments.Length; i++) + { + segments[i] = segments[i - 1] + "__" + segments[i]; + } + + Array.Reverse(segments); + + return segments; + }); + + foreach (var shapeTypeSegment in alternateShapeTypes) + { foreach (var shapeBindingResolver in _shapeBindingResolvers) { - var binding = await shapeBindingResolver.GetShapeBindingAsync(shapeTypeScan); + var binding = await shapeBindingResolver.GetShapeBindingAsync(shapeTypeSegment); if (binding != null) { @@ -257,28 +279,15 @@ private async Task GetShapeBindingAsync(string shapeType, Alternat } } - if (shapeTable.Bindings.TryGetValue(shapeTypeScan, out var shapeBinding)) + if (shapeTable.Bindings.TryGetValue(shapeTypeSegment, out var shapeBinding)) { return shapeBinding; } } - while (TryGetParentShapeTypeName(ref shapeTypeScan)); return null; } - private static bool TryGetParentShapeTypeName(ref string shapeTypeScan) - { - var delimiterIndex = shapeTypeScan.LastIndexOf("__", StringComparison.Ordinal); - if (delimiterIndex > 0) - { - shapeTypeScan = shapeTypeScan[..delimiterIndex]; - return true; - } - - return false; - } - private static ValueTask ProcessAsync(ShapeBinding shapeBinding, IShape shape, DisplayContext context) { static async ValueTask Awaited(Task task) From 04ff7d84d6920451c29f8d3d3f1578ebb103ec4c Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 12:04:13 -0700 Subject: [PATCH 2/9] Skip caching overhead when it is disabled --- .../DynamicCacheShapeDisplayEvents.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs b/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs index 237d288f156..231640fa7ec 100644 --- a/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs +++ b/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs @@ -37,6 +37,11 @@ public DynamicCacheShapeDisplayEvents( public async Task DisplayingAsync(ShapeDisplayContext context) { + if (!_cacheOptions.Enabled) + { + return; + } + // The shape has cache settings and no content yet. if (context.Shape.Metadata.IsCached && context.ChildContent == null) { @@ -62,6 +67,11 @@ public async Task DisplayingAsync(ShapeDisplayContext context) public async Task DisplayedAsync(ShapeDisplayContext context) { + if (!_cacheOptions.Enabled) + { + return; + } + var cacheContext = context.Shape.Metadata.Cache(); // If the shape is not configured to be cached, continue as usual. @@ -96,6 +106,11 @@ public async Task DisplayedAsync(ShapeDisplayContext context) public Task DisplayingFinalizedAsync(ShapeDisplayContext context) { + if (!_cacheOptions.Enabled) + { + return Task.CompletedTask; + } + var cacheContext = context.Shape.Metadata.Cache(); if (cacheContext != null && _openScopes.ContainsKey(cacheContext.CacheId)) From 80e7d618c8c87f438930dc8c9bf0a182468ee409 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 19:10:55 -0700 Subject: [PATCH 3/9] Reduce ResourceTagHelper allocations --- .../TagHelpers/ResourcesTagHelper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.ResourceManagement/TagHelpers/ResourcesTagHelper.cs b/src/OrchardCore/OrchardCore.ResourceManagement/TagHelpers/ResourcesTagHelper.cs index 54d4e608987..e95cdb49b2a 100644 --- a/src/OrchardCore/OrchardCore.ResourceManagement/TagHelpers/ResourcesTagHelper.cs +++ b/src/OrchardCore/OrchardCore.ResourceManagement/TagHelpers/ResourcesTagHelper.cs @@ -1,5 +1,5 @@ using System; -using System.IO; +using Cysharp.Text; using Microsoft.AspNetCore.Razor.TagHelpers; using Microsoft.Extensions.Logging; @@ -36,7 +36,7 @@ public override void Process(TagHelperContext tagHelperContext, TagHelperOutput { try { - using var sw = new StringWriter(); + using var sw = new ZStringWriter(); switch (Type) { From bdad860f12fb8ea2f5b2a8c6e8a34c8c0f982272 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 19:11:30 -0700 Subject: [PATCH 4/9] Improve ViewEngine DI resolution --- .../OrchardCoreBuilderExtensions.cs | 4 ++- .../Razor/RazorShapeTemplateViewEngine.cs | 26 +++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs b/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs index 44c4c3355cf..e88a0954ff1 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs @@ -54,7 +54,9 @@ public static OrchardCoreBuilder AddTheming(this OrchardCoreBuilder builder) services.AddScoped(); services.AddScoped(); - services.AddScoped(); + services.AddScoped(); + services.AddScoped(sp => sp.GetService()); + services.AddSingleton, ThemingViewsFeatureProvider>(); services.AddScoped(); diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs index 454c9667b50..beb54f6e477 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs @@ -25,30 +25,37 @@ namespace OrchardCore.DisplayManagement.Razor public class RazorShapeTemplateViewEngine : IShapeTemplateViewEngine { private readonly IOptions _options; + private readonly IEnumerable _viewExtensionProviders; private readonly IHttpContextAccessor _httpContextAccessor; private readonly ViewContextAccessor _viewContextAccessor; private readonly ITempDataProvider _tempDataProvider; - private readonly List _templateFileExtensions = new([RazorViewEngine.ViewExtension]); + private readonly IHtmlHelper _htmlHelper; public RazorShapeTemplateViewEngine( IOptions options, IEnumerable viewExtensionProviders, IHttpContextAccessor httpContextAccessor, ViewContextAccessor viewContextAccessor, - ITempDataProvider tempDataProvider) + ITempDataProvider tempDataProvider, + IHtmlHelper htmlHelper) { _options = options; + _viewExtensionProviders = viewExtensionProviders; _httpContextAccessor = httpContextAccessor; _viewContextAccessor = viewContextAccessor; _tempDataProvider = tempDataProvider; - _templateFileExtensions.AddRange(viewExtensionProviders.Select(x => x.ViewExtension)); + _htmlHelper = htmlHelper; } public IEnumerable TemplateFileExtensions { get { - return _templateFileExtensions; + yield return RazorViewEngine.ViewExtension; + foreach (var provider in _viewExtensionProviders) + { + yield return provider.ViewExtension; + } } } @@ -164,18 +171,15 @@ private async Task GetActionContextAsync() return actionContext; } - private static IHtmlHelper MakeHtmlHelper(ViewContext viewContext, ViewDataDictionary viewData) + private IHtmlHelper MakeHtmlHelper(ViewContext viewContext, ViewDataDictionary viewData) { - var newHelper = viewContext.HttpContext.RequestServices.GetRequiredService(); - - var contextable = newHelper as IViewContextAware; - if (contextable != null) + if (_htmlHelper is IViewContextAware contextAwareHelper) { var newViewContext = new ViewContext(viewContext, viewContext.View, viewData, viewContext.Writer); - contextable.Contextualize(newViewContext); + contextAwareHelper.Contextualize(newViewContext); } - return newHelper; + return _htmlHelper; } } } From 1cd8a76df1a395d3b6e545cad7d97e05eef5a1b9 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 19:12:13 -0700 Subject: [PATCH 5/9] Reduce css class string allocations in ShapeTagBuilder --- src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs index 33a8e9a7c06..b01d3db949e 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs @@ -105,10 +105,8 @@ public static TagBuilder GetTagBuilder(this IShape shape, string defaultTagName if (shape.Classes != null) { - foreach (var cssClass in shape.Classes) - { - tagBuilder.AddCssClass(cssClass); - } + // Faster than AddCssClass which will do twice as many concatenations as classes. + tagBuilder.Attributes["class"] = string.Join(' ', shape.Classes); } if (!string.IsNullOrWhiteSpace(shape.Id)) From f11b7fb35326efd3d1e8e4e43f7bddd7a2250107 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 4 Apr 2024 19:12:41 -0700 Subject: [PATCH 6/9] Improve dynamic proxy creation performance --- .../IShapeFactory.cs | 26 +++++-- .../ShapeProxyBenchmark.cs | 68 +++++++++++++++++++ 2 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 test/OrchardCore.Benchmarks/ShapeProxyBenchmark.cs diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/IShapeFactory.cs b/src/OrchardCore/OrchardCore.DisplayManagement/IShapeFactory.cs index 682cdcf9b72..07713386e5e 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/IShapeFactory.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/IShapeFactory.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Concurrent; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Castle.DynamicProxy; @@ -25,6 +27,7 @@ ValueTask CreateAsync( public static class ShapeFactoryExtensions { + private static readonly ConcurrentDictionary _proxyTypesCache = []; private static readonly ProxyGenerator _proxyGenerator = new(); private static readonly Func> _newShape = () => new(new Shape()); @@ -42,18 +45,27 @@ public static ValueTask CreateAsync(this IShapeFactory factory, private static IShape CreateShape(Type baseType) { + var shapeType = baseType; + // Don't generate a proxy for shape types - if (typeof(IShape).IsAssignableFrom(baseType)) + if (typeof(IShape).IsAssignableFrom(shapeType)) { - var shape = Activator.CreateInstance(baseType) as IShape; - return shape; + return (IShape)Activator.CreateInstance(baseType); } - else + + if (_proxyTypesCache.TryGetValue(baseType, out var proxyType)) { - var options = new ProxyGenerationOptions(); - options.AddMixinInstance(new ShapeViewModel()); - return (IShape)_proxyGenerator.CreateClassProxy(baseType, options); + var model = new ShapeViewModel(); + return (IShape)Activator.CreateInstance(proxyType, model, model, Array.Empty()); } + + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new ShapeViewModel()); + var shape = (IShape)_proxyGenerator.CreateClassProxy(baseType, options); + + _proxyTypesCache.TryAdd(baseType, shape.GetType()); + + return shape; } public static ValueTask CreateAsync(this IShapeFactory factory, string shapeType) diff --git a/test/OrchardCore.Benchmarks/ShapeProxyBenchmark.cs b/test/OrchardCore.Benchmarks/ShapeProxyBenchmark.cs new file mode 100644 index 00000000000..01b0cbc9d88 --- /dev/null +++ b/test/OrchardCore.Benchmarks/ShapeProxyBenchmark.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Concurrent; +using BenchmarkDotNet.Attributes; +using Castle.DynamicProxy; +using OrchardCore.ContentManagement.Display.ViewModels; +using OrchardCore.DisplayManagement; +using OrchardCore.DisplayManagement.Views; +using OrchardCore.Navigation; + +namespace OrchardCore.Benchmark +{ + /* + | Method | Mean | Error | StdDev | Gen0 | Allocated | + |----------------------------- |-----------:|-----------:|----------:|-------:|----------:| + | CreateInstance | 9.774 ns | 17.795 ns | 0.9754 ns | 0.0093 | 88 B | + | CreateDynamicProxy | 722.913 ns | 121.855 ns | 6.6793 ns | 0.2222 | 2096 B | + | CreateCachedProxy | 233.722 ns | 65.978 ns | 3.6165 ns | 0.0756 | 712 B | + */ + + [MemoryDiagnoser] + [ShortRunJob] + public class ShapeProxyBenchmark + { + private static ConcurrentDictionary _proxyTypesCache = []; + private static readonly ProxyGenerator _proxyGenerator = new(); + private static readonly Type _proxyType; + + static ShapeProxyBenchmark() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new ShapeViewModel()); + _proxyType = _proxyGenerator.CreateClassProxy(typeof(MenuItem), options).GetType(); + } + + [Benchmark] + public object CreateInstance() + { + var shape = (IShape)Activator.CreateInstance(typeof(ContentItemViewModel)); + return shape; + } + + [Benchmark] + public object CreateDynamicProxy() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new ShapeViewModel()); + return (IShape)_proxyGenerator.CreateClassProxy(typeof(MenuItem), options); + } + + [Benchmark] + public object CreateCachedProxy() + { + if (_proxyTypesCache.TryGetValue(typeof(MenuItem), out var proxyType)) + { + var model = new ShapeViewModel(); + return (IShape)Activator.CreateInstance(_proxyType, model, model, Array.Empty()); + } + + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new ShapeViewModel()); + var shape = (IShape)_proxyGenerator.CreateClassProxy(typeof(MenuItem), options); + + _proxyTypesCache.TryAdd(typeof(MenuItem), shape.GetType()); + + return shape; + } + } +} From 001caba737265051d0bdb43c3d763296af987a33 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 5 Apr 2024 09:57:22 -0700 Subject: [PATCH 7/9] Empty checks --- src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs index b01d3db949e..78f17dc4796 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using System.Text.Json.Nodes; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Rendering; @@ -98,12 +99,12 @@ public static TagBuilder GetTagBuilder(this IShape shape, string defaultTagName var tagBuilder = new TagBuilder(tagName); - if (shape.Attributes != null) + if (shape.Attributes != null && shape.Attributes.Any()) { tagBuilder.MergeAttributes(shape.Attributes, false); } - if (shape.Classes != null) + if (shape.Classes != null && shape.Classes.Any()) { // Faster than AddCssClass which will do twice as many concatenations as classes. tagBuilder.Attributes["class"] = string.Join(' ', shape.Classes); From 697cbef7f92cfae1b4109bb6b3fbc6aa2d4a6595 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 5 Apr 2024 11:44:49 -0700 Subject: [PATCH 8/9] Simplify alternates creation --- .../Implementation/DefaultHtmlDisplay.cs | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs index 264d4b992eb..69bb24bdde3 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Collections.Concurrent; using System.Collections.Generic; using System.Threading.Tasks; @@ -15,7 +16,7 @@ namespace OrchardCore.DisplayManagement.Implementation public class DefaultHtmlDisplay : IHtmlDisplay { private const string _separator = "__"; - private static ConcurrentDictionary _alternateShapeTypes = []; + private static readonly ConcurrentDictionary _alternateShapeTypes = []; private readonly IShapeTableManager _shapeTableManager; private readonly IEnumerable _shapeDisplayEvents; @@ -245,21 +246,16 @@ private async Task GetShapeBindingAsync(string shapeType, Alternat // Build a cache of such values var alternateShapeTypes = _alternateShapeTypes.GetOrAdd(shapeType, shapeType => { - var segments = shapeType.Split("__"); + var segments = new List(2); - if (segments.Length == 1) - { - return segments; - } + var alternate = shapeType; - for (var i = 1; i < segments.Length; i++) + do { - segments[i] = segments[i - 1] + "__" + segments[i]; - } + segments.Add(alternate); + } while (TryGetParentShapeTypeName(alternate, out alternate)); - Array.Reverse(segments); - - return segments; + return segments.ToArray(); }); foreach (var shapeTypeSegment in alternateShapeTypes) @@ -283,6 +279,20 @@ private async Task GetShapeBindingAsync(string shapeType, Alternat return null; } + private static bool TryGetParentShapeTypeName(string shapeTypeScan, out string parentType) + { + parentType = shapeTypeScan; + + var delimiterIndex = shapeTypeScan.LastIndexOf(_separator, StringComparison.Ordinal); + if (delimiterIndex > 0) + { + parentType = shapeTypeScan[..delimiterIndex]; + return true; + } + + return false; + } + private static ValueTask ProcessAsync(ShapeBinding shapeBinding, IShape shape, DisplayContext context) { static async ValueTask Awaited(Task task) From 40eda3cb99a82eb626dfcf9357de828779e9c907 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 5 Apr 2024 14:34:05 -0700 Subject: [PATCH 9/9] PR feedback --- .../Services/AdminTemplatesShapeBindingResolver.cs | 6 +++--- .../Services/TemplatesShapeBindingResolver.cs | 8 ++++---- src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs | 5 ++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs b/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs index 116875f69f6..a8f0abcbaf1 100644 --- a/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Templates/Services/AdminTemplatesShapeBindingResolver.cs @@ -17,7 +17,7 @@ public class AdminTemplatesShapeBindingResolver : IShapeBindingResolver private readonly AdminPreviewTemplatesProvider _previewTemplatesProvider; private readonly IHttpContextAccessor _httpContextAccessor; private readonly HtmlEncoder _htmlEncoder; - private bool? isAdmin; + private bool? _isAdmin; public AdminTemplatesShapeBindingResolver( AdminTemplatesManager templatesManager, @@ -37,9 +37,9 @@ public async Task GetShapeBindingAsync(string shapeType) { // Cache this value since the service is scoped and this method is invoked for every // alternate of every shape. - isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); + _isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); - if (!isAdmin.Value) + if (!_isAdmin.Value) { return null; } diff --git a/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs b/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs index 761abb8b7fa..86df95fc5c4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Templates/Services/TemplatesShapeBindingResolver.cs @@ -19,7 +19,7 @@ public class TemplatesShapeBindingResolver : IShapeBindingResolver private readonly ILiquidTemplateManager _liquidTemplateManager; private readonly IHttpContextAccessor _httpContextAccessor; private readonly HtmlEncoder _htmlEncoder; - private bool? isAdmin; + private bool? _isAdmin; public TemplatesShapeBindingResolver( TemplatesManager templatesManager, @@ -39,14 +39,14 @@ public async Task GetShapeBindingAsync(string shapeType) { // Cache this value since the service is scoped and this method is invoked for every // alternate of every shape. - isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); + _isAdmin ??= AdminAttribute.IsApplied(_httpContextAccessor.HttpContext); - if (isAdmin.Value) + if (_isAdmin.Value) { return null; } - if (_localTemplates != null && _localTemplates.Templates.Any()) + if (_localTemplates?.Templates?.Count != 0) { if (_localTemplates.Templates.TryGetValue(shapeType, out var localTemplate)) { diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs index 78f17dc4796..0e30e432c1b 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/IShape.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; using System.Text.Json.Nodes; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Rendering; @@ -99,12 +98,12 @@ public static TagBuilder GetTagBuilder(this IShape shape, string defaultTagName var tagBuilder = new TagBuilder(tagName); - if (shape.Attributes != null && shape.Attributes.Any()) + if (shape.Attributes?.Count > 0) { tagBuilder.MergeAttributes(shape.Attributes, false); } - if (shape.Classes != null && shape.Classes.Any()) + if (shape.Classes?.Count > 0) { // Faster than AddCssClass which will do twice as many concatenations as classes. tagBuilder.Attributes["class"] = string.Join(' ', shape.Classes);