From f65650d2fa5238f920a54804b28b4fb38b92c7c6 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Fri, 30 Jul 2021 19:28:30 +0300 Subject: [PATCH 1/2] Reduce allocations in ImageSharpMiddleware.Invoke * remove virtual dispatch and enumeration allocations for common path in ImageSharpMiddleware.Invoke --- .../Middleware/ImageSharpMiddleware.cs | 30 +++++++++++++++---- ...etOnlyQueryCollectionRequestParserTests.cs | 1 - 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs index fdc2b070..33dca9a9 100644 --- a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs +++ b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs @@ -9,7 +9,6 @@ using System.IO; using System.Linq; using System.Text; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -185,16 +184,24 @@ public ImageSharpMiddleware( public async Task Invoke(HttpContext context) #pragma warning restore IDE1006 // Naming Styles { - IDictionary commands = this.requestParser.ParseRequestCommands(context); + // We expect to get concrete collection type which removes virtual dispatch concerns and enumerator allocations + IDictionary parsedCommands = this.requestParser.ParseRequestCommands(context); + Dictionary commands = parsedCommands as Dictionary ?? new Dictionary(parsedCommands, StringComparer.OrdinalIgnoreCase); + if (commands.Count > 0) { - // Strip out any unknown commands. - foreach (string command in new List(commands.Keys)) + // Strip out any unknown commands, if needed. + int index = 0; + foreach (string command in commands.Keys) { if (!this.knownCommands.Contains(command)) { - commands.Remove(command); + // Need to actually remove, allocates new list to allow modifications + this.StripUnknownCommands(commands, startAtIndex: index); + break; } + + ++index; } } @@ -239,6 +246,19 @@ await this.ProcessRequestAsync( commands); } + private void StripUnknownCommands(Dictionary commands, int startAtIndex) + { + var keys = new List(commands.Keys); + for (var index = startAtIndex; index < keys.Count; index++) + { + string command = keys[index]; + if (!this.knownCommands.Contains(command)) + { + commands.Remove(command); + } + } + } + private async Task ProcessRequestAsync( HttpContext context, IImageResolver sourceImageResolver, diff --git a/tests/ImageSharp.Web.Tests/Commands/PresetOnlyQueryCollectionRequestParserTests.cs b/tests/ImageSharp.Web.Tests/Commands/PresetOnlyQueryCollectionRequestParserTests.cs index a8cd8125..e96d08f0 100644 --- a/tests/ImageSharp.Web.Tests/Commands/PresetOnlyQueryCollectionRequestParserTests.cs +++ b/tests/ImageSharp.Web.Tests/Commands/PresetOnlyQueryCollectionRequestParserTests.cs @@ -54,7 +54,6 @@ public void PresetOnlyQueryCollectionRequestParserExtractsCommandsWithOtherCasin Assert.Equal(expected, actual); } - [Fact] public void PresetOnlyQueryCollectionRequestParserCommandsWithoutPresetParam() { From 86568cd5fd484797c5f0a4b65f203584fc0c548a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 10 Oct 2021 12:03:47 +1100 Subject: [PATCH 2/2] Inject invalid command for testing --- src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs | 4 ++-- .../Processing/PhysicalFileSystemCacheServerTests.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs index 43df9e96..78532d55 100644 --- a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs +++ b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs @@ -201,7 +201,7 @@ public async Task Invoke(HttpContext context) break; } - ++index; + index++; } } @@ -249,7 +249,7 @@ await this.ProcessRequestAsync( private void StripUnknownCommands(Dictionary commands, int startAtIndex) { var keys = new List(commands.Keys); - for (var index = startAtIndex; index < keys.Count; index++) + for (int index = startAtIndex; index < keys.Count; index++) { string command = keys[index]; if (!this.knownCommands.Contains(command)) diff --git a/tests/ImageSharp.Web.Tests/Processing/PhysicalFileSystemCacheServerTests.cs b/tests/ImageSharp.Web.Tests/Processing/PhysicalFileSystemCacheServerTests.cs index 118b8ca7..076311da 100644 --- a/tests/ImageSharp.Web.Tests/Processing/PhysicalFileSystemCacheServerTests.cs +++ b/tests/ImageSharp.Web.Tests/Processing/PhysicalFileSystemCacheServerTests.cs @@ -16,8 +16,8 @@ namespace SixLabors.ImageSharp.Web.Tests.Processing public class PhysicalFileSystemCacheServerTests : ServerTestBase { private const int Width = 20; - private static readonly string Command = "?width=" + Width + "&v=" + Guid.NewGuid().ToString(); - private static readonly string Command2 = "?width=" + (Width + 1) + "&v=" + Guid.NewGuid().ToString(); + private static readonly string Command = "?invalidcommand=qwerty&width=" + Width + "&v=" + Guid.NewGuid().ToString(); + private static readonly string Command2 = "?invalidcommand=qwerty&width=" + (Width + 1) + "&v=" + Guid.NewGuid().ToString(); public PhysicalFileSystemCacheServerTests(PhysicalFileSystemCacheTestServerFixture fixture) : base(fixture)