Avoid allocating strings in DefaultViewLocationCache

Fixes #3135

src/Microsoft.AspNet.Mvc.Razor/DefaultViewLocationCache.cs

@@ -3,8 +3,7 @@
 
 using System;
 using System.Collections.Concurrent;
-using System.Linq;
+using System.Collections.Generic;
-using System.Text;
 using Microsoft.Framework.Internal;
 
 namespace Microsoft.AspNet.Mvc.Razor
@@ -14,23 +13,22 @@ namespace Microsoft.AspNet.Mvc.Razor
     /// </summary>
     public class DefaultViewLocationCache : IViewLocationCache
     {
-        private const char CacheKeySeparator = ':';
-
         // A mapping of keys generated from ViewLocationExpanderContext to view locations.
-        private readonly ConcurrentDictionary<string, ViewLocationCacheResult> _cache;
+        private readonly ConcurrentDictionary<ViewLocationCacheKey, ViewLocationCacheResult> _cache;
 
         /// <summary>
         /// Initializes a new instance of <see cref="DefaultViewLocationCache"/>.
         /// </summary>
         public DefaultViewLocationCache()
         {
-            _cache = new ConcurrentDictionary<string, ViewLocationCacheResult>(StringComparer.Ordinal);
+            _cache = new ConcurrentDictionary<ViewLocationCacheKey, ViewLocationCacheResult>(
+                ViewLocationCacheKeyComparer.Instance);
         }
 
         /// <inheritdoc />
         public ViewLocationCacheResult Get([NotNull] ViewLocationExpanderContext context)
         {
-            var cacheKey = GenerateKey(context);
+            var cacheKey = GenerateKey(context, copyViewExpanderValues: false);
             ViewLocationCacheResult result;
             if (_cache.TryGetValue(cacheKey, out result))
             {
@@ -45,46 +43,124 @@ namespace Microsoft.AspNet.Mvc.Razor
             [NotNull] ViewLocationExpanderContext context,
             [NotNull] ViewLocationCacheResult value)
         {
-            var cacheKey = GenerateKey(context);
+            var cacheKey = GenerateKey(context, copyViewExpanderValues: true);
             _cache.TryAdd(cacheKey, value);
         }
 
-        internal static string GenerateKey(ViewLocationExpanderContext context)
+        // Internal for unit testing
+        internal static ViewLocationCacheKey GenerateKey(
+            ViewLocationExpanderContext context,
+            bool copyViewExpanderValues)
         {
-            var keyBuilder = new StringBuilder();
-            var routeValues = context.ActionContext.RouteData.Values;
             var controller = RazorViewEngine.GetNormalizedRouteValue(
                 context.ActionContext,
                 RazorViewEngine.ControllerKey);
 
-            // format is "{viewName}:{isPartial}:{controllerName}:{areaName}:"
+            var area = RazorViewEngine.GetNormalizedRouteValue(
-            keyBuilder.Append(context.ViewName)
+                context.ActionContext,
-                      .Append(CacheKeySeparator)
+                RazorViewEngine.AreaKey);
-                      .Append(context.IsPartial ? 1 : 0)
-                      .Append(CacheKeySeparator)
-                      .Append(controller);
 
-            var area = RazorViewEngine.GetNormalizedRouteValue(context.ActionContext, RazorViewEngine.AreaKey);
+
-            if (!string.IsNullOrEmpty(area))
+            var values = context.Values;
+            if (values != null && copyViewExpanderValues)
             {
-                keyBuilder.Append(CacheKeySeparator)
+                // When performing a Get, avoid creating a copy of the values dictionary
-                          .Append(area);
+                values = new Dictionary<string, string>(values, StringComparer.Ordinal);
             }
 
-            if (context.Values != null)
+            return new ViewLocationCacheKey(
+                context.ViewName,
+                controller,
+                area,
+                context.IsPartial,
+                values);
+        }
+
+        // Internal for unit testing
+        internal class ViewLocationCacheKeyComparer : IEqualityComparer<ViewLocationCacheKey>
+        {
+            public static readonly ViewLocationCacheKeyComparer Instance = new ViewLocationCacheKeyComparer();
+
+            public bool Equals(ViewLocationCacheKey x, ViewLocationCacheKey y)
             {
-                var valuesDictionary = context.Values;
+                if (x.IsPartial != y.IsPartial ||
-                foreach (var item in valuesDictionary.OrderBy(k => k.Key, StringComparer.Ordinal))
+                    !string.Equals(x.ViewName, y.ViewName, StringComparison.Ordinal) ||
+                    !string.Equals(x.ControllerName, y.ControllerName, StringComparison.Ordinal) ||
+                    !string.Equals(x.AreaName, y.AreaName, StringComparison.Ordinal))
                 {
-                    keyBuilder.Append(CacheKeySeparator)
+                    return false;
-                              .Append(item.Key)
-                              .Append(CacheKeySeparator)
-                              .Append(item.Value);
                 }
+
+                if (ReferenceEquals(x.Values, y.Values))
+                {
+                    return true;
+                }
+
+                if (x.Values == null || y.Values == null || (x.Values.Count != y.Values.Count))
+                {
+                    return false;
+                }
+
+                foreach (var item in x.Values)
+                {
+                    string yValue;
+                    if (!y.Values.TryGetValue(item.Key, out yValue) ||
+                        !string.Equals(item.Value, yValue, StringComparison.Ordinal))
+                    {
+                        return false;
+                    }
+                }
+
+                return true;
             }
 
-            var cacheKey = keyBuilder.ToString();
+            public int GetHashCode(ViewLocationCacheKey key)
-            return cacheKey;
+            {
+                var hashCodeCombiner = HashCodeCombiner.Start();
+                hashCodeCombiner.Add(key.IsPartial ? 1 : 0);
+                hashCodeCombiner.Add(key.ViewName, StringComparer.Ordinal);
+                hashCodeCombiner.Add(key.ControllerName, StringComparer.Ordinal);
+                hashCodeCombiner.Add(key.AreaName, StringComparer.Ordinal);
+
+                if (key.Values != null)
+                {
+                    foreach (var item in key.Values)
+                    {
+                        hashCodeCombiner.Add(item.Key, StringComparer.Ordinal);
+                        hashCodeCombiner.Add(item.Value, StringComparer.Ordinal);
+                    }
+                }
+
+                return hashCodeCombiner;
+            }
+        }
+
+        // Internal for unit testing
+        internal struct ViewLocationCacheKey
+        {
+            public ViewLocationCacheKey(
+                string viewName,
+                string controllerName,
+                string areaName,
+                bool isPartial,
+                IDictionary<string, string> values)
+            {
+                ViewName = viewName;
+                ControllerName = controllerName;
+                AreaName = areaName;
+                IsPartial = isPartial;
+                Values = values;
+            }
+
+            public string ViewName { get; }
+
+            public string ControllerName { get; }
+
+            public string AreaName { get; }
+
+            public bool IsPartial { get; }
+
+            public IDictionary<string, string> Values { get; }
         }
     }
 }

