Make CandidateState (publicly) immutable

src/Microsoft.AspNetCore.Routing/Matching/CandidateSet.cs

@@ -2,6 +2,8 @@
 // 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.Specialized;
 using System.Runtime.CompilerServices;
 using Microsoft.AspNetCore.Http;
 
@@ -14,6 +16,11 @@ namespace Microsoft.AspNetCore.Routing.Matching
     /// </summary>
     public sealed class CandidateSet
     {
+        private const int BitVectorSize = 32;
+
+        private BitVector32 _validity;
+        private BitArray _largeCapactityValidity;
+
         // We inline storage for 4 candidates here to avoid allocations in common
         // cases. There's no real reason why 4 is important, it just seemed like 
         // a plausible number.
@@ -26,8 +33,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
 
         /// <summary>
         /// <para>
-        /// Initializes a new instances of the candidate set structure with the provided list of endpoints 
+        /// Initializes a new instances of the candidate set structure with the provided data.
-        /// and associated scores.
         /// </para>
         /// <para>
         /// The constructor is provided to enable unit tests of implementations of <see cref="EndpointSelector"/>
@@ -35,8 +41,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
         /// </para>
         /// </summary>
         /// <param name="endpoints">The list of endpoints, sorted in descending priority order.</param>
+        /// <param name="values">The list of <see cref="RouteValueDictionary"/> instances.</param>
         /// <param name="scores">The list of endpoint scores. <see cref="CandidateState.Score"/>.</param>
-        public CandidateSet(Endpoint[] endpoints, int[] scores)
+        public CandidateSet(Endpoint[] endpoints, RouteValueDictionary[] values, int[] scores)
         {
             Count = endpoints.Length;
 
@@ -80,6 +87,17 @@ namespace Microsoft.AspNetCore.Routing.Matching
                     }
                     break;
             }
+
+            // Initialize validity to valid by default.
+            if (Count < BitVectorSize)
+            {
+                // Sets the bit for each candidate that exists (bits > Count will be 0).
+                _validity = new BitVector32(unchecked((int)~(0xFFFFFFFFu << Count)));
+            }
+            else
+            {
+                _largeCapactityValidity = new BitArray(Count, defaultValue: true);
+            }
         }
 
         internal CandidateSet(Candidate[] candidates)
@@ -126,25 +144,35 @@ namespace Microsoft.AspNetCore.Routing.Matching
                     }
                     break;
             }
+
+            // Initialize validity to valid by default.
+            if (Count < BitVectorSize)
+            {
+                // Sets the bit for each candidate that exists (bits > Count will be 0).
+                _validity = new BitVector32(unchecked((int)~(0xFFFFFFFFu << Count)));
+            }
+            else
+            {
+                _largeCapactityValidity = new BitArray(Count, defaultValue: true);
+            }
         }
 
         /// <summary>
         /// Gets the count of candidates in the set.
         /// </summary>
         public int Count { get; }
-
+        
         /// <summary>
         /// Gets the <see cref="CandidateState"/> associated with the candidate <see cref="Endpoint"/>
         /// at <paramref name="index"/>.
         /// </summary>
         /// <param name="index">The candidate index.</param>
         /// <returns>
-        /// A reference to the <see cref="CandidateState"/>. The result is returned by reference
+        /// A reference to the <see cref="CandidateState"/>. The result is returned by reference.
-        /// and intended to be mutated.
         /// </returns>
         public ref CandidateState this[int index]
         {
-            // Note that this is a ref-return because of both mutability and performance.
+            // Note that this is a ref-return because of performance.
             // We don't want to copy these fat structs if it can be avoided.
 
             // PERF: Force inlining
@@ -177,6 +205,56 @@ namespace Microsoft.AspNetCore.Routing.Matching
             }
         }
 
