From ec3fa060c75d163b7d62ee8de8ad835e153307c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= Date: Thu, 28 Mar 2024 13:19:26 -0700 Subject: [PATCH] Improve notification filter performance (#15610) --- .../DynamicCacheShapeDisplayEvents.cs | 2 +- .../TagHelpers/DynamicCacheTagHelper.cs | 2 +- .../Pooling/ZStringWriter.cs | 207 ------------------ .../Queries/Types/ContentItemType.cs | 2 +- .../Notify/Notifier.cs | 17 +- .../Notify/NotifyEntry.cs | 36 +++ .../Notify/NotifyEntryComparer.cs | 5 +- .../Notify/NotifyEntryConverter.cs | 17 +- .../Notify/NotifyEntryExtensions.cs | 21 -- .../Notify/NotifyFilter.cs | 22 +- .../Notify/NotifyJsonSerializerOptions.cs | 8 + ...otifyJsonSerializerOptionsConfiguration.cs | 19 ++ .../OrchardCoreBuilderExtensions.cs | 2 + .../Razor/RazorShapeTemplateViewEngine.cs | 2 +- 14 files changed, 100 insertions(+), 262 deletions(-) delete mode 100644 src/OrchardCore/OrchardCore.Abstractions/Pooling/ZStringWriter.cs delete mode 100644 src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryExtensions.cs create mode 100644 src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptions.cs create mode 100644 src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptionsConfiguration.cs diff --git a/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs b/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs index 0f79bc5ed69..237d288f156 100644 --- a/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs +++ b/src/OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs @@ -1,9 +1,9 @@ using System.Collections.Generic; using System.Text.Encodings.Web; using System.Threading.Tasks; +using Cysharp.Text; using Microsoft.AspNetCore.Html; using Microsoft.Extensions.Options; -using OrchardCore.Abstractions.Pooling; using OrchardCore.DisplayManagement.Implementation; using OrchardCore.Environment.Cache; diff --git a/src/OrchardCore.Modules/OrchardCore.DynamicCache/TagHelpers/DynamicCacheTagHelper.cs b/src/OrchardCore.Modules/OrchardCore.DynamicCache/TagHelpers/DynamicCacheTagHelper.cs index 273bf3255f0..b0e1289e333 100644 --- a/src/OrchardCore.Modules/OrchardCore.DynamicCache/TagHelpers/DynamicCacheTagHelper.cs +++ b/src/OrchardCore.Modules/OrchardCore.DynamicCache/TagHelpers/DynamicCacheTagHelper.cs @@ -1,11 +1,11 @@ using System; using System.Text.Encodings.Web; using System.Threading.Tasks; +using Cysharp.Text; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.TagHelpers.Cache; using Microsoft.AspNetCore.Razor.TagHelpers; using Microsoft.Extensions.Options; -using OrchardCore.Abstractions.Pooling; using OrchardCore.Environment.Cache; namespace OrchardCore.DynamicCache.TagHelpers diff --git a/src/OrchardCore/OrchardCore.Abstractions/Pooling/ZStringWriter.cs b/src/OrchardCore/OrchardCore.Abstractions/Pooling/ZStringWriter.cs deleted file mode 100644 index e68ce00b898..00000000000 --- a/src/OrchardCore/OrchardCore.Abstractions/Pooling/ZStringWriter.cs +++ /dev/null @@ -1,207 +0,0 @@ -using System; -using System.Globalization; -using System.IO; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using Cysharp.Text; - -namespace OrchardCore.Abstractions.Pooling -{ - /// - /// Based StringWriter but made to use ZString as backing implementation. - /// - internal sealed class ZStringWriter : TextWriter - { - private Utf16ValueStringBuilder _sb; - private bool _isOpen; - private UnicodeEncoding _encoding; - - public ZStringWriter() : this(CultureInfo.CurrentCulture) - { - } - - public ZStringWriter(IFormatProvider formatProvider) : base(formatProvider) - { - _sb = ZString.CreateStringBuilder(); - _isOpen = true; - } - - public override void Close() - { - Dispose(true); - } - - protected override void Dispose(bool disposing) - { - _sb.Dispose(); - _isOpen = false; - base.Dispose(disposing); - } - - public override Encoding Encoding => _encoding ??= new UnicodeEncoding(false, false); - - // Writes a character to the underlying string buffer. - // - public override void Write(char value) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(value); - } - - public override void Write(char[] buffer, int index, int count) - { - ArgumentNullException.ThrowIfNull(buffer); - - ArgumentOutOfRangeException.ThrowIfLessThan(index, 0); - ArgumentOutOfRangeException.ThrowIfLessThan(count, 0); - if (buffer.Length - index < count) - { - throw new ArgumentException("Buffer overflow."); - } - - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(buffer.AsSpan(index, count)); - } - - public override void Write(ReadOnlySpan buffer) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(buffer); - } - - // Writes a string to the underlying string buffer. If the given string is - // null, nothing is written. - // - public override void Write(string value) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - if (value != null) - { - _sb.Append(value); - } - } - - public override void Write(StringBuilder value) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(value); - } - - public override void WriteLine(ReadOnlySpan buffer) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(buffer); - WriteLine(); - } - - public override void WriteLine(StringBuilder value) - { - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(value); - WriteLine(); - } - - public override Task WriteAsync(char value) - { - Write(value); - return Task.CompletedTask; - } - - public override Task WriteAsync(string value) - { - Write(value); - return Task.CompletedTask; - } - - public override Task WriteAsync(char[] buffer, int index, int count) - { - Write(buffer, index, count); - return Task.CompletedTask; - } - - public override Task WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - Write(buffer.Span); - return Task.CompletedTask; - } - - public override Task WriteAsync(StringBuilder value, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(value); - return Task.CompletedTask; - } - - public override Task WriteLineAsync(char value) - { - WriteLine(value); - return Task.CompletedTask; - } - - public override Task WriteLineAsync(string value) - { - WriteLine(value); - return Task.CompletedTask; - } - - public override Task WriteLineAsync(StringBuilder value, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - ObjectDisposedException.ThrowIf(!_isOpen, nameof(_sb)); - - _sb.Append(value); - WriteLine(); - return Task.CompletedTask; - } - - public override Task WriteLineAsync(char[] buffer, int index, int count) - { - WriteLine(buffer, index, count); - return Task.CompletedTask; - } - - public override Task WriteLineAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - WriteLine(buffer.Span); - return Task.CompletedTask; - } - - public override Task FlushAsync() - { - return Task.CompletedTask; - } - - public override string ToString() - { - return _sb.ToString(); - } - } -} diff --git a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/ContentItemType.cs b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/ContentItemType.cs index 61baf94f252..391c7caa0f3 100644 --- a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/ContentItemType.cs +++ b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/ContentItemType.cs @@ -1,10 +1,10 @@ using System.Text.Encodings.Web; using System.Threading.Tasks; +using Cysharp.Text; using GraphQL; using GraphQL.Types; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using OrchardCore.Abstractions.Pooling; using OrchardCore.Apis.GraphQL; using OrchardCore.ContentManagement.Display; using OrchardCore.ContentManagement.GraphQL.Options; diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/Notifier.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/Notifier.cs index b205283de06..08ca4931a90 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/Notifier.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/Notifier.cs @@ -1,5 +1,6 @@ -using System; using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Localization; using Microsoft.Extensions.Logging; @@ -7,7 +8,7 @@ namespace OrchardCore.DisplayManagement.Notify { public class Notifier : INotifier { - private readonly IList _entries; + private readonly List _entries; private readonly ILogger _logger; public Notifier(ILogger logger) @@ -16,17 +17,21 @@ public Notifier(ILogger logger) _logger = logger; } - [Obsolete("This method will be removed in a later version. Use AddAsync()")] - // TODO The implementation for this is provided as an interface default implementation - // when the interface method is removed, replace this with AddAsync. public void Add(NotifyType type, LocalizedHtmlString message) + { + throw new System.NotImplementedException(); + } + + public ValueTask AddAsync(NotifyType type, LocalizedHtmlString message) { if (_logger.IsEnabled(LogLevel.Information)) { - _logger.LogInformation("Notification '{NotificationType}' with message '{NotificationMessage}'", type, message.Value); + _logger.LogInformation("Notification '{NotificationType}' with message '{NotificationMessage}'", type, message.ToString()); } _entries.Add(new NotifyEntry { Type = type, Message = message }); + + return ValueTask.CompletedTask; } public IList List() diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntry.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntry.cs index 3d37536af8f..11d71a347e2 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntry.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntry.cs @@ -1,3 +1,6 @@ +using System.Text.Encodings.Web; +using System.Web; +using Cysharp.Text; using Microsoft.AspNetCore.Html; namespace OrchardCore.DisplayManagement.Notify @@ -12,7 +15,40 @@ public enum NotifyType public class NotifyEntry { + private (HtmlEncoder HtmlEncoder, string Message) _cache; + public NotifyType Type { get; set; } public IHtmlContent Message { get; set; } + + public string ToHtmlString(HtmlEncoder htmlEncoder) + { + // When the object is created from a cookie the message + // is an HtmlString so we can use this instead of using + // the TextWriter path. + + if (Message is IHtmlString htmlString) + { + return htmlString.ToHtmlString(); + } + + // Cache the encoded version for the specified encoder. + // This is necessary as long as there will be string-based comparisons + // and the need of NotifyEntryComparer + + var cache = _cache; + + if (cache.Message != null && cache.HtmlEncoder == htmlEncoder) + { + return cache.Message; + } + + using var stringWriter = new ZStringWriter(); + Message.WriteTo(stringWriter, htmlEncoder); + stringWriter.Flush(); + + _cache = cache = new(htmlEncoder, stringWriter.ToString()); + + return cache.Message; + } } } diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryComparer.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryComparer.cs index 8f31d04776e..eae765b08b7 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryComparer.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryComparer.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Text.Encodings.Web; @@ -14,12 +15,12 @@ public NotifyEntryComparer(HtmlEncoder htmlEncoder) public bool Equals(NotifyEntry x, NotifyEntry y) { - return x.Type == y.Type && x.GetMessageAsString(_htmlEncoder) == y.GetMessageAsString(_htmlEncoder); + return x.Type == y.Type && x.ToHtmlString(_htmlEncoder) == y.ToHtmlString(_htmlEncoder); } public int GetHashCode(NotifyEntry obj) { - return obj.GetMessageAsString(_htmlEncoder).GetHashCode() & 23 * obj.Type.GetHashCode(); + return HashCode.Combine(obj.ToHtmlString(_htmlEncoder), obj.Type); } } } diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryConverter.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryConverter.cs index 82a8c1fff6a..9446ec8df7e 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryConverter.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryConverter.cs @@ -1,6 +1,4 @@ using System; -using System.IO; -using System.Text; using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Nodes; @@ -48,18 +46,11 @@ public override void Write(Utf8JsonWriter writer, NotifyEntry value, JsonSeriali return; } - var o = new JsonObject(); - - // Serialize the message as it's an IHtmlContent - var stringBuilder = new StringBuilder(); - using (var stringWriter = new StringWriter(stringBuilder)) + var o = new JsonObject { - notifyEntry.Message.WriteTo(stringWriter, _htmlEncoder); - } - - // Write all well-known properties - o.Add(nameof(NotifyEntry.Type), notifyEntry.Type.ToString()); - o.Add(nameof(NotifyEntry.Message), notifyEntry.GetMessageAsString(_htmlEncoder)); + { nameof(NotifyEntry.Type), notifyEntry.Type.ToString() }, + { nameof(NotifyEntry.Message), notifyEntry.ToHtmlString(_htmlEncoder) } + }; o.WriteTo(writer); } diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryExtensions.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryExtensions.cs deleted file mode 100644 index 1451f0a068d..00000000000 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyEntryExtensions.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.IO; -using System.Text; -using System.Text.Encodings.Web; - -namespace OrchardCore.DisplayManagement.Notify -{ - internal static class NotifyEntryExtensions - { - public static string GetMessageAsString(this NotifyEntry entry, HtmlEncoder htmlEncoder) - { - var stringBuilder = new StringBuilder(); - using (var stringWriter = new StringWriter(stringBuilder)) - { - entry.Message.WriteTo(stringWriter, htmlEncoder); - stringWriter.Flush(); - } - - return stringBuilder.ToString(); - } - } -} diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyFilter.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyFilter.cs index 870c22c4703..073e42ea57b 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyFilter.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyFilter.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.RazorPages; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using OrchardCore.DisplayManagement.Layout; namespace OrchardCore.DisplayManagement.Notify @@ -23,9 +24,9 @@ public class NotifyFilter : IActionFilter, IAsyncResultFilter, IPageFilter private readonly ILayoutAccessor _layoutAccessor; private readonly IDataProtectionProvider _dataProtectionProvider; private readonly HtmlEncoder _htmlEncoder; + private readonly IOptions _notifyJsonSerializerOptions; private readonly ILogger _logger; - public readonly JsonSerializerOptions _settings; private NotifyEntry[] _existingEntries = []; private bool _shouldDeleteCookie; @@ -35,6 +36,7 @@ public NotifyFilter( ILayoutAccessor layoutAccessor, IDataProtectionProvider dataProtectionProvider, HtmlEncoder htmlEncoder, + IOptions notifyJsonSerializerOptions, ILogger logger) { _notifier = notifier; @@ -42,15 +44,17 @@ public NotifyFilter( _layoutAccessor = layoutAccessor; _dataProtectionProvider = dataProtectionProvider; _htmlEncoder = htmlEncoder; + _notifyJsonSerializerOptions = notifyJsonSerializerOptions; _logger = logger; - - _settings = new(JOptions.Default); - _settings.Converters.Add(new NotifyEntryConverter(htmlEncoder)); } private void OnHandlerExecuting(FilterContext filterContext) { - var messages = Convert.ToString(filterContext.HttpContext.Request.Cookies[CookiePrefix]); + if (!filterContext.HttpContext.Request.Cookies.TryGetValue(CookiePrefix, out var messages)) + { + return; + } + if (string.IsNullOrEmpty(messages)) { return; @@ -76,10 +80,10 @@ private void OnHandlerExecuting(FilterContext filterContext) private void OnHandlerExecuted(FilterContext filterContext) { - var messageEntries = _notifier.List().ToArray(); + var messageEntries = _notifier.List(); // Don't touch temp data if there's no work to perform. - if (messageEntries.Length == 0 && _existingEntries.Length == 0) + if (messageEntries.Count == 0 && _existingEntries.Length == 0) { return; } @@ -175,7 +179,7 @@ private string SerializeNotifyEntry(NotifyEntry[] notifyEntries) try { var protector = _dataProtectionProvider.CreateProtector(nameof(NotifyFilter)); - var signed = protector.Protect(JConvert.SerializeObject(notifyEntries, _settings)); + var signed = protector.Protect(JConvert.SerializeObject(notifyEntries, _notifyJsonSerializerOptions.Value.SerializerOptions)); return WebUtility.UrlEncode(signed); } catch @@ -190,7 +194,7 @@ private void DeserializeNotifyEntries(string value, out NotifyEntry[] messageEnt { var protector = _dataProtectionProvider.CreateProtector(nameof(NotifyFilter)); var decoded = protector.Unprotect(WebUtility.UrlDecode(value)); - messageEntries = JConvert.DeserializeObject(decoded, _settings); + messageEntries = JConvert.DeserializeObject(decoded, _notifyJsonSerializerOptions.Value.SerializerOptions); } catch { diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptions.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptions.cs new file mode 100644 index 00000000000..7fab6149dbd --- /dev/null +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptions.cs @@ -0,0 +1,8 @@ +using System.Text.Json; + +namespace OrchardCore.DisplayManagement.Notify; + +public class NotifyJsonSerializerOptions +{ + public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(); +} diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptionsConfiguration.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptionsConfiguration.cs new file mode 100644 index 00000000000..c1ed3ace57e --- /dev/null +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Notify/NotifyJsonSerializerOptionsConfiguration.cs @@ -0,0 +1,19 @@ +using System.Text.Encodings.Web; +using Microsoft.Extensions.Options; + +namespace OrchardCore.DisplayManagement.Notify; + +internal sealed class NotifyJsonSerializerOptionsConfiguration : IConfigureOptions +{ + private readonly HtmlEncoder _htmlEncoder; + + public NotifyJsonSerializerOptionsConfiguration(HtmlEncoder htmlEncoder) + { + _htmlEncoder = htmlEncoder; + } + + public void Configure(NotifyJsonSerializerOptions options) + { + options.SerializerOptions.Converters.Add(new NotifyEntryConverter(_htmlEncoder)); + } +} diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs b/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs index ca2bff861f0..2ea811a587a 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs @@ -44,6 +44,8 @@ public static OrchardCoreBuilder AddTheming(this OrchardCoreBuilder builder) options.Filters.Add(typeof(RazorViewActionFilter)); }); + services.AddTransient, NotifyJsonSerializerOptionsConfiguration>(); + // Used as a service when we create a fake 'ActionContext'. services.AddScoped(); diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs index 04980a050ac..454c9667b50 100644 --- a/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs +++ b/src/OrchardCore/OrchardCore.DisplayManagement/Razor/RazorShapeTemplateViewEngine.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using System.Threading.Tasks; +using Cysharp.Text; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -16,7 +17,6 @@ using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using OrchardCore.Abstractions.Pooling; using OrchardCore.DisplayManagement.Descriptors.ShapeTemplateStrategy; using OrchardCore.DisplayManagement.Implementation;