test/Microsoft.AspNet.Mvc.Razor.Test/DefaultViewLocationCacheTest.cs

@@ -43,9 +43,12 @@ namespace Microsoft.AspNet.Mvc.Razor
             }
         }
 
+        private static DefaultViewLocationCache.ViewLocationCacheKeyComparer CacheKeyComparer =>
+            DefaultViewLocationCache.ViewLocationCacheKeyComparer.Instance;
+
         [Theory]
         [MemberData(nameof(CacheEntryData))]
-        public void Get_GeneratesCacheKeyIfItemDoesNotExist(ViewLocationExpanderContext context)
+        public void Get_ReturnsNoneResultIfItemDoesNotExist(ViewLocationExpanderContext context)
         {
             // Arrange
             var cache = new DefaultViewLocationCache();
@@ -85,129 +88,390 @@ namespace Microsoft.AspNet.Mvc.Razor
             Assert.Equal(value, result);
         }
 
-        public static IEnumerable<object[]> CacheKeyData
+        [Theory]
+        [InlineData("View1", "View2")]
+        [InlineData("View1", "view1")]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfViewNamesAreDifferent(
+            string viewName1,
+            string viewName2)
         {
-            get
+            // Arrange
-            {
+            var actionContext = GetActionContext();
-                yield return new object[]
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
-                {
+                actionContext,
-                    new ViewLocationExpanderContext(GetActionContext(), "test", isPartial: false),
+                viewName1,
-                    "test:0:mycontroller"
+                isPartial: true);
-                };
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               actionContext,
+               viewName2,
+               isPartial: true);
 