+        /// <summary>
+        /// Gets or sets a value which indicates where the <see cref="Http.Endpoint"/> is considered
+        /// a valid candiate for the current request. Set this value to <c>false</c> to exclude an
+        /// <see cref="Http.Endpoint"/> from consideration.
+        /// </summary>
+        public bool IsValidCandidate(int index)
+        {
+            // Friendliness for inlining
+            if ((uint)index >= Count)
+            {
+                ThrowIndexArgumentOutOfRangeException();
+            }
+
+            if (Count < BitVectorSize)
+            {
+                // Get the n-th bit
+                return _validity[0x00000001 << index];
+            }
+            else
+            {
+                return _largeCapactityValidity[index];
+            }
+        }
+
+        /// <summary>
+        /// Sets the validitity of the candidate at the provided index.
+        /// </summary>
+        /// <param name="index">The candidate index.</param>
+        /// <param name="value">
+        /// The value to set. If <c>true</c> the candidate is considered valid for the current request.
+        /// </param>
+        public void SetValidity(int index, bool value)
+        {
+            // Friendliness for inlining
+            if ((uint)index >= Count)
+            {
+                ThrowIndexArgumentOutOfRangeException();
+            }
+
+            if (Count < BitVectorSize)
+            {
+                // Set the n-th bit
+                _validity[0x00000001 << index] = value;
+            }
+            else
+            {
+                _largeCapactityValidity[index] = value;
+            }
+        }
+
         private static void ThrowIndexArgumentOutOfRangeException()
         {
             throw new ArgumentOutOfRangeException("index");

src/Microsoft.AspNetCore.Routing/Matching/CandidateState.cs

@@ -14,11 +14,16 @@ namespace Microsoft.AspNetCore.Routing.Matching
         {
             Endpoint = endpoint;
             Score = score;
-
-            IsValidCandidate = true;
             Values = null;
         }
 
+        internal CandidateState(Endpoint endpoint, RouteValueDictionary values, int score)
+        {
+            Endpoint = endpoint;
+            Values = values;
+            Score = score;
+        }
+
         /// <summary>
         /// Gets the <see cref="Http.Endpoint"/>.
         /// </summary>
@@ -41,17 +46,10 @@ namespace Microsoft.AspNetCore.Routing.Matching
         /// </remarks>
         public int Score { get; }
 
-        /// <summary>
-        /// Gets or sets a value which indicates where the <see cref="Http.Endpoint"/> is considered
-        /// a valid candiate for the current request. Set this value to <c>false</c> to exclude an
-        /// <see cref="Http.Endpoint"/> from consideration.
-        /// </summary>
-        public bool IsValidCandidate { get; set; }
-
         /// <summary>
         /// Gets or sets the <see cref="RouteValueDictionary"/> associated with the
         /// <see cref="Http.Endpoint"/> and the current request.
         /// </summary>
-        public RouteValueDictionary Values { get; set; }
+        public RouteValueDictionary Values { get; internal set; }
     }
 }

src/Microsoft.AspNetCore.Routing/Matching/DefaultEndpointSelector.cs

