From 0788edbd4bc6878b7ca929aba798517f1cfdc6c9 Mon Sep 17 00:00:00 2001 From: mnltejaswini Date: Thu, 21 Apr 2016 15:07:44 -0700 Subject: [PATCH] [Perf] Cache the metadata for known type "object" Fixes #4377 --- .../Metadata/DefaultModelMetadataProvider.cs | 37 ++++++++++++++++--- ...taDictionaryControllerPropertyActivator.cs | 1 - .../DefaultModelMetadataProviderTest.cs | 14 +++++++ .../Internal/ModelMetadataProviderTest.cs | 11 ++++-- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs index f8bc8524d5..2c564af29a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { private readonly TypeCache _typeCache = new TypeCache(); private readonly Func _cacheEntryFactory; + private readonly ModelMetadataCacheEntry _metadataCacheEntryForObjectType; /// /// Creates a new . @@ -26,6 +27,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata DetailsProvider = detailsProvider; _cacheEntryFactory = CreateCacheEntry; + + _metadataCacheEntryForObjectType = GetMetadataCacheEntryForObjectType(); } /// @@ -41,14 +44,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata throw new ArgumentNullException(nameof(modelType)); } - var key = ModelMetadataIdentity.ForType(modelType); - - var cacheEntry = _typeCache.GetOrAdd(key, _cacheEntryFactory); + var cacheEntry = GetCacheEntry(modelType); // We're relying on a safe race-condition for Properties - take care only // to set the value onces the properties are fully-initialized. if (cacheEntry.Details.Properties == null) { + var key = ModelMetadataIdentity.ForType(modelType); var propertyDetails = CreatePropertyDetails(key); var properties = new ModelMetadata[propertyDetails.Length]; @@ -71,12 +73,30 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata throw new ArgumentNullException(nameof(modelType)); } - var key = ModelMetadataIdentity.ForType(modelType); + var cacheEntry = GetCacheEntry(modelType); - var cacheEntry = _typeCache.GetOrAdd(key, _cacheEntryFactory); return cacheEntry.Metadata; } + private ModelMetadataCacheEntry GetCacheEntry(Type modelType) + { + ModelMetadataCacheEntry cacheEntry; + + // Perf: We cached model metadata cache entry for "object" type to save ConcurrentDictionary lookups. + if (modelType == typeof(object)) + { + cacheEntry = _metadataCacheEntryForObjectType; + } + else + { + var key = ModelMetadataIdentity.ForType(modelType); + + cacheEntry = _typeCache.GetOrAdd(key, _cacheEntryFactory); + } + + return cacheEntry; + } + private ModelMetadataCacheEntry CreateCacheEntry(ModelMetadataIdentity key) { var details = CreateTypeDetails(key); @@ -84,6 +104,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata return new ModelMetadataCacheEntry(metadata, details); } + private ModelMetadataCacheEntry GetMetadataCacheEntryForObjectType() + { + var key = ModelMetadataIdentity.ForType(typeof(object)); + var entry = CreateCacheEntry(key); + return entry; + } + /// /// Creates a new from a . /// diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataDictionaryControllerPropertyActivator.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataDictionaryControllerPropertyActivator.cs index 6dfc2ffc93..aae07d28b2 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataDictionaryControllerPropertyActivator.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataDictionaryControllerPropertyActivator.cs @@ -49,7 +49,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures private ViewDataDictionary GetViewDataDictionary(ControllerContext context) { - var serviceProvider = context.HttpContext.RequestServices; return new ViewDataDictionary( _modelMetadataProvider, context.ModelState); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs index b05b213782..66fda668b3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs @@ -44,6 +44,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata Assert.Same(metadata1.ValidationMetadata, metadata2.ValidationMetadata); } + [Fact] + public void GetMetadataForObjectType_Cached() + { + // Arrange + var provider = CreateProvider(); + + // Act + var metadata1 = provider.GetMetadataForType(typeof(object)); + var metadata2 = provider.GetMetadataForType(typeof(object)); + + // Assert + Assert.Same(metadata1, metadata2); + } + [Fact] public void GetMetadataForProperties_IncludesAllProperties() { diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs index 8b413c582a..f50bf402c7 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs @@ -1074,9 +1074,14 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal protected override DefaultMetadataDetails CreateTypeDetails(ModelMetadataIdentity key) { var entry = base.CreateTypeDetails(key); - return new DefaultMetadataDetails( - key, - new ModelAttributes(_attributes.Concat(entry.ModelAttributes.TypeAttributes).ToArray())); + if (_attributes?.Length > 0) + { + return new DefaultMetadataDetails( + key, + new ModelAttributes(_attributes.Concat(entry.ModelAttributes.TypeAttributes).ToArray())); + } + + return entry; } protected override DefaultMetadataDetails[] CreatePropertyDetails(ModelMetadataIdentity key)