-                yield return new object[]
+            // Act
-                {
+            var key1 = DefaultViewLocationCache.GenerateKey(
-                    new ViewLocationExpanderContext(GetActionContext(), "test", isPartial: true),
+                viewLocationExpanderContext1,
-                    "test:1:mycontroller"
+                copyViewExpanderValues: false);
-                };
 
-                var areaActionContext = GetActionContext("controller2", "myarea");
+            var key2 = DefaultViewLocationCache.GenerateKey(
-                yield return new object[]
+                viewLocationExpanderContext2,
-                {
+                copyViewExpanderValues: false);
-                    new ViewLocationExpanderContext(areaActionContext, "test2", isPartial: false),
-                    "test2:0:controller2:myarea"
-                };
-                yield return new object[]
-                {
-                    new ViewLocationExpanderContext(areaActionContext, "test2", isPartial: true),
-                    "test2:1:controller2:myarea"
-                };
 
-                var actionContext = GetActionContext("controller3", "area3");
+            var result = CacheKeyComparer.Equals(key1, key2);
-                var values = new Dictionary<string, string>(StringComparer.Ordinal)
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
-                {
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
-                    { "culture", "fr" },
-                    { "theme", "sleek" }
-                };
-                var expanderContext = new ViewLocationExpanderContext(actionContext, "test3", isPartial: false)
-                {
-                    Values = values
-                };
 
-                yield return new object[]
+            // Assert
-                {
+            Assert.False(result);
-                    expanderContext,
+            Assert.NotEqual(hash1, hash2);
-                    "test3:0:controller3:area3:culture:fr:theme:sleek"
-                };
-
-                expanderContext = new ViewLocationExpanderContext(actionContext, "test3", isPartial: true)
-                {
-                    Values = values
-                };
-                yield return new object[]
-                {
-                    expanderContext,
-                    "test3:1:controller3:area3:culture:fr:theme:sleek"
-                };
-
-                yield return new object[]
-                {
-                    new ViewLocationExpanderContext(
-                        GetActionContextWithActionDescriptor(
-                            new Dictionary<string, object>()
-                            {
-                                {"controller", "MyController" },
-                            },
-                            new Dictionary<string, string>()
-                            {
-                                {"controller", "mycontroller" },
-                            },
-                            isAttributeRouted: true),
-                        "test",
-                        isPartial: false),
-                    "test:0:mycontroller"
-                };
-
-                yield return new object[]
-                {
-                    new ViewLocationExpanderContext(
-                        GetActionContextWithActionDescriptor(
-                            new Dictionary<string, object>()
-                            {
-                                {"controller", "MyController" },
-                            },
-                            new Dictionary<string, string>()
-                            {
-                                {"controller", "mycontroller" },
-                            },
-                            isAttributeRouted: true),
-                        "test",
-                        isPartial: false),
-                    "test:0:mycontroller"
-                };
-
-                yield return new object[]
-                {
-                    new ViewLocationExpanderContext(
-                        GetActionContextWithActionDescriptor(
-                            new Dictionary<string, object>()
-                            {
-                                {"controller", "mycontroller" },
-                            },
-                            new Dictionary<string, string>()
-                            {
-                            },
-                            isAttributeRouted: true),
-                        "test",
-                        isPartial: false),
-                    "test:0:mycontroller"
-                };
-            }
         }
 
         [Theory]
-        [MemberData(nameof(CacheKeyData))]
+        [InlineData(false, true)]
-        public void CacheKeyIsComputedBasedOnValuesInExpander(ViewLocationExpanderContext context, string expected)
+        [InlineData(true, false)]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsPartialAreDifferent(
+            bool isPartial1,
+            bool isPartial2)
         {
+            // Arrange
+            var actionContext = GetActionContext();
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                actionContext,
+                "View1",
+                isPartial1);
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               actionContext,
+               "View1",
+               isPartial2);
+
             // Act
-            var result = DefaultViewLocationCache.GenerateKey(context);
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
 
             // Assert
