Merge pull request #181 from lahma/faster-invoke

Reduce allocations in ImageSharpMiddleware.Invoke for common case
SixLabors · Oct 10, 2021 · a5b0da1 · a5b0da1
2 parents 11caa09 + 86568cd
commit a5b0da1
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 10 deletions.
diff --git 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;
@@ -161,7 +160,7 @@ public ImageSharpMiddleware(
                 ? CultureInfo.InvariantCulture
                 : CultureInfo.CurrentCulture;
 
-            var commands = new HashSet<string>();
+            var commands = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
             foreach (IImageWebProcessor processor in this.processors)
             {
                 foreach (string command in processor.Commands)
@@ -185,16 +184,24 @@ public ImageSharpMiddleware(
         public async Task Invoke(HttpContext context)
 #pragma warning restore IDE1006 // Naming Styles
         {
-            IDictionary<string, string> commands = this.requestParser.ParseRequestCommands(context);
+            // We expect to get concrete collection type which removes virtual dispatch concerns and enumerator allocations
+            IDictionary<string, string> parsedCommands = this.requestParser.ParseRequestCommands(context);
+            Dictionary<string, string> commands = parsedCommands as Dictionary<string, string> ?? new Dictionary<string, string>(parsedCommands, StringComparer.OrdinalIgnoreCase);
+
             if (commands.Count > 0)
             {
-                // Strip out any unknown commands
-                foreach (string command in new List<string>(commands.Keys))
+                // Strip out any unknown commands, if needed.
+                int index = 0;
+                foreach (string command in commands.Keys)
                 {
-                    if (!this.knownCommands.Contains(command, StringComparer.OrdinalIgnoreCase))
+                    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<string, string> commands, int startAtIndex)
+        {
+            var keys = new List<string>(commands.Keys);
+            for (int 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
@@ -54,7 +54,6 @@ public void PresetOnlyQueryCollectionRequestParserExtractsCommandsWithOtherCasin
             Assert.Equal(expected, actual);
         }
 
-
         [Fact]
         public void PresetOnlyQueryCollectionRequestParserCommandsWithoutPresetParam()
         {

diff --git 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<PhysicalFileSystemCacheTestServerFixture>
     {
         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)