NameValueCollection.ToQueryString performance optimisation (#4952)

Author: stevejgordon

src/Elasticsearch.Net/Elasticsearch.Net.csproj

@@ -23,6 +23,8 @@
     <PackageReference Include="Microsoft.CSharp" Version="4.6.0" />
     <PackageReference Include="System.Buffers" Version="4.5.0" />
     <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="4.5.1" />
+
+    <PackageReference Condition="'$(TargetFramework)' != 'netstandard2.1'" Include="System.Memory" Version="4.5.0" />
 
     <PackageReference Condition="'$(TargetFramework)' == 'netstandard2.0'" Include="System.Reflection.Emit" Version="4.3.0" />
     <PackageReference Condition="'$(TargetFramework)' == 'netstandard2.0'" Include="System.Reflection.Emit.Lightweight" Version="4.3.0" />

src/Elasticsearch.Net/Extensions/NameValueCollectionExtensions.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information
 
 using System;
+using System.Buffers;
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
@@ -12,28 +13,58 @@ namespace Elasticsearch.Net.Extensions
 {
 	internal static class NameValueCollectionExtensions
 	{
+		private const int MaxCharsOnStack = 256; // 512 bytes
+
 		internal static string ToQueryString(this NameValueCollection nv)
 		{
 			if (nv == null || nv.AllKeys.Length == 0) return string.Empty;
 
-			// initialize with capacity for number of key/values with length 5 each
-			var builder = new StringBuilder("?", nv.AllKeys.Length * 2 * 5);
-			for (var i = 0; i < nv.AllKeys.Length; i++)
+			var maxLength = 1 + nv.AllKeys.Length - 1; // account for '?', and any required '&' chars
+			foreach (var key in nv.AllKeys)
 			{
-				if (i != 0)
-					builder.Append("&");
+				var bytes = Encoding.UTF8.GetByteCount(key) + Encoding.UTF8.GetByteCount(nv[key] ?? string.Empty);
+				var maxEncodedSize = bytes * 3; // worst case, assume all bytes are URL escaped to 3 chars
+				maxLength += 1 + maxEncodedSize; // '=' + encoded chars
+			}
 
-				var key = nv.AllKeys[i];
-				builder.Append(Uri.EscapeDataString(key));
-				var value = nv[key];
-				if (!value.IsNullOrEmpty())
+			// prefer stack allocated array for short lengths
+			// note: renting for larger lengths is slightly more efficient since no zeroing occurs
+			char[] rentedFromPool = null;
+			var buffer = maxLength > MaxCharsOnStack
+				? rentedFromPool = ArrayPool<char>.Shared.Rent(maxLength)
+				: stackalloc char[maxLength];
+
+			try
+			{
+				var position = 0;
+				buffer[position++] = '?';
+
+				foreach (var key in nv.AllKeys)
 				{
-					builder.Append("=");
-					builder.Append(Uri.EscapeDataString(nv[key]));
+					if (position != 1)
+						buffer[position++] = '&';
+
+					var escapedKey = Uri.EscapeDataString(key);
+					escapedKey.AsSpan().CopyTo(buffer.Slice(position));
+					position += escapedKey.Length;
+
+					var value = nv[key];
+
+					if (value.IsNullOrEmpty()) continue;
+
+					buffer[position++] = '=';
+					var escapedValue = Uri.EscapeDataString(value);
+					escapedValue.AsSpan().CopyTo(buffer.Slice(position));
+					position += escapedValue.Length;
 				}
-			}
 
-			return builder.ToString();
+				return buffer.Slice(0, position).ToString();
+			}
+			finally
+			{
+				if (rentedFromPool is object)
+					ArrayPool<char>.Shared.Return(rentedFromPool, clearArray: false);
+			}
 		}
 
 		internal static void UpdateFromDictionary(this NameValueCollection queryString, Dictionary<string, object> queryStringUpdates,

tests/Tests/Extensions/NameValueCollectionExtensionsTests.cs

@@ -0,0 +1,47 @@
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using Elasticsearch.Net.Extensions;
+using FluentAssertions;
+using Xunit;
+
+namespace Tests.Extensions
+{
+	public class NameValueCollectionExtensionsTests
+	{
+		[Theory]
+		[MemberData(nameof(QueryStringTestData))]
+		public void ToQueryString_ReturnsExpectedString(NameValueCollection nvc, string expected) => nvc.ToQueryString().Should().Be(expected);
+
+		public static IEnumerable<object[]> QueryStringTestData =>
+			new List<object[]>
+			{
+				new object[] { new NameValueCollection
+				{
+					{ "q", "title:\"The Right Way\" AND mod_date:[20020101 TO 20030101]" },
+					{ "from", "10000" },
+					{ "request_cache", bool.TrueString },
+					{ "size", "100" }
+				}, "?q=title%3A%22The%20Right%20Way%22%20AND%20mod_date%3A%5B20020101%20TO%2020030101%5D&from=10000&request_cache=True&size=100" },
+
+				new object[] { new NameValueCollection
+				{
+					{ "q", "name:john~1 AND (age:[30 TO 40} OR surname:K*) AND -city" },
+				}, "?q=name%3Ajohn~1%20AND%20%28age%3A%5B30%20TO%2040%7D%20OR%20surname%3AK%2A%29%20AND%20-city" },
+
+				new object[] { new NameValueCollection
+				{
+					{ "q", null },
+				}, "?q" },
+
+				new object[] { new NameValueCollection
+				{
+					{ "emoji", "😅"}
+				}, "?emoji=%F0%9F%98%85" },
+
+				new object[] { new NameValueCollection
+				{
+					{ "€", "€"}
+				}, "?%E2%82%AC=%E2%82%AC" }
+			};
+	}
+}