@@ -53,10 +53,10 @@ namespace Microsoft.AspNetCore.Routing.Matching
             for (var i = 0; i < candidateSet.Count; i++)
             {
                 ref var state = ref candidateSet[i];
-
+                var isValid = candidateSet.IsValidCandidate(i);
-                var isValid = state.IsValidCandidate;
                 if (isValid && foundScore == null)
                 {
+
                     // This is the first match we've seen - speculatively assign it.
                     endpoint = state.Endpoint;
                     values = state.Values;
@@ -98,10 +98,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var matches = new List<Endpoint>();
             for (var i = 0; i < candidates.Count; i++)
             {
-                ref var state = ref candidates[i];
+                if (candidates.IsValidCandidate(i))
-                if (state.IsValidCandidate)
                 {
-                    matches.Add(state.Endpoint);
+                    matches.Add(candidates[i].Endpoint);
                 }
             }
 

src/Microsoft.AspNetCore.Routing/Matching/DfaMatcher.cs

@@ -5,7 +5,6 @@ using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Routing.Patterns;
 
 namespace Microsoft.AspNetCore.Routing.Matching
@@ -118,18 +117,23 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 // Now that we have the route values, we need to process complex segments.
                 // Complex segments go through an old API that requires a fully-materialized
                 // route value dictionary.
-                var isMatch = true;
                 if ((flags & Candidate.CandidateFlags.HasComplexSegments) != 0)
                 {
-                    isMatch &= ProcessComplexSegments(candidate.ComplexSegments, path, segments, values);
+                    if (!ProcessComplexSegments(candidate.ComplexSegments, path, segments, values))
+                    {
+                        candidateSet.SetValidity(i, false);
+                        continue;
+                    }
                 }
 
                 if ((flags & Candidate.CandidateFlags.HasConstraints) != 0)
                 {
-                    isMatch &= ProcessConstraints(candidate.Constraints, httpContext, values);
+                    if (!ProcessConstraints(candidate.Constraints, httpContext, values))
+                    {
+                        candidateSet.SetValidity(i, false);
+                        continue;
+                    }
                 }
-
-                state.IsValidCandidate = isMatch;
             }
 
             return _selector.SelectAsync(httpContext, context, candidateSet);

src/Microsoft.AspNetCore.Routing/Matching/IEndpointSelectorPolicy.cs

@@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
         /// <para>
         /// Implementations of <see cref="IEndpointSelectorPolicy"/> should implement this method
         /// and filter the set of candidates in the <paramref name="candidates"/> by setting
-        /// <see cref="CandidateState.IsValidCandidate"/> to <c>false</c> where desired.
+        /// <see cref="CandidateSet.SetValidity(int, bool)"/> to <c>false</c> where desired.
         /// </para>
         /// <para>
         /// To signal an error condition, set <see cref="EndpointSelectorContext.Endpoint"/> to an

test/Microsoft.AspNetCore.Routing.Tests/Matching/CandidateSetTest.cs

@@ -22,6 +22,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
         [InlineData(4)]
         [InlineData(5)] // this is the break-point where we start to use a list.
         [InlineData(6)]
+        [InlineData(31)]
+        [InlineData(32)] // this is the break point where we use a BitArray
+        [InlineData(33)]
         public void Create_CreatesCandidateSet(int count)
         {
             // Arrange
@@ -41,10 +44,13 @@ namespace Microsoft.AspNetCore.Routing.Matching
             for (var i = 0; i < candidateSet.Count; i++)
             {
                 ref var state = ref candidateSet[i];
-                Assert.True(state.IsValidCandidate);
+                Assert.True(candidateSet.IsValidCandidate(i));
                 Assert.Same(endpoints[i], state.Endpoint);
                 Assert.Equal(candidates[i].Score, state.Score);
                 Assert.Null(state.Values);
+
+                candidateSet.SetValidity(i, false);
+                Assert.False(candidateSet.IsValidCandidate(i));
             }
         }
 
@@ -58,6 +64,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
         [InlineData(4)]
         [InlineData(5)] // this is the break-point where we start to use a list.
         [InlineData(6)]
+        [InlineData(31)]
+        [InlineData(32)] // this is the break point where we use a BitArray
+        [InlineData(33)]
         public void Create_CreatesCandidateSet_TestConstructor(int count)
         {
             // Arrange
@@ -68,16 +77,19 @@ namespace Microsoft.AspNetCore.Routing.Matching
             }
 
             // Act
-            var candidateSet = new CandidateSet(endpoints, Enumerable.Range(0, count).ToArray());
+            var candidateSet = new CandidateSet(endpoints, new RouteValueDictionary[count], Enumerable.Range(0, count).ToArray());
 
             // Assert
             for (var i = 0; i < candidateSet.Count; i++)
             {
                 ref var state = ref candidateSet[i];
-                Assert.True(state.IsValidCandidate);
+                Assert.True(candidateSet.IsValidCandidate(i));
                 Assert.Same(endpoints[i], state.Endpoint);
                 Assert.Equal(i, state.Score);
                 Assert.Null(state.Values);
+
+                candidateSet.SetValidity(i, false);
+                Assert.False(candidateSet.IsValidCandidate(i));
             }
         }
 

test/Microsoft.AspNetCore.Routing.Tests/Matching/DefaultEndpointSelectorTest.cs

@@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
             candidateSet[0].Values = new RouteValueDictionary();
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
             candidateSet[0].Values = new RouteValueDictionary();