-            Assert.Equal(expected, result);
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
         }
 
-        public static ActionContext GetActionContext(string controller = "mycontroller",
+        [Theory]
-                                                     string area = null)
+        [InlineData("Controller1", "Controller2")]
+        [InlineData("controller1", "Controller1")]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsControllerNamesAreDifferent(
+            string controller1,
+            string controller2)
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext(controller1),
+                "View1",
+                isPartial: false);
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext(controller2),
+               "View1",
+               isPartial: false);
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        [Theory]
+        [InlineData("area1", null)]
+        [InlineData("Area1", "Area2")]
+        [InlineData("area1", "aRea1")]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsAreaNamesAreDifferent(
+            string area1,
+            string area2)
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", area1),
+                "View1",
+                isPartial: false);
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", area2),
+               "View1",
+               isPartial: false);
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        [Fact]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsTrueIfControllerAreaAndViewNamesAreIdentical()
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.True(result);
+            Assert.Equal(hash1, hash2);
+        }
+
+        [Fact]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfViewLocationExpanderIsNullForEitherKey()
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            viewLocationExpanderContext1.Values = new Dictionary<string, string>
+            {
+                { "somekey", "somevalue" }
+            };
+
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        [Fact]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfExpanderValueCountIsDifferent()
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            viewLocationExpanderContext1.Values = new Dictionary<string, string>
+            {
+                { "somekey", "somevalue" }
+            };
+
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+            viewLocationExpanderContext2.Values = new Dictionary<string, string>
+            {
+                { "somekey", "somevalue" },
+                { "somekey2", "somevalue2" },
+            };
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        [Theory]
+        [InlineData("key1", "key2")]
+        [InlineData("Key1", "key1")]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfKeysAreDifferent(
+            string keyName1,
+            string keyName2)
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            viewLocationExpanderContext1.Values = new Dictionary<string, string>
+            {
+                { keyName1, "somevalue" }
+            };
+
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+            viewLocationExpanderContext2.Values = new Dictionary<string, string>
+            {
+                { keyName2, "somevalue" },
+            };
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        [Theory]
+        [InlineData("value1", null)]
+        [InlineData("value1", "value2")]
+        [InlineData("value1", "Value1")]
+        public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfValuesAreDifferent(
+            string value1,
+            string value2)
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            viewLocationExpanderContext1.Values = new Dictionary<string, string>
+            {
+                { "somekey", value1 }
+            };
+
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+            viewLocationExpanderContext2.Values = new Dictionary<string, string>
+            {
+                { "somekey", value2 },
+            };
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotEqual(hash1, hash2);
+        }
+
+        public void ViewLocationCacheKeyComparer_EqualsReturnsTrueIfValuesAreSame()
+        {
+            // Arrange
+            var viewLocationExpanderContext1 = new ViewLocationExpanderContext(
+                GetActionContext("Controller1", "Area1"),
+                "View1",
+                isPartial: false);
+            viewLocationExpanderContext1.Values = new Dictionary<string, string>
+            {
+                { "somekey1", "value1" },
+                { "somekey2", "value2" },
+            };
+
+            var viewLocationExpanderContext2 = new ViewLocationExpanderContext(
+               GetActionContext("Controller1", "Area1"),
+               "View1",
+               isPartial: false);
+            viewLocationExpanderContext2.Values = new Dictionary<string, string>
+            {
+                { "somekey2", "value2" },
+                { "somekey1", "value1" },
+            };
+
+            // Act
+            var key1 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext1,
+                copyViewExpanderValues: false);
+
+            var key2 = DefaultViewLocationCache.GenerateKey(
+                viewLocationExpanderContext2,
+                copyViewExpanderValues: false);
+
+            var result = CacheKeyComparer.Equals(key1, key2);
+            var hash1 = CacheKeyComparer.GetHashCode(key1);
+            var hash2 = CacheKeyComparer.GetHashCode(key2);
+
+            // Assert
+            Assert.True(result);
+            Assert.Equal(hash1, hash2);
+        }
+
+        public static ActionContext GetActionContext(
+            string controller = "mycontroller",
+            string area = null)
         {
             var routeData = new RouteData();
             routeData.Values["controller"] = controller;