From 793b523f583a079c614ed41cb060a0dba7add0c7 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 6 Apr 2021 16:03:37 -0700 Subject: [PATCH 01/16] Working on dictionary for query --- .../QueryCollectionBenchmarks.cs | 33 +++ src/Http/Http/src/Features/QueryFeature.cs | 172 +++++++++++- .../Http/src/Microsoft.AspNetCore.Http.csproj | 1 + src/Http/Http/src/QueryCollectionInternal.cs | 255 ++++++++++++++++++ .../samples/MapActionSample/Startup.cs | 3 +- .../Microsoft.AspNetCore.WebUtilities.csproj | 1 + src/Http/WebUtilities/src/QueryHelpers.cs | 1 + 7 files changed, 463 insertions(+), 3 deletions(-) create mode 100644 src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs create mode 100644 src/Http/Http/src/QueryCollectionInternal.cs diff --git a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs new file mode 100644 index 000000000000..339c511b3578 --- /dev/null +++ b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs @@ -0,0 +1,33 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.Http +{ + public class QueryCollectionBenchmarks + { + private string _queryString; + + [IterationSetup] + public void Setup() + { + _queryString = "?key1=value1&key2=value2&key3=value3&key4=&key5="; + } + + [Benchmark] + public void ParseNew() + { + var dict = QueryFeature.ParseNullableQueryInternal(_queryString); + } + + [Benchmark] + public void Parse() + { + var dict = QueryHelpers.ParseNullableQuery(_queryString); + } + } +} diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 289f577ee339..e3a298962187 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Http.Features { @@ -69,7 +72,7 @@ public IQueryCollection Query { _original = current; - var result = QueryHelpers.ParseNullableQuery(current); + var result = ParseNullableQueryInternal(current); if (result == null) { @@ -77,7 +80,7 @@ public IQueryCollection Query } else { - _parsedValues = new QueryCollection(result); + _parsedValues = new QueryCollectionInternal(result); } } return _parsedValues; @@ -100,5 +103,170 @@ public IQueryCollection Query } } } + + /// + /// Parse a query string into its component key and value parts. + /// + /// The raw query string value, with or without the leading '?'. + /// A collection of parsed keys and values, null if there are no entries. + internal static AdaptiveCapacityDictionary? ParseNullableQueryInternal(string? queryString) + { + var accumulator = new KvpAccumulator(); + + if (string.IsNullOrEmpty(queryString) || queryString == "?") + { + return null; + } + + int scanIndex = 0; + if (queryString[0] == '?') + { + scanIndex = 1; + } + + int textLength = queryString.Length; + int equalIndex = queryString.IndexOf('='); + if (equalIndex == -1) + { + equalIndex = textLength; + } + while (scanIndex < textLength) + { + int delimiterIndex = queryString.IndexOf('&', scanIndex); + if (delimiterIndex == -1) + { + delimiterIndex = textLength; + } + if (equalIndex < delimiterIndex) + { + while (scanIndex != equalIndex && char.IsWhiteSpace(queryString[scanIndex])) + { + ++scanIndex; + } + string name = queryString.Substring(scanIndex, equalIndex - scanIndex); + string value = queryString.Substring(equalIndex + 1, delimiterIndex - equalIndex - 1); + accumulator.Append( + Uri.UnescapeDataString(name.Replace('+', ' ')), + Uri.UnescapeDataString(value.Replace('+', ' '))); + equalIndex = queryString.IndexOf('=', delimiterIndex); + if (equalIndex == -1) + { + equalIndex = textLength; + } + } + else + { + if (delimiterIndex > scanIndex) + { + accumulator.Append(queryString.Substring(scanIndex, delimiterIndex - scanIndex), string.Empty); + } + } + scanIndex = delimiterIndex + 1; + } + + if (!accumulator.HasValues) + { + return null; + } + + return accumulator.GetResults(); + } + + internal struct KvpAccumulator + { + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + private AdaptiveCapacityDictionary _accumulator; + private AdaptiveCapacityDictionary> _expandingAccumulator; + + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + public void Append(string key, string value) + { + if (_accumulator == null) + { + _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); + } + + StringValues values; + if (_accumulator.TryGetValue(key, out values)) + { + if (values.Count == 0) + { + // Marker entry for this key to indicate entry already in expanding list dictionary + _expandingAccumulator[key].Add(value); + } + else if (values.Count < 5) + { + _accumulator[key] = StringValues.Concat(values, value); + } + else + { + // Add zero count entry and move to data to expanding list dictionary + _accumulator[key] = default(StringValues); + + if (_expandingAccumulator == null) + { + _expandingAccumulator = new AdaptiveCapacityDictionary>(StringComparer.OrdinalIgnoreCase); + } + + // Already 5 entries so use starting allocated as 10; then use List's expansion mechanism for more + var list = new List(10); + + list.AddRange(values); + list.Add(value); + + _expandingAccumulator[key] = list; + } + } + else + { + // First value for this key + _accumulator[key] = new StringValues(value); + } + + ValueCount++; + } + + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + public bool HasValues => ValueCount > 0; + + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + public int KeyCount => _accumulator?.Count ?? 0; + + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + public int ValueCount { get; private set; } + + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + public AdaptiveCapacityDictionary GetResults() + { + if (_expandingAccumulator != null) + { + // Coalesce count 3+ multi-value entries into _accumulator dictionary + foreach (var entry in _expandingAccumulator) + { + _accumulator[entry.Key] = new StringValues(entry.Value.ToArray()); + } + } + + return _accumulator ?? new AdaptiveCapacityDictionary(0, StringComparer.OrdinalIgnoreCase); + } + } } } diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index 669a90b01f12..8f9773316efd 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Http/Http/src/QueryCollectionInternal.cs b/src/Http/Http/src/QueryCollectionInternal.cs new file mode 100644 index 000000000000..db23b2edbc57 --- /dev/null +++ b/src/Http/Http/src/QueryCollectionInternal.cs @@ -0,0 +1,255 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections; +using System.Collections.Generic; +using Microsoft.AspNetCore.Internal; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.Http +{ + /// + /// The HttpRequest query string collection + /// + internal class QueryCollectionInternal : IQueryCollection + { + /// + /// Gets an empty . + /// + public static readonly QueryCollectionInternal Empty = new QueryCollectionInternal(); + private static readonly string[] EmptyKeys = Array.Empty(); + private static readonly StringValues[] EmptyValues = Array.Empty(); + private static readonly Enumerator EmptyEnumerator = new Enumerator(); + // Pre-box + private static readonly IEnumerator> EmptyIEnumeratorType = EmptyEnumerator; + private static readonly IEnumerator EmptyIEnumerator = EmptyEnumerator; + + private AdaptiveCapacityDictionary? Store { get; } + + /// + /// Initializes a new instance of . + /// + public QueryCollectionInternal() + { + } + + /// + /// Initializes a new instance of . + /// + /// The backing store. + internal QueryCollectionInternal(AdaptiveCapacityDictionary store) + { + Store = store; + } + + /// + /// Creates a shallow copy of the specified . + /// + /// The to clone. + public QueryCollectionInternal(QueryCollectionInternal store) + { + Store = store.Store; + } + + /// + /// Initializes a new instance of . + /// + /// The initial number of query items that this instance can contain. + public QueryCollectionInternal(int capacity) + { + Store = new AdaptiveCapacityDictionary(capacity, StringComparer.OrdinalIgnoreCase); + } + + /// + /// Gets the associated set of values from the collection. + /// + /// The key name. + /// the associated value from the collection as a StringValues or StringValues.Empty if the key is not present. + public StringValues this[string key] + { + get + { + if (Store == null) + { + return StringValues.Empty; + } + + if (TryGetValue(key, out var value)) + { + return value; + } + return StringValues.Empty; + } + } + + /// + /// Gets the number of elements contained in the ;. + /// + /// The number of elements contained in the . + public int Count + { + get + { + if (Store == null) + { + return 0; + } + return Store.Count; + } + } + + /// + /// Gets the collection of query names in this instance. + /// + public ICollection Keys + { + get + { + if (Store == null) + { + return EmptyKeys; + } + return Store.Keys; + } + } + + /// + /// Determines whether the contains a specific key. + /// + /// The key. + /// true if the contains a specific key; otherwise, false. + public bool ContainsKey(string key) + { + if (Store == null) + { + return false; + } + return Store.ContainsKey(key); + } + + /// + /// Retrieves a value from the collection. + /// + /// The key. + /// The value. + /// true if the contains the key; otherwise, false. + public bool TryGetValue(string key, out StringValues value) + { + if (Store == null) + { + value = default(StringValues); + return false; + } + return Store.TryGetValue(key, out value); + } + + /// + /// Returns an enumerator that iterates through a collection. + /// + /// An object that can be used to iterate through the collection. + public Enumerator GetEnumerator() + { + if (Store == null || Store.Count == 0) + { + // Non-boxed Enumerator + return EmptyEnumerator; + } + return new Enumerator(Store.GetEnumerator()); + } + + /// + /// Returns an enumerator that iterates through a collection. + /// + /// An object that can be used to iterate through the collection. + IEnumerator> IEnumerable>.GetEnumerator() + { + if (Store == null || Store.Count == 0) + { + // Non-boxed Enumerator + return EmptyIEnumeratorType; + } + return Store.GetEnumerator(); + } + + /// + /// Returns an enumerator that iterates through a collection. + /// + /// An object that can be used to iterate through the collection. + IEnumerator IEnumerable.GetEnumerator() + { + if (Store == null || Store.Count == 0) + { + // Non-boxed Enumerator + return EmptyIEnumerator; + } + return Store.GetEnumerator(); + } + + /// + /// Enumerates a . + /// + public struct Enumerator : IEnumerator> + { + // Do NOT make this readonly, or MoveNext will not work + private AdaptiveCapacityDictionary.Enumerator _dictionaryEnumerator; + private bool _notEmpty; + + internal Enumerator(AdaptiveCapacityDictionary.Enumerator dictionaryEnumerator) + { + _dictionaryEnumerator = dictionaryEnumerator; + _notEmpty = true; + } + + /// + /// Advances the enumerator to the next element of the . + /// + /// if the enumerator was successfully advanced to the next element; + /// if the enumerator has passed the end of the collection. + public bool MoveNext() + { + if (_notEmpty) + { + return _dictionaryEnumerator.MoveNext(); + } + return false; + } + + /// + /// Gets the element at the current position of the enumerator. + /// + public KeyValuePair Current + { + get + { + if (_notEmpty) + { + return _dictionaryEnumerator.Current; + } + return default(KeyValuePair); + } + } + + /// + public void Dispose() + { + } + + object IEnumerator.Current + { + get + { + return Current; + } + } + + void IEnumerator.Reset() + { + if (_notEmpty) + { + ((IEnumerator)_dictionaryEnumerator).Reset(); + } + } + } + } +} diff --git a/src/Http/Routing/samples/MapActionSample/Startup.cs b/src/Http/Routing/samples/MapActionSample/Startup.cs index e3c44398834b..b349611d0da2 100644 --- a/src/Http/Routing/samples/MapActionSample/Startup.cs +++ b/src/Http/Routing/samples/MapActionSample/Startup.cs @@ -40,7 +40,8 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) endpoints.MapGet("/", async context => { - await context.Response.WriteAsync("Hello World!"); + var res = context.Request.Query["foo"]; + await context.Response.WriteAsync(res.ToString()); }); }); } diff --git a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj index 95b496204d4e..a849caa35ea4 100644 --- a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj +++ b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj @@ -14,6 +14,7 @@ + diff --git a/src/Http/WebUtilities/src/QueryHelpers.cs b/src/Http/WebUtilities/src/QueryHelpers.cs index 137035b34076..d72975a5c66b 100644 --- a/src/Http/WebUtilities/src/QueryHelpers.cs +++ b/src/Http/WebUtilities/src/QueryHelpers.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Text; using System.Text.Encodings.Web; +using Microsoft.AspNetCore.Internal; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.WebUtilities From ec818d0b67f098516a17e1bb3c24de7bbbe5ca94 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 7 Apr 2021 10:32:18 -0700 Subject: [PATCH 02/16] Make initial capacity smaller when not specifying capacity --- .../QueryCollectionBenchmarks.cs | 19 +++++++++++++++++++ src/Http/Http/src/Features/QueryFeature.cs | 3 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs index 339c511b3578..757f517ca91c 100644 --- a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs +++ b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs @@ -5,17 +5,20 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Primitives; +using static Microsoft.AspNetCore.Http.Features.QueryFeature; namespace Microsoft.AspNetCore.Http { public class QueryCollectionBenchmarks { private string _queryString; + private string _singleValue; [IterationSetup] public void Setup() { _queryString = "?key1=value1&key2=value2&key3=value3&key4=&key5="; + _singleValue = "?key1=value1"; } [Benchmark] @@ -24,10 +27,26 @@ public void ParseNew() var dict = QueryFeature.ParseNullableQueryInternal(_queryString); } + [Benchmark] + public void ParseNewSingle() + { + var dict = QueryFeature.ParseNullableQueryInternal(_singleValue); + } + [Benchmark] public void Parse() { var dict = QueryHelpers.ParseNullableQuery(_queryString); } + + [Benchmark] + public void Constructor() + { + var dict = new KvpAccumulator(); + if (dict.HasValues) + { + return; + } + } } } diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index e3a298962187..3d56c5d05f3b 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -148,6 +148,7 @@ public IQueryCollection Query accumulator.Append( Uri.UnescapeDataString(name.Replace('+', ' ')), Uri.UnescapeDataString(value.Replace('+', ' '))); + equalIndex = queryString.IndexOf('=', delimiterIndex); if (equalIndex == -1) { @@ -189,7 +190,7 @@ public void Append(string key, string value) { if (_accumulator == null) { - _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); + _accumulator = new AdaptiveCapacityDictionary(capacity: 5, StringComparer.OrdinalIgnoreCase); } StringValues values; From 18d4aff9aeeeb9b57db3f035f7259c5f2bfe9bf5 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 7 Apr 2021 10:36:41 -0700 Subject: [PATCH 03/16] Change defaults for dictionary --- src/Http/Http/src/Features/QueryFeature.cs | 2 +- src/Shared/Dictionary/AdaptiveCapacityDictionary.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 3d56c5d05f3b..2b55a44be4f6 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -190,7 +190,7 @@ public void Append(string key, string value) { if (_accumulator == null) { - _accumulator = new AdaptiveCapacityDictionary(capacity: 5, StringComparer.OrdinalIgnoreCase); + _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); } StringValues values; diff --git a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs index d039c4ffbbbd..112cd1c923c9 100644 --- a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs +++ b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs @@ -553,7 +553,8 @@ private void EnsureCapacitySlow(int capacity) } else { - capacity = _arrayStorage.Length == 0 ? DefaultArrayThreshold : _arrayStorage.Length * 2; + // Initial capacity is 5, grows to 10 once. + capacity = _arrayStorage.Length == 0 ? DefaultArrayThreshold / 2 : _arrayStorage.Length * 2; var array = new KeyValuePair[capacity]; if (_count > 0) { From 11c3afa4c7518242f4ba534f09fbfd7a8cceb2d9 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 7 Apr 2021 10:58:20 -0700 Subject: [PATCH 04/16] Updating some benchmarks --- src/Http/Http/src/Features/QueryFeature.cs | 5 ++--- src/Http/Routing/samples/MapActionSample/Startup.cs | 3 +-- .../src/Microsoft.AspNetCore.WebUtilities.csproj | 1 - src/Http/WebUtilities/src/QueryHelpers.cs | 1 - src/Shared/Dictionary/AdaptiveCapacityDictionary.cs | 2 +- 5 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 2b55a44be4f6..42f240ba1ba0 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -148,7 +148,6 @@ public IQueryCollection Query accumulator.Append( Uri.UnescapeDataString(name.Replace('+', ' ')), Uri.UnescapeDataString(value.Replace('+', ' '))); - equalIndex = queryString.IndexOf('=', delimiterIndex); if (equalIndex == -1) { @@ -201,7 +200,7 @@ public void Append(string key, string value) // Marker entry for this key to indicate entry already in expanding list dictionary _expandingAccumulator[key].Add(value); } - else if (values.Count < 5) + else if (values.Count == 1) { _accumulator[key] = StringValues.Concat(values, value); } @@ -212,7 +211,7 @@ public void Append(string key, string value) if (_expandingAccumulator == null) { - _expandingAccumulator = new AdaptiveCapacityDictionary>(StringComparer.OrdinalIgnoreCase); + _expandingAccumulator = new AdaptiveCapacityDictionary>(5, StringComparer.OrdinalIgnoreCase); } // Already 5 entries so use starting allocated as 10; then use List's expansion mechanism for more diff --git a/src/Http/Routing/samples/MapActionSample/Startup.cs b/src/Http/Routing/samples/MapActionSample/Startup.cs index b349611d0da2..e3c44398834b 100644 --- a/src/Http/Routing/samples/MapActionSample/Startup.cs +++ b/src/Http/Routing/samples/MapActionSample/Startup.cs @@ -40,8 +40,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) endpoints.MapGet("/", async context => { - var res = context.Request.Query["foo"]; - await context.Response.WriteAsync(res.ToString()); + await context.Response.WriteAsync("Hello World!"); }); }); } diff --git a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj index a849caa35ea4..95b496204d4e 100644 --- a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj +++ b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj @@ -14,7 +14,6 @@ - diff --git a/src/Http/WebUtilities/src/QueryHelpers.cs b/src/Http/WebUtilities/src/QueryHelpers.cs index d72975a5c66b..137035b34076 100644 --- a/src/Http/WebUtilities/src/QueryHelpers.cs +++ b/src/Http/WebUtilities/src/QueryHelpers.cs @@ -6,7 +6,6 @@ using System.Linq; using System.Text; using System.Text.Encodings.Web; -using Microsoft.AspNetCore.Internal; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.WebUtilities diff --git a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs index 112cd1c923c9..2028be3b1f15 100644 --- a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs +++ b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs @@ -554,7 +554,7 @@ private void EnsureCapacitySlow(int capacity) else { // Initial capacity is 5, grows to 10 once. - capacity = _arrayStorage.Length == 0 ? DefaultArrayThreshold / 2 : _arrayStorage.Length * 2; + capacity = _arrayStorage.Length == 0 ? DefaultArrayThreshold : _arrayStorage.Length * 2; var array = new KeyValuePair[capacity]; if (_count > 0) { From 8012a7211c4237cd420b64e55d210f4f0cb2a972 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 7 Apr 2021 11:02:49 -0700 Subject: [PATCH 05/16] remove comment --- src/Shared/Dictionary/AdaptiveCapacityDictionary.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs index 2028be3b1f15..d039c4ffbbbd 100644 --- a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs +++ b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs @@ -553,7 +553,6 @@ private void EnsureCapacitySlow(int capacity) } else { - // Initial capacity is 5, grows to 10 once. capacity = _arrayStorage.Length == 0 ? DefaultArrayThreshold : _arrayStorage.Length * 2; var array = new KeyValuePair[capacity]; if (_count > 0) From 446855954299e67b3bb6974aae34896926fbf01c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Tue, 18 May 2021 20:57:12 +0200 Subject: [PATCH 06/16] Updated benchmarks --- .../QueryCollectionBenchmarks.cs | 45 +++++++++++++++---- .../RequestCookieCollectionBenchmarks.cs | 2 +- .../RouteValueDictionaryBenchmark.cs | 4 +- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs index 757f517ca91c..73d00daa4692 100644 --- a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs +++ b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs @@ -2,44 +2,73 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.Primitives; using static Microsoft.AspNetCore.Http.Features.QueryFeature; namespace Microsoft.AspNetCore.Http { + [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)] + [CategoriesColumn] public class QueryCollectionBenchmarks { private string _queryString; private string _singleValue; + private string _singleValueWithPlus; [IterationSetup] public void Setup() { _queryString = "?key1=value1&key2=value2&key3=value3&key4=&key5="; _singleValue = "?key1=value1"; + _singleValueWithPlus = "?key1=value1+value2+value3"; } - [Benchmark] + [Benchmark(Description = "ParseNew")] + [BenchmarkCategory("QueryString")] public void ParseNew() { - var dict = QueryFeature.ParseNullableQueryInternal(_queryString); + _ = QueryFeature.ParseNullableQueryInternal(_queryString); } - [Benchmark] + [Benchmark(Description = "ParseNew")] + [BenchmarkCategory("Single")] public void ParseNewSingle() { - var dict = QueryFeature.ParseNullableQueryInternal(_singleValue); + _ = QueryFeature.ParseNullableQueryInternal(_singleValue); } - [Benchmark] - public void Parse() + [Benchmark(Description = "ParseNew")] + [BenchmarkCategory("SingleWithPlus")] + public void ParseNewSingleWithPlus() + { + _ = QueryFeature.ParseNullableQueryInternal(_singleValueWithPlus); + } + + [Benchmark(Description = "QueryHelpersParse")] + [BenchmarkCategory("QueryString")] + public void QueryHelpersParse() + { + _ = QueryHelpers.ParseNullableQuery(_queryString); + } + + [Benchmark(Description = "QueryHelpersParse")] + [BenchmarkCategory("Single")] + public void QueryHelpersParseSingle() + { + _ = QueryHelpers.ParseNullableQuery(_singleValue); + } + + [Benchmark(Description = "QueryHelpersParse")] + [BenchmarkCategory("SingleWithPlus")] + public void QueryHelpersParseSingleWithPlus() { - var dict = QueryHelpers.ParseNullableQuery(_queryString); + _ = QueryHelpers.ParseNullableQuery(_singleValueWithPlus); } [Benchmark] + [BenchmarkCategory("Constructor")] public void Constructor() { var dict = new KvpAccumulator(); diff --git a/src/Http/Http/perf/Microbenchmarks/RequestCookieCollectionBenchmarks.cs b/src/Http/Http/perf/Microbenchmarks/RequestCookieCollectionBenchmarks.cs index 035a367b1324..41aea5de4e60 100644 --- a/src/Http/Http/perf/Microbenchmarks/RequestCookieCollectionBenchmarks.cs +++ b/src/Http/Http/perf/Microbenchmarks/RequestCookieCollectionBenchmarks.cs @@ -19,7 +19,7 @@ public void Setup() [Benchmark] public void Parse_TypicalCookie() { - RequestCookieCollection.Parse(_cookie); + _ = RequestCookieCollection.Parse(_cookie); } } } diff --git a/src/Http/Http/perf/Microbenchmarks/RouteValueDictionaryBenchmark.cs b/src/Http/Http/perf/Microbenchmarks/RouteValueDictionaryBenchmark.cs index 76a8659c2a48..f1bc86c68a75 100644 --- a/src/Http/Http/perf/Microbenchmarks/RouteValueDictionaryBenchmark.cs +++ b/src/Http/Http/perf/Microbenchmarks/RouteValueDictionaryBenchmark.cs @@ -33,9 +33,9 @@ public void Ctor_Values_RouteValueDictionary_EmptyArray() } [Benchmark] - public void Ctor_Values_RouteValueDictionary_Array() + public RouteValueDictionary Ctor_Values_RouteValueDictionary_Array() { - new RouteValueDictionary(_arrayValues); + return new RouteValueDictionary(_arrayValues); } [Benchmark] From ed2afafda5b0e64390f77dd8eee0db2c0634d63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Thu, 8 Apr 2021 14:53:29 +0200 Subject: [PATCH 07/16] Span-based version for ParseNullableQueryInternal --- src/Http/Http/src/Features/QueryFeature.cs | 137 +++++++++++++++------ 1 file changed, 102 insertions(+), 35 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 42f240ba1ba0..1cce150878bd 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -2,7 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Buffers; using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.X86; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Primitives; @@ -109,59 +113,77 @@ public IQueryCollection Query /// /// The raw query string value, with or without the leading '?'. /// A collection of parsed keys and values, null if there are no entries. + [SkipLocalsInit] internal static AdaptiveCapacityDictionary? ParseNullableQueryInternal(string? queryString) { - var accumulator = new KvpAccumulator(); - - if (string.IsNullOrEmpty(queryString) || queryString == "?") + if (string.IsNullOrEmpty(queryString) || (queryString.Length == 1 && queryString[0] == '?')) { return null; } - int scanIndex = 0; - if (queryString[0] == '?') - { - scanIndex = 1; - } + KvpAccumulator accumulator = new(); + int queryStringLength = queryString.Length; - int textLength = queryString.Length; - int equalIndex = queryString.IndexOf('='); - if (equalIndex == -1) + char[]? arryToReturnToPool = null; + Span query = (queryStringLength <= 128 + ? stackalloc char[128] + : arryToReturnToPool = ArrayPool.Shared.Rent(queryStringLength) + ).Slice(0, queryStringLength); + + queryString.AsSpan().CopyTo(query); + + if (query[0] == '?') { - equalIndex = textLength; + query = query[1..]; } - while (scanIndex < textLength) + + while (!query.IsEmpty) { - int delimiterIndex = queryString.IndexOf('&', scanIndex); - if (delimiterIndex == -1) - { - delimiterIndex = textLength; - } - if (equalIndex < delimiterIndex) + int delimiterIndex = query.IndexOf('&'); + + Span querySegment = delimiterIndex >= 0 + ? query.Slice(0, delimiterIndex) + : query; + + int equalIndex = querySegment.IndexOf('='); + + if (equalIndex >= 0) { - while (scanIndex != equalIndex && char.IsWhiteSpace(queryString[scanIndex])) + int i = 0; + for (; i < querySegment.Length; ++i) { - ++scanIndex; + if (!char.IsWhiteSpace(querySegment[i])) + { + break; + } } - string name = queryString.Substring(scanIndex, equalIndex - scanIndex); - string value = queryString.Substring(equalIndex + 1, delimiterIndex - equalIndex - 1); + + Span name = querySegment[i..equalIndex]; + Span value = querySegment.Slice(equalIndex + 1); + + name.ReplacePlusWithSpaceInPlace(); + value.ReplacePlusWithSpaceInPlace(); + accumulator.Append( - Uri.UnescapeDataString(name.Replace('+', ' ')), - Uri.UnescapeDataString(value.Replace('+', ' '))); - equalIndex = queryString.IndexOf('=', delimiterIndex); - if (equalIndex == -1) - { - equalIndex = textLength; - } + Uri.UnescapeDataString(name.ToString()), + Uri.UnescapeDataString(value.ToString())); } else { - if (delimiterIndex > scanIndex) - { - accumulator.Append(queryString.Substring(scanIndex, delimiterIndex - scanIndex), string.Empty); - } + accumulator.Append(querySegment); + } + + if (delimiterIndex < 0) + { + break; } - scanIndex = delimiterIndex + 1; + + query = query.Slice(delimiterIndex + 1); + } + + if (arryToReturnToPool is not null) + { + ArrayPool.Shared.Return(arryToReturnToPool); } if (!accumulator.HasValues) @@ -181,6 +203,9 @@ internal struct KvpAccumulator private AdaptiveCapacityDictionary _accumulator; private AdaptiveCapacityDictionary> _expandingAccumulator; + public void Append(ReadOnlySpan key, ReadOnlySpan value = default) + => Append(key.ToString(), value.IsEmpty ? string.Empty : value.ToString()); + /// /// This API supports infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. @@ -269,4 +294,46 @@ public AdaptiveCapacityDictionary GetResults() } } } + + internal static class MySpanExtensions + { + public static void ReplacePlusWithSpaceInPlace(this Span span) + => ReplaceInPlace(span, '+', ' '); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void ReplaceInPlace(this Span span, char oldChar, char newChar) + { + nint i = 0; + nint n = (nint)(uint)span.Length; + + fixed (char* ptr = span) + { + ushort* pVec = (ushort*)ptr; + + if (Sse41.IsSupported && n >= Vector128.Count) + { + Vector128 vecOldChar = Vector128.Create((ushort)oldChar); + Vector128 vecNewChar = Vector128.Create((ushort)newChar); + + do + { + Vector128 vec = Sse2.LoadVector128(pVec + i); + Vector128 mask = Sse2.CompareEqual(vec, vecOldChar); + Vector128 res = Sse41.BlendVariable(vec, vecNewChar, mask); + Sse2.Store(pVec + i, res); + + i += Vector128.Count; + } while (i <= n - Vector128.Count); + } + + for (; i < n; ++i) + { + if (ptr[i] == oldChar) + { + ptr[i] = newChar; + } + } + } + } + } } From 05ebdcf16b4be87f5ea6b70465c6f39e27ae857f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Tue, 18 May 2021 21:30:43 +0200 Subject: [PATCH 08/16] Cleaned span based version --- src/Http/Http/src/Features/QueryFeature.cs | 82 +++++++++++----------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 1cce150878bd..84f68bc98bae 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -8,7 +8,6 @@ using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.X86; using Microsoft.AspNetCore.Internal; -using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Http.Features @@ -122,7 +121,7 @@ public IQueryCollection Query } KvpAccumulator accumulator = new(); - int queryStringLength = queryString.Length; + var queryStringLength = queryString.Length; char[]? arryToReturnToPool = null; Span query = (queryStringLength <= 128 @@ -139,17 +138,17 @@ public IQueryCollection Query while (!query.IsEmpty) { - int delimiterIndex = query.IndexOf('&'); + var delimiterIndex = query.IndexOf('&'); - Span querySegment = delimiterIndex >= 0 + var querySegment = delimiterIndex >= 0 ? query.Slice(0, delimiterIndex) : query; - int equalIndex = querySegment.IndexOf('='); + var equalIndex = querySegment.IndexOf('='); if (equalIndex >= 0) { - int i = 0; + var i = 0; for (; i < querySegment.Length; ++i) { if (!char.IsWhiteSpace(querySegment[i])) @@ -158,11 +157,11 @@ public IQueryCollection Query } } - Span name = querySegment[i..equalIndex]; - Span value = querySegment.Slice(equalIndex + 1); + var name = querySegment[i..equalIndex]; + var value = querySegment.Slice(equalIndex + 1); - name.ReplacePlusWithSpaceInPlace(); - value.ReplacePlusWithSpaceInPlace(); + SpanHelper.ReplacePlusWithSpaceInPlace(name); + SpanHelper.ReplacePlusWithSpaceInPlace(value); accumulator.Append( Uri.UnescapeDataString(name.ToString()), @@ -212,13 +211,12 @@ public void Append(ReadOnlySpan key, ReadOnlySpan value = default) /// public void Append(string key, string value) { - if (_accumulator == null) + if (_accumulator is null) { _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); } - StringValues values; - if (_accumulator.TryGetValue(key, out values)) + if (_accumulator.TryGetValue(key, out var values)) { if (values.Count == 0) { @@ -232,9 +230,9 @@ public void Append(string key, string value) else { // Add zero count entry and move to data to expanding list dictionary - _accumulator[key] = default(StringValues); + _accumulator[key] = default; - if (_expandingAccumulator == null) + if (_expandingAccumulator is null) { _expandingAccumulator = new AdaptiveCapacityDictionary>(5, StringComparer.OrdinalIgnoreCase); } @@ -293,44 +291,44 @@ public AdaptiveCapacityDictionary GetResults() return _accumulator ?? new AdaptiveCapacityDictionary(0, StringComparer.OrdinalIgnoreCase); } } - } - - internal static class MySpanExtensions - { - public static void ReplacePlusWithSpaceInPlace(this Span span) - => ReplaceInPlace(span, '+', ' '); - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static unsafe void ReplaceInPlace(this Span span, char oldChar, char newChar) + private static class SpanHelper { - nint i = 0; - nint n = (nint)(uint)span.Length; + public static void ReplacePlusWithSpaceInPlace(Span span) + => ReplaceInPlace(span, '+', ' '); - fixed (char* ptr = span) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void ReplaceInPlace(Span span, char oldChar, char newChar) { - ushort* pVec = (ushort*)ptr; + var i = (nint)0; + var n = (nint)(uint)span.Length; - if (Sse41.IsSupported && n >= Vector128.Count) + fixed (char* ptr = span) { - Vector128 vecOldChar = Vector128.Create((ushort)oldChar); - Vector128 vecNewChar = Vector128.Create((ushort)newChar); + var pVec = (ushort*)ptr; - do + if (Sse41.IsSupported && n >= Vector128.Count) { - Vector128 vec = Sse2.LoadVector128(pVec + i); - Vector128 mask = Sse2.CompareEqual(vec, vecOldChar); - Vector128 res = Sse41.BlendVariable(vec, vecNewChar, mask); - Sse2.Store(pVec + i, res); + var vecOldChar = Vector128.Create((ushort)oldChar); + var vecNewChar = Vector128.Create((ushort)newChar); - i += Vector128.Count; - } while (i <= n - Vector128.Count); - } + do + { + var vec = Sse2.LoadVector128(pVec + i); + var mask = Sse2.CompareEqual(vec, vecOldChar); + var res = Sse41.BlendVariable(vec, vecNewChar, mask); + Sse2.Store(pVec + i, res); - for (; i < n; ++i) - { - if (ptr[i] == oldChar) + i += Vector128.Count; + } while (i <= n - Vector128.Count); + } + + for (; i < n; ++i) { - ptr[i] = newChar; + if (ptr[i] == oldChar) + { + ptr[i] = newChar; + } } } } From b06ca3a5a6df16a68484544066aaa397e8c78f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Tue, 18 May 2021 21:53:04 +0200 Subject: [PATCH 09/16] Fixed test failure --- src/Http/Http/src/Features/QueryFeature.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 84f68bc98bae..86dad97988dc 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -61,13 +61,9 @@ public IQueryCollection Query { get { - if (_features.Collection == null) + if (_features.Collection is null) { - if (_parsedValues == null) - { - _parsedValues = QueryCollection.Empty; - } - return _parsedValues; + return _parsedValues ?? QueryCollection.Empty; } var current = HttpRequestFeature.QueryString; @@ -211,6 +207,11 @@ public void Append(ReadOnlySpan key, ReadOnlySpan value = default) /// public void Append(string key, string value) { + if (key.Length == 0) + { + return; + } + if (_accumulator is null) { _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); From 3685ee2e0ab10f1ac76602bdd3c702b604bdd8bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Wed, 19 May 2021 12:25:14 +0200 Subject: [PATCH 10/16] Added benchmark case for % encoded values --- .../Microbenchmarks/QueryCollectionBenchmarks.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs index 73d00daa4692..cc165ee1e430 100644 --- a/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs +++ b/src/Http/Http/perf/Microbenchmarks/QueryCollectionBenchmarks.cs @@ -16,6 +16,7 @@ public class QueryCollectionBenchmarks private string _queryString; private string _singleValue; private string _singleValueWithPlus; + private string _encoded; [IterationSetup] public void Setup() @@ -23,6 +24,7 @@ public void Setup() _queryString = "?key1=value1&key2=value2&key3=value3&key4=&key5="; _singleValue = "?key1=value1"; _singleValueWithPlus = "?key1=value1+value2+value3"; + _encoded = "?key1=value%231"; } [Benchmark(Description = "ParseNew")] @@ -46,6 +48,13 @@ public void ParseNewSingleWithPlus() _ = QueryFeature.ParseNullableQueryInternal(_singleValueWithPlus); } + [Benchmark(Description = "ParseNew")] + [BenchmarkCategory("Encoded")] + public void ParseNewEncoded() + { + _ = QueryFeature.ParseNullableQueryInternal(_encoded); + } + [Benchmark(Description = "QueryHelpersParse")] [BenchmarkCategory("QueryString")] public void QueryHelpersParse() @@ -67,6 +76,13 @@ public void QueryHelpersParseSingleWithPlus() _ = QueryHelpers.ParseNullableQuery(_singleValueWithPlus); } + [Benchmark(Description = "QueryHelpersParse")] + [BenchmarkCategory("Encoded")] + public void QueryHelpersParseEncoded() + { + _ = QueryHelpers.ParseNullableQuery(_encoded); + } + [Benchmark] [BenchmarkCategory("Constructor")] public void Constructor() From a6353d151439e1ab7aa08e1a02fa660e6d1dcff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Wed, 19 May 2021 12:49:59 +0200 Subject: [PATCH 11/16] Port over tests from QueryHelpersTests + fixes --- src/Http/Http/src/Features/QueryFeature.cs | 21 ++--- .../Http/test/Features/QueryFeatureTests.cs | 90 +++++++++++++++++-- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 86dad97988dc..14a4264bc740 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -73,14 +73,9 @@ public IQueryCollection Query var result = ParseNullableQueryInternal(current); - if (result == null) - { - _parsedValues = QueryCollection.Empty; - } - else - { - _parsedValues = new QueryCollectionInternal(result); - } + _parsedValues = result is not null + ? new QueryCollectionInternal(result) + : QueryCollection.Empty; } return _parsedValues; } @@ -165,7 +160,10 @@ public IQueryCollection Query } else { - accumulator.Append(querySegment); + if (!querySegment.IsEmpty) + { + accumulator.Append(querySegment); + } } if (delimiterIndex < 0) @@ -207,11 +205,6 @@ public void Append(ReadOnlySpan key, ReadOnlySpan value = default) /// public void Append(string key, string value) { - if (key.Length == 0) - { - return; - } - if (_accumulator is null) { _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); diff --git a/src/Http/Http/test/Features/QueryFeatureTests.cs b/src/Http/Http/test/Features/QueryFeatureTests.cs index e43e3ce7a976..61e8dfbca6aa 100644 --- a/src/Http/Http/test/Features/QueryFeatureTests.cs +++ b/src/Http/Http/test/Features/QueryFeatureTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Linq; using Xunit; namespace Microsoft.AspNetCore.Http.Features @@ -12,9 +13,7 @@ public void QueryReturnsParsedQueryCollection() { // Arrange var features = new FeatureCollection(); - var request = new HttpRequestFeature(); - request.QueryString = "foo=bar"; - features[typeof(IHttpRequestFeature)] = request; + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "foo=bar" }; var provider = new QueryFeature(features); @@ -25,6 +24,23 @@ public void QueryReturnsParsedQueryCollection() Assert.Equal("bar", queryCollection["foo"]); } + [Theory] + [InlineData("?key1=value1&key2=value2")] + [InlineData("key1=value1&key2=value2")] + public void ParseQueryWithUniqueKeysWorks(string queryString) + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = queryString }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Equal(2, queryCollection.Count); + Assert.Equal("value1", queryCollection["key1"].FirstOrDefault()); + Assert.Equal("value2", queryCollection["key2"].FirstOrDefault()); + } + [Theory] [InlineData("?q", "q")] [InlineData("?q&", "q")] @@ -34,9 +50,7 @@ public void QueryReturnsParsedQueryCollection() public void KeyWithoutValuesAddedToQueryCollection(string queryString, string emptyParam) { var features = new FeatureCollection(); - var request = new HttpRequestFeature(); - request.QueryString = queryString; - features[typeof(IHttpRequestFeature)] = request; + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = queryString }; var provider = new QueryFeature(features); @@ -53,9 +67,7 @@ public void KeyWithoutValuesAddedToQueryCollection(string queryString, string em public void EmptyKeysNotAddedToQueryCollection(string queryString) { var features = new FeatureCollection(); - var request = new HttpRequestFeature(); - request.QueryString = queryString; - features[typeof(IHttpRequestFeature)] = request; + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = queryString }; var provider = new QueryFeature(features); @@ -63,5 +75,65 @@ public void EmptyKeysNotAddedToQueryCollection(string queryString) Assert.Equal(0, queryCollection.Count); } + + [Fact] + public void ParseQueryWithEmptyKeyWorks() + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "?=value1&=" }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Single(queryCollection); + Assert.Equal(new[] { "value1", "" }, queryCollection[""]); + } + + [Fact] + public void ParseQueryWithDuplicateKeysGroups() + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "?key1=valueA&key2=valueB&key1=valueC" }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Equal(2, queryCollection.Count); + Assert.Equal(new[] { "valueA", "valueC" }, queryCollection["key1"]); + Assert.Equal("valueB", queryCollection["key2"].FirstOrDefault()); + } + + [Fact] + public void ParseQueryWithEmptyValuesWorks() + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "?key1=&key2=" }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Equal(2, queryCollection.Count); + Assert.Equal(string.Empty, queryCollection["key1"].FirstOrDefault()); + Assert.Equal(string.Empty, queryCollection["key2"].FirstOrDefault()); + } + + [Theory] + [InlineData("?")] + [InlineData("")] + [InlineData(null)] + public void ParseEmptyOrNullQueryWorks(string queryString) + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = queryString }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Empty(queryCollection); + } } } From b9f61a07ec5965cf7ecf56426268ccb20ee29ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Wed, 19 May 2021 14:04:34 +0200 Subject: [PATCH 12/16] Combine ReplacePlusWithSpace and string materialization and void span-copying to a scratch-buffer --- src/Http/Http/src/Features/QueryFeature.cs | 81 ++++++++++------------ 1 file changed, 37 insertions(+), 44 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 14a4264bc740..b47b4746c1a3 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -5,6 +5,7 @@ using System.Buffers; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.X86; using Microsoft.AspNetCore.Internal; @@ -111,16 +112,8 @@ public IQueryCollection Query return null; } - KvpAccumulator accumulator = new(); - var queryStringLength = queryString.Length; - - char[]? arryToReturnToPool = null; - Span query = (queryStringLength <= 128 - ? stackalloc char[128] - : arryToReturnToPool = ArrayPool.Shared.Rent(queryStringLength) - ).Slice(0, queryStringLength); - - queryString.AsSpan().CopyTo(query); + var accumulator = new KvpAccumulator(); + var query = queryString.AsSpan(); if (query[0] == '?') { @@ -148,15 +141,12 @@ public IQueryCollection Query } } - var name = querySegment[i..equalIndex]; - var value = querySegment.Slice(equalIndex + 1); - - SpanHelper.ReplacePlusWithSpaceInPlace(name); - SpanHelper.ReplacePlusWithSpaceInPlace(value); + var name = SpanHelper.ReplacePlusWithSpace(querySegment[i..equalIndex]); + var value = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(equalIndex + 1)); accumulator.Append( - Uri.UnescapeDataString(name.ToString()), - Uri.UnescapeDataString(value.ToString())); + Uri.UnescapeDataString(name), + Uri.UnescapeDataString(value)); } else { @@ -174,17 +164,9 @@ public IQueryCollection Query query = query.Slice(delimiterIndex + 1); } - if (arryToReturnToPool is not null) - { - ArrayPool.Shared.Return(arryToReturnToPool); - } - - if (!accumulator.HasValues) - { - return null; - } - - return accumulator.GetResults(); + return accumulator.HasValues + ? accumulator.GetResults() + : null; } internal struct KvpAccumulator @@ -288,40 +270,51 @@ public AdaptiveCapacityDictionary GetResults() private static class SpanHelper { - public static void ReplacePlusWithSpaceInPlace(Span span) - => ReplaceInPlace(span, '+', ' '); + private static readonly SpanAction s_replacePlusWithSpace = ReplacePlusWithSpaceCore; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static unsafe void ReplaceInPlace(Span span, char oldChar, char newChar) + public static unsafe string ReplacePlusWithSpace(ReadOnlySpan span) { - var i = (nint)0; - var n = (nint)(uint)span.Length; + fixed (char* ptr = &MemoryMarshal.GetReference(span)) + { + return string.Create(span.Length, (IntPtr)ptr, s_replacePlusWithSpace); + } + } - fixed (char* ptr = span) + private static unsafe void ReplacePlusWithSpaceCore(Span buffer, IntPtr state) + { + fixed (char* ptr = &MemoryMarshal.GetReference(buffer)) { - var pVec = (ushort*)ptr; + var input = (ushort*)state.ToPointer(); + var output = (ushort*)ptr; + + var i = (nint)0; + var n = (nint)(uint)buffer.Length; if (Sse41.IsSupported && n >= Vector128.Count) { - var vecOldChar = Vector128.Create((ushort)oldChar); - var vecNewChar = Vector128.Create((ushort)newChar); + var vecPlus = Vector128.Create((ushort)'+'); + var vecSpace = Vector128.Create((ushort)' '); do { - var vec = Sse2.LoadVector128(pVec + i); - var mask = Sse2.CompareEqual(vec, vecOldChar); - var res = Sse41.BlendVariable(vec, vecNewChar, mask); - Sse2.Store(pVec + i, res); - + var vec = Sse2.LoadVector128(input + i); + var mask = Sse2.CompareEqual(vec, vecPlus); + var res = Sse41.BlendVariable(vec, vecSpace, mask); + Sse2.Store(output + i, res); i += Vector128.Count; } while (i <= n - Vector128.Count); } for (; i < n; ++i) { - if (ptr[i] == oldChar) + if (input[i] != '+') + { + output[i] = input[i]; + } + else { - ptr[i] = newChar; + output[i] = ' '; } } } From 41a4dd7bf6a1d72a28497fca932e4aa36e5a8276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Thu, 20 May 2021 13:11:51 +0200 Subject: [PATCH 13/16] PR feedback --- src/Http/Http/src/Features/QueryFeature.cs | 18 +-- src/Http/Http/src/QueryCollectionInternal.cs | 148 ++---------------- .../Http/test/Features/QueryFeatureTests.cs | 15 ++ .../Dictionary/AdaptiveCapacityDictionary.cs | 3 +- 4 files changed, 32 insertions(+), 152 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index b47b4746c1a3..847bdb500e28 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -132,16 +132,7 @@ public IQueryCollection Query if (equalIndex >= 0) { - var i = 0; - for (; i < querySegment.Length; ++i) - { - if (!char.IsWhiteSpace(querySegment[i])) - { - break; - } - } - - var name = SpanHelper.ReplacePlusWithSpace(querySegment[i..equalIndex]); + var name = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(0, equalIndex)); var value = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(equalIndex + 1)); accumulator.Append( @@ -210,11 +201,12 @@ public void Append(string key, string value) if (_expandingAccumulator is null) { - _expandingAccumulator = new AdaptiveCapacityDictionary>(5, StringComparer.OrdinalIgnoreCase); + _expandingAccumulator = new AdaptiveCapacityDictionary>(capacity: 5, StringComparer.OrdinalIgnoreCase); } - // Already 5 entries so use starting allocated as 10; then use List's expansion mechanism for more - var list = new List(10); + // Already 3 (2 existing + the new one) entries so use starting allocated as 6; then use List's expansion + // mechanism for more + var list = new List(capacity: 6); list.AddRange(values); list.Add(value); diff --git a/src/Http/Http/src/QueryCollectionInternal.cs b/src/Http/Http/src/QueryCollectionInternal.cs index db23b2edbc57..45b40fe5feb1 100644 --- a/src/Http/Http/src/QueryCollectionInternal.cs +++ b/src/Http/Http/src/QueryCollectionInternal.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections; using System.Collections.Generic; using Microsoft.AspNetCore.Internal; @@ -14,25 +13,7 @@ namespace Microsoft.AspNetCore.Http /// internal class QueryCollectionInternal : IQueryCollection { - /// - /// Gets an empty . - /// - public static readonly QueryCollectionInternal Empty = new QueryCollectionInternal(); - private static readonly string[] EmptyKeys = Array.Empty(); - private static readonly StringValues[] EmptyValues = Array.Empty(); - private static readonly Enumerator EmptyEnumerator = new Enumerator(); - // Pre-box - private static readonly IEnumerator> EmptyIEnumeratorType = EmptyEnumerator; - private static readonly IEnumerator EmptyIEnumerator = EmptyEnumerator; - - private AdaptiveCapacityDictionary? Store { get; } - - /// - /// Initializes a new instance of . - /// - public QueryCollectionInternal() - { - } + private AdaptiveCapacityDictionary Store { get; } /// /// Initializes a new instance of . @@ -43,90 +24,30 @@ internal QueryCollectionInternal(AdaptiveCapacityDictionary - /// Creates a shallow copy of the specified . - /// - /// The to clone. - public QueryCollectionInternal(QueryCollectionInternal store) - { - Store = store.Store; - } - - /// - /// Initializes a new instance of . - /// - /// The initial number of query items that this instance can contain. - public QueryCollectionInternal(int capacity) - { - Store = new AdaptiveCapacityDictionary(capacity, StringComparer.OrdinalIgnoreCase); - } - /// /// Gets the associated set of values from the collection. /// /// The key name. /// the associated value from the collection as a StringValues or StringValues.Empty if the key is not present. - public StringValues this[string key] - { - get - { - if (Store == null) - { - return StringValues.Empty; - } - - if (TryGetValue(key, out var value)) - { - return value; - } - return StringValues.Empty; - } - } + public StringValues this[string key] => TryGetValue(key, out var value) ? value : StringValues.Empty; /// /// Gets the number of elements contained in the ;. /// /// The number of elements contained in the . - public int Count - { - get - { - if (Store == null) - { - return 0; - } - return Store.Count; - } - } + public int Count => Store.Count; /// /// Gets the collection of query names in this instance. /// - public ICollection Keys - { - get - { - if (Store == null) - { - return EmptyKeys; - } - return Store.Keys; - } - } + public ICollection Keys => Store.Keys; /// /// Determines whether the contains a specific key. /// /// The key. /// true if the contains a specific key; otherwise, false. - public bool ContainsKey(string key) - { - if (Store == null) - { - return false; - } - return Store.ContainsKey(key); - } + public bool ContainsKey(string key) => Store.ContainsKey(key); /// /// Retrieves a value from the collection. @@ -134,57 +55,26 @@ public bool ContainsKey(string key) /// The key. /// The value. /// true if the contains the key; otherwise, false. - public bool TryGetValue(string key, out StringValues value) - { - if (Store == null) - { - value = default(StringValues); - return false; - } - return Store.TryGetValue(key, out value); - } + public bool TryGetValue(string key, out StringValues value) => Store.TryGetValue(key, out value); /// /// Returns an enumerator that iterates through a collection. /// /// An object that can be used to iterate through the collection. - public Enumerator GetEnumerator() - { - if (Store == null || Store.Count == 0) - { - // Non-boxed Enumerator - return EmptyEnumerator; - } - return new Enumerator(Store.GetEnumerator()); - } + public Enumerator GetEnumerator() => new Enumerator(Store.GetEnumerator()); /// /// Returns an enumerator that iterates through a collection. /// /// An object that can be used to iterate through the collection. IEnumerator> IEnumerable>.GetEnumerator() - { - if (Store == null || Store.Count == 0) - { - // Non-boxed Enumerator - return EmptyIEnumeratorType; - } - return Store.GetEnumerator(); - } + => Store.GetEnumerator(); /// /// Returns an enumerator that iterates through a collection. /// /// An object that can be used to iterate through the collection. - IEnumerator IEnumerable.GetEnumerator() - { - if (Store == null || Store.Count == 0) - { - // Non-boxed Enumerator - return EmptyIEnumerator; - } - return Store.GetEnumerator(); - } + IEnumerator IEnumerable.GetEnumerator() => Store.GetEnumerator(); /// /// Enumerates a . @@ -218,30 +108,14 @@ public bool MoveNext() /// /// Gets the element at the current position of the enumerator. /// - public KeyValuePair Current - { - get - { - if (_notEmpty) - { - return _dictionaryEnumerator.Current; - } - return default(KeyValuePair); - } - } + public KeyValuePair Current => _notEmpty ? _dictionaryEnumerator.Current : default; /// public void Dispose() { } - object IEnumerator.Current - { - get - { - return Current; - } - } + object IEnumerator.Current => Current; void IEnumerator.Reset() { diff --git a/src/Http/Http/test/Features/QueryFeatureTests.cs b/src/Http/Http/test/Features/QueryFeatureTests.cs index 61e8dfbca6aa..04a0a7c8c462 100644 --- a/src/Http/Http/test/Features/QueryFeatureTests.cs +++ b/src/Http/Http/test/Features/QueryFeatureTests.cs @@ -105,6 +105,21 @@ public void ParseQueryWithDuplicateKeysGroups() Assert.Equal("valueB", queryCollection["key2"].FirstOrDefault()); } + [Fact] + public void ParseQueryWithThreefoldKeysGroups() + { + var features = new FeatureCollection(); + features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "?key1=valueA&key2=valueB&key1=valueC&key1=valueD" }; + + var provider = new QueryFeature(features); + + var queryCollection = provider.Query; + + Assert.Equal(2, queryCollection.Count); + Assert.Equal(new[] { "valueA", "valueC", "valueD" }, queryCollection["key1"]); + Assert.Equal("valueB", queryCollection["key2"].FirstOrDefault()); + } + [Fact] public void ParseQueryWithEmptyValuesWorks() { diff --git a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs index d039c4ffbbbd..3d2c208cd120 100644 --- a/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs +++ b/src/Shared/Dictionary/AdaptiveCapacityDictionary.cs @@ -22,7 +22,7 @@ internal class AdaptiveCapacityDictionary : IDictionary[]? _arrayStorage; private int _count; internal Dictionary? _dictionaryStorage; - private IEqualityComparer _comparer; + private readonly IEqualityComparer _comparer; /// /// Creates an empty . @@ -621,7 +621,6 @@ private bool TryFindItem(TKey key, out TValue? value) [MethodImpl(MethodImplOptions.AggressiveInlining)] private bool ContainsKeyArray(TKey key) => TryFindItem(key, out _); - /// public struct Enumerator : IEnumerator> { From bbcda61981e578faecdfce1f167eb2990a9cc3b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Thu, 20 May 2021 14:09:18 +0200 Subject: [PATCH 14/16] Restructured KvpAccumulator.Append --- src/Http/Http/src/Features/QueryFeature.cs | 59 ++++++++++++---------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 847bdb500e28..244f7ed56da1 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -183,44 +183,47 @@ public void Append(string key, string value) _accumulator = new AdaptiveCapacityDictionary(StringComparer.OrdinalIgnoreCase); } - if (_accumulator.TryGetValue(key, out var values)) + if (!_accumulator.TryGetValue(key, out var values)) { - if (values.Count == 0) - { - // Marker entry for this key to indicate entry already in expanding list dictionary - _expandingAccumulator[key].Add(value); - } - else if (values.Count == 1) - { - _accumulator[key] = StringValues.Concat(values, value); - } - else - { - // Add zero count entry and move to data to expanding list dictionary - _accumulator[key] = default; + // First value for this key + _accumulator[key] = new StringValues(value); + } + else + { + AppendToExpandingAccumulator(key, value, values); + } - if (_expandingAccumulator is null) - { - _expandingAccumulator = new AdaptiveCapacityDictionary>(capacity: 5, StringComparer.OrdinalIgnoreCase); - } + ValueCount++; + } - // Already 3 (2 existing + the new one) entries so use starting allocated as 6; then use List's expansion - // mechanism for more - var list = new List(capacity: 6); + private void AppendToExpandingAccumulator(string key, string value, StringValues values) + { + // When there are some values for the same key, so switch to expanding accumulator, and + // add a zero count marker in the accumulator to indicate that switch. - list.AddRange(values); - list.Add(value); + if (values.Count != 0) + { + _accumulator[key] = default; - _expandingAccumulator[key] = list; + if (_expandingAccumulator is null) + { + _expandingAccumulator = new AdaptiveCapacityDictionary>(capacity: 5, StringComparer.OrdinalIgnoreCase); } + + // Already 3 (2 existing + the new one) entries so use starting allocated as 6; then use List's expansion + // mechanism for more + var list = new List(capacity: 6); + + list.AddRange(values); + list.Add(value); + + _expandingAccumulator[key] = list; } else { - // First value for this key - _accumulator[key] = new StringValues(value); + // The marker indicates we are in the expanding accumulator, so just append to the list. + _expandingAccumulator[key].Add(value); } - - ValueCount++; } /// From 719eff449164a403f31fdb9c435eaf59268e9234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Thu, 20 May 2021 14:19:46 +0200 Subject: [PATCH 15/16] Update src/Http/Http/src/Features/QueryFeature.cs --- src/Http/Http/src/Features/QueryFeature.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Http/Http/src/Features/QueryFeature.cs b/src/Http/Http/src/Features/QueryFeature.cs index 244f7ed56da1..bc7b1c7d2cf2 100644 --- a/src/Http/Http/src/Features/QueryFeature.cs +++ b/src/Http/Http/src/Features/QueryFeature.cs @@ -210,9 +210,8 @@ private void AppendToExpandingAccumulator(string key, string value, StringValues _expandingAccumulator = new AdaptiveCapacityDictionary>(capacity: 5, StringComparer.OrdinalIgnoreCase); } - // Already 3 (2 existing + the new one) entries so use starting allocated as 6; then use List's expansion - // mechanism for more - var list = new List(capacity: 6); + // Already 2 (1 existing + the new one) entries so use List's expansion mechanism for more + var list = new List(); list.AddRange(values); list.Add(value); From a050faee334c586eec64d8cc5ac326554ad77b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Fri, 21 May 2021 10:45:16 +0200 Subject: [PATCH 16/16] Revert import of $(SharedSourceRoot)UrlDecoder --- src/Http/Http/src/Microsoft.AspNetCore.Http.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index 8f9773316efd..669a90b01f12 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -13,7 +13,6 @@ -