-            candidateSet[0].IsValidCandidate = true;
+            candidateSet.SetValidity(0, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -81,8 +81,8 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var scores = new int[] { 0, 0 };
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = true;
+            candidateSet.SetValidity(1, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -102,9 +102,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var scores = new int[] { 0, 0, 1 };
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = true;
+            candidateSet.SetValidity(1, true);
-            candidateSet[2].IsValidCandidate = true;
+            candidateSet.SetValidity(2, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -131,11 +131,11 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var scores = new int[] { 0, 1, 2, 3, 4 };
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = false;
+            candidateSet.SetValidity(1, false);
-            candidateSet[2].IsValidCandidate = false;
+            candidateSet.SetValidity(2, false);
-            candidateSet[3].IsValidCandidate = false;
+            candidateSet.SetValidity(3, false);
-            candidateSet[4].IsValidCandidate = true;
+            candidateSet.SetValidity(4, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -155,9 +155,9 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var scores = new int[] { 0, 1, 1 };
             var candidateSet = CreateCandidateSet(endpoints, scores);
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = true;
+            candidateSet.SetValidity(1, true);
-            candidateSet[2].IsValidCandidate = true;
+            candidateSet.SetValidity(2, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector();
@@ -188,13 +188,13 @@ test: /test3", ex.Message);
                 .Setup(p => p.ApplyAsync(It.IsAny<HttpContext>(), It.IsAny<EndpointSelectorContext>(), It.IsAny<CandidateSet>()))
                 .Returns<HttpContext, EndpointSelectorContext, CandidateSet>((c, f, cs) =>
                 {
-                    cs[1].IsValidCandidate = false;
+                    cs.SetValidity(1, false);
                     return Task.CompletedTask;
                 });
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = true;
+            candidateSet.SetValidity(1, true);
-            candidateSet[2].IsValidCandidate = true;
+            candidateSet.SetValidity(2, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector(policy.Object);
@@ -234,9 +234,9 @@ test: /test3", ex.Message);
                 .Setup(p => p.ApplyAsync(It.IsAny<HttpContext>(), It.IsAny<EndpointSelectorContext>(), It.IsAny<CandidateSet>()))
                 .Throws(new InvalidOperationException());
 
-            candidateSet[0].IsValidCandidate = false;
+            candidateSet.SetValidity(0, false);
-            candidateSet[1].IsValidCandidate = true;
+            candidateSet.SetValidity(1, true);
-            candidateSet[2].IsValidCandidate = true;
+            candidateSet.SetValidity(2, true);
 
             var (httpContext, context) = CreateContext();
             var selector = CreateSelector(policy1.Object, policy2.Object);
@@ -270,7 +270,7 @@ test: /test3", ex.Message);
 
         private static CandidateSet CreateCandidateSet(RouteEndpoint[] endpoints, int[] scores)
         {
-            return new CandidateSet(endpoints, scores);
+            return new CandidateSet(endpoints, new RouteValueDictionary[endpoints.Length], scores);
         }
 
         private static DefaultEndpointSelector CreateSelector(params MatcherPolicy[] policies)

test/Microsoft.AspNetCore.Routing.Tests/Matching/DfaMatcherTest.cs

@@ -126,12 +126,12 @@ namespace Microsoft.AspNetCore.Routing.Matching
                     Assert.Equal(2, cs.Count);
 
                     Assert.Same(endpoint1, cs[0].Endpoint);
-                    Assert.True(cs[0].IsValidCandidate);
+                    Assert.True(cs.IsValidCandidate(0));
                     Assert.Equal(0, cs[0].Score);
                     Assert.Empty(cs[0].Values);
 
                     Assert.Same(endpoint2, cs[1].Endpoint);
-                    Assert.True(cs[1].IsValidCandidate);
+                    Assert.True(cs.IsValidCandidate(1));
                     Assert.Equal(1, cs[1].Score);
                     Assert.Empty(cs[1].Values);
 

test/Microsoft.AspNetCore.Routing.Tests/Matching/RouteMatcherBuilder.cs

@@ -76,6 +76,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
         {
             private readonly EndpointSelector _selector;
             private readonly RouteEndpoint[] _candidates;
+            private readonly RouteValueDictionary[] _values;
             private readonly int[] _scores;
 
             public SelectorRouter(EndpointSelector selector, RouteEndpoint[] candidates)
@@ -83,6 +84,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 _selector = selector;
                 _candidates = candidates;
 
+                _values = new RouteValueDictionary[_candidates.Length];
                 _scores = new int[_candidates.Length];
             }
 
@@ -99,7 +101,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 // across requests.
                 context.Endpoint = null;
 
-                await _selector.SelectAsync(routeContext.HttpContext, context, new CandidateSet(_candidates, _scores));
+                await _selector.SelectAsync(routeContext.HttpContext, context, new CandidateSet(_candidates, _values, _scores));
                 if (context.Endpoint != null)
                 {
                     routeContext.Handler = (_) => Task.CompletedTask;

test/Microsoft.AspNetCore.Routing.Tests/Matching/TreeRouterMatcherBuilder.cs

@@ -78,6 +78,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
         {
             private readonly EndpointSelector _selector;
             private readonly RouteEndpoint[] _candidates;
+            private readonly RouteValueDictionary[] _values;
             private readonly int[] _scores;
 
             public SelectorRouter(EndpointSelector selector, RouteEndpoint[] candidates)
@@ -85,6 +86,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 _selector = selector;
                 _candidates = candidates;
 
+                _values = new RouteValueDictionary[_candidates.Length];
                 _scores = new int[_candidates.Length];
             }
 
@@ -100,7 +102,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 // This is needed due to a quirk of our tests - they reuse the endpoint feature.
                 context.Endpoint = null;
                 
-                await _selector.SelectAsync(routeContext.HttpContext, context, new CandidateSet(_candidates, _scores));
+                await _selector.SelectAsync(routeContext.HttpContext, context, new CandidateSet(_candidates, _values, _scores));
                 if (context.Endpoint != null)
                 {
                     routeContext.Handler = (_) => Task.CompletedTask;