Skip to content

Remove unnecessary GetTypeInfo from Microsoft.Extensions. #44891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stephentoub merged 1 commit into from
Nov 18, 2020
Merged

Remove unnecessary GetTypeInfo from Microsoft.Extensions. #44891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/libraries/Common/src/Extensions/ActivatorUtilities/ActivatorUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static object CreateInstance(

ConstructorMatcher bestMatcher = default;

if (!instanceType.GetTypeInfo().IsAbstract)
if (!instanceType.IsAbstract)
{
foreach (ConstructorInfo? constructor in instanceType.GetConstructors())
{
Expand Down Expand Up @@ -323,7 +323,7 @@ private static bool TryCreateParameterMap(ParameterInfo[] constructorParameters,
for (int i = 0; i < argumentTypes.Length; i++)
{
bool foundMatch = false;
TypeInfo? givenParameter = argumentTypes[i].GetTypeInfo();
Type? givenParameter = argumentTypes[i];

for (int j = 0; j < constructorParameters.Length; j++)
{
Expand All @@ -333,7 +333,7 @@ private static bool TryCreateParameterMap(ParameterInfo[] constructorParameters,
continue;
}

if (constructorParameters[j].ParameterType.GetTypeInfo().IsAssignableFrom(givenParameter))
if (constructorParameters[j].ParameterType.IsAssignableFrom(givenParameter))
{
foundMatch = true;
parameterMap[j] = i;
Expand Down Expand Up @@ -369,13 +369,13 @@ public int Match(object[] givenParameters)
int applyExactLength = 0;
for (int givenIndex = 0; givenIndex != givenParameters.Length; givenIndex++)
{
TypeInfo? givenType = givenParameters[givenIndex]?.GetType().GetTypeInfo();
Type? givenType = givenParameters[givenIndex]?.GetType();
bool givenMatched = false;

for (int applyIndex = applyIndexStart; givenMatched == false && applyIndex != _parameters.Length; ++applyIndex)
{
if (_parameterValues[applyIndex] == null &&
_parameters[applyIndex].ParameterType.GetTypeInfo().IsAssignableFrom(givenType))
_parameters[applyIndex].ParameterType.IsAssignableFrom(givenType))
{
givenMatched = true;
_parameterValues[applyIndex] = givenParameters[givenIndex];
Expand Down
73 changes: 33 additions & 40 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace Microsoft.Extensions.Configuration
/// </summary>
public static class ConfigurationBinder
{
private const BindingFlags DeclaredOnlyLookup = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly;

/// <summary>
/// Attempts to bind the configuration instance to a new instance of type T.
/// If this configuration section has a value, that will be used.
Expand Down Expand Up @@ -179,7 +181,7 @@ private static void BindNonScalar(this IConfiguration configuration, object inst
{
if (instance != null)
{
foreach (PropertyInfo property in GetAllProperties(instance.GetType().GetTypeInfo()))
foreach (PropertyInfo property in GetAllProperties(instance.GetType()))
{
BindProperty(property, instance, configuration, options);
}
Expand Down Expand Up @@ -214,20 +216,18 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig
}
}

private static object BindToCollection(TypeInfo typeInfo, IConfiguration config, BinderOptions options)
private static object BindToCollection(Type type, IConfiguration config, BinderOptions options)
{
Type type = typeof(List<>).MakeGenericType(typeInfo.GenericTypeArguments[0]);
object instance = Activator.CreateInstance(type);
BindCollection(instance, type, config, options);
Type genericType = typeof(List<>).MakeGenericType(type.GenericTypeArguments[0]);
object instance = Activator.CreateInstance(genericType);
BindCollection(instance, genericType, config, options);
return instance;
}

// Try to create an array/dictionary instance to back various collection interfaces
private static object AttemptBindToCollectionInterfaces(Type type, IConfiguration config, BinderOptions options)
{
TypeInfo typeInfo = type.GetTypeInfo();

if (!typeInfo.IsInterface)
if (!type.IsInterface)
{
return null;
}
Expand All @@ -236,13 +236,13 @@ private static object AttemptBindToCollectionInterfaces(Type type, IConfiguratio
if (collectionInterface != null)
{
// IEnumerable<T> is guaranteed to have exactly one parameter
return BindToCollection(typeInfo, config, options);
return BindToCollection(type, config, options);
}

collectionInterface = FindOpenGenericInterface(typeof(IReadOnlyDictionary<,>), type);
if (collectionInterface != null)
{
Type dictionaryType = typeof(Dictionary<,>).MakeGenericType(typeInfo.GenericTypeArguments[0], typeInfo.GenericTypeArguments[1]);
Type dictionaryType = typeof(Dictionary<,>).MakeGenericType(type.GenericTypeArguments[0], type.GenericTypeArguments[1]);
object instance = Activator.CreateInstance(dictionaryType);
BindDictionary(instance, dictionaryType, config, options);
return instance;
Expand All @@ -251,7 +251,7 @@ private static object AttemptBindToCollectionInterfaces(Type type, IConfiguratio
collectionInterface = FindOpenGenericInterface(typeof(IDictionary<,>), type);
if (collectionInterface != null)
{
object instance = Activator.CreateInstance(typeof(Dictionary<,>).MakeGenericType(typeInfo.GenericTypeArguments[0], typeInfo.GenericTypeArguments[1]));
object instance = Activator.CreateInstance(typeof(Dictionary<,>).MakeGenericType(type.GenericTypeArguments[0], type.GenericTypeArguments[1]));
BindDictionary(instance, collectionInterface, config, options);
return instance;
}
Expand All @@ -260,21 +260,21 @@ private static object AttemptBindToCollectionInterfaces(Type type, IConfiguratio
if (collectionInterface != null)
{
// IReadOnlyCollection<T> is guaranteed to have exactly one parameter
return BindToCollection(typeInfo, config, options);
return BindToCollection(type, config, options);
}

collectionInterface = FindOpenGenericInterface(typeof(ICollection<>), type);
if (collectionInterface != null)
{
// ICollection<T> is guaranteed to have exactly one parameter
return BindToCollection(typeInfo, config, options);
return BindToCollection(type, config, options);
}

collectionInterface = FindOpenGenericInterface(typeof(IEnumerable<>), type);
if (collectionInterface != null)
{
// IEnumerable<T> is guaranteed to have exactly one parameter
return BindToCollection(typeInfo, config, options);
return BindToCollection(type, config, options);
}

return null;
Expand Down Expand Up @@ -349,26 +349,24 @@ private static object BindInstance(Type type, object instance, IConfiguration co

private static object CreateInstance(Type type)
{
TypeInfo typeInfo = type.GetTypeInfo();

if (typeInfo.IsInterface || typeInfo.IsAbstract)
if (type.IsInterface || type.IsAbstract)
{
throw new InvalidOperationException(SR.Format(SR.Error_CannotActivateAbstractOrInterface, type));
}

if (type.IsArray)
{
if (typeInfo.GetArrayRank() > 1)
if (type.GetArrayRank() > 1)
{
throw new InvalidOperationException(SR.Format(SR.Error_UnsupportedMultidimensionalArray, type));
}

return Array.CreateInstance(typeInfo.GetElementType(), 0);
return Array.CreateInstance(type.GetElementType(), 0);
}

if (!typeInfo.IsValueType)
if (!type.IsValueType)
{
bool hasDefaultConstructor = typeInfo.DeclaredConstructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0);
bool hasDefaultConstructor = type.GetConstructors(DeclaredOnlyLookup).Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0);
if (!hasDefaultConstructor)
{
throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type));
Expand All @@ -387,20 +385,18 @@ private static object CreateInstance(Type type)

private static void BindDictionary(object dictionary, Type dictionaryType, IConfiguration config, BinderOptions options)
{
TypeInfo typeInfo = dictionaryType.GetTypeInfo();

// IDictionary<K,V> is guaranteed to have exactly two parameters
Type keyType = typeInfo.GenericTypeArguments[0];
Type valueType = typeInfo.GenericTypeArguments[1];
bool keyTypeIsEnum = keyType.GetTypeInfo().IsEnum;
Type keyType = dictionaryType.GenericTypeArguments[0];
Type valueType = dictionaryType.GenericTypeArguments[1];
bool keyTypeIsEnum = keyType.IsEnum;

if (keyType != typeof(string) && !keyTypeIsEnum)
{
// We only support string and enum keys
return;
}

PropertyInfo setter = typeInfo.GetDeclaredProperty("Item");
PropertyInfo setter = dictionaryType.GetProperty("Item", DeclaredOnlyLookup);
foreach (IConfigurationSection child in config.GetChildren())
{
object item = BindInstance(
Expand All @@ -426,11 +422,9 @@ private static void BindDictionary(object dictionary, Type dictionaryType, IConf

private static void BindCollection(object collection, Type collectionType, IConfiguration config, BinderOptions options)
{
TypeInfo typeInfo = collectionType.GetTypeInfo();

// ICollection<T> is guaranteed to have exactly one parameter
Type itemType = typeInfo.GenericTypeArguments[0];
MethodInfo addMethod = typeInfo.GetDeclaredMethod("Add");
Type itemType = collectionType.GenericTypeArguments[0];
MethodInfo addMethod = collectionType.GetMethod("Add", DeclaredOnlyLookup);

foreach (IConfigurationSection section in config.GetChildren())
{
Expand Down Expand Up @@ -497,7 +491,7 @@ private static bool TryConvertValue(Type type, string value, string path, out ob
return true;
}

if (type.GetTypeInfo().IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
{
if (string.IsNullOrEmpty(value))
{
Expand Down Expand Up @@ -550,17 +544,16 @@ private static object ConvertValue(Type type, string value, string path)

private static Type FindOpenGenericInterface(Type expected, Type actual)
{
TypeInfo actualTypeInfo = actual.GetTypeInfo();
if (actualTypeInfo.IsGenericType &&
if (actual.IsGenericType &&
actual.GetGenericTypeDefinition() == expected)
{
return actual;
}

IEnumerable<Type> interfaces = actualTypeInfo.ImplementedInterfaces;
Type[] interfaces = actual.GetInterfaces();
foreach (Type interfaceType in interfaces)
{
if (interfaceType.GetTypeInfo().IsGenericType &&
if (interfaceType.IsGenericType &&
interfaceType.GetGenericTypeDefinition() == expected)
{
return interfaceType;
Expand All @@ -569,16 +562,16 @@ private static Type FindOpenGenericInterface(Type expected, Type actual)
return null;
}

private static IEnumerable<PropertyInfo> GetAllProperties(TypeInfo type)
private static IEnumerable<PropertyInfo> GetAllProperties(Type type)
{
var allProperties = new List<PropertyInfo>();

do
{
allProperties.AddRange(type.DeclaredProperties);
type = type.BaseType.GetTypeInfo();
allProperties.AddRange(type.GetProperties(DeclaredOnlyLookup));
type = type.BaseType;
}
while (type != typeof(object).GetTypeInfo());
while (type != typeof(object));

return allProperties;
}
Expand Down
6 changes: 3 additions & 3 deletions .../Microsoft.Extensions.Configuration.UserSecrets/src/UserSecretsConfigurationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static class UserSecretsConfigurationExtensions
/// <returns>The configuration builder.</returns>
public static IConfigurationBuilder AddUserSecrets<T>(this IConfigurationBuilder configuration)
where T : class
=> configuration.AddUserSecrets(typeof(T).GetTypeInfo().Assembly, optional: false, reloadOnChange: false);
=> configuration.AddUserSecrets(typeof(T).Assembly, optional: false, reloadOnChange: false);

/// <summary>
/// <para>
Expand All @@ -47,7 +47,7 @@ public static IConfigurationBuilder AddUserSecrets<T>(this IConfigurationBuilder
/// <returns>The configuration builder.</returns>
public static IConfigurationBuilder AddUserSecrets<T>(this IConfigurationBuilder configuration, bool optional)
where T : class
=> configuration.AddUserSecrets(typeof(T).GetTypeInfo().Assembly, optional, reloadOnChange: false);
=> configuration.AddUserSecrets(typeof(T).Assembly, optional, reloadOnChange: false);

/// <summary>
/// <para>
Expand All @@ -66,7 +66,7 @@ public static IConfigurationBuilder AddUserSecrets<T>(this IConfigurationBuilder
/// <returns>The configuration builder.</returns>
public static IConfigurationBuilder AddUserSecrets<T>(this IConfigurationBuilder configuration, bool optional, bool reloadOnChange)
where T : class
=> configuration.AddUserSecrets(typeof(T).GetTypeInfo().Assembly, optional, reloadOnChange);
=> configuration.AddUserSecrets(typeof(T).Assembly, optional, reloadOnChange);

/// <summary>
/// <para>
Expand Down
6 changes: 3 additions & 3 deletions ...raries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void AddUserSecrets_FindsAssemblyAttribute()

SetSecret(TestSecretsId, configKey, randValue);
var config = new ConfigurationBuilder()
.AddUserSecrets(typeof(ConfigurationExtensionTest).GetTypeInfo().Assembly)
.AddUserSecrets(typeof(ConfigurationExtensionTest).Assembly)
.Build();

Assert.Equal(randValue, config[configKey]);
Expand All @@ -86,12 +86,12 @@ public void AddUserSecrets_ThrowsIfAssemblyAttributeFromType()
{
var ex = Assert.Throws<InvalidOperationException>(() =>
new ConfigurationBuilder().AddUserSecrets<string>());
Assert.Equal(SR.Format(SR.Error_Missing_UserSecretsIdAttribute, typeof(string).GetTypeInfo().Assembly.GetName().Name),
Assert.Equal(SR.Format(SR.Error_Missing_UserSecretsIdAttribute, typeof(string).Assembly.GetName().Name),
ex.Message);

ex = Assert.Throws<InvalidOperationException>(() =>
new ConfigurationBuilder().AddUserSecrets(typeof(JObject).Assembly));
Assert.Equal(SR.Format(SR.Error_Missing_UserSecretsIdAttribute, typeof(JObject).GetTypeInfo().Assembly.GetName().Name),
Assert.Equal(SR.Format(SR.Error_Missing_UserSecretsIdAttribute, typeof(JObject).Assembly.GetName().Name),
ex.Message);
}

Expand Down
24 changes: 12 additions & 12 deletions src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Microsoft.Extensions.DependencyInjection.ServiceLookup
internal class CallSiteFactory
{
private const int DefaultSlot = 0;
private readonly List<ServiceDescriptor> _descriptors;
private readonly ServiceDescriptor[] _descriptors;
private readonly ConcurrentDictionary<Type, ServiceCallSite> _callSiteCache = new ConcurrentDictionary<Type, ServiceCallSite>();
private readonly Dictionary<Type, ServiceDescriptorCacheItem> _descriptorLookup = new Dictionary<Type, ServiceDescriptorCacheItem>();

Expand All @@ -24,27 +24,27 @@ internal class CallSiteFactory
public CallSiteFactory(IEnumerable<ServiceDescriptor> descriptors)
{
_stackGuard = new StackGuard();
_descriptors = descriptors.ToList();
_descriptors = descriptors.ToArray();

@stephentoub stephentoub Nov 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the benefit of this change? Is _descriptors accessed a lot such that the overhead of going through _descriptors as a list vs an array mattered?

@eerhardt eerhardt Nov 18, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit was very minimal, probably not even observable. I started this change by looking at CallSiteFactory.Populate (which is 75% of the above benchmark). And this ToList() jumped out at me and looking at the code I noticed we don't need a List, but can just use an Array instead. I just figured while I was in here, it was one less object to create.

@stephentoub stephentoub Nov 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just figured while I was in here, it was one less object to create.

But unless the array is the exact right size, ToArray needs to allocate one at the end and copy all the elements to it. ToList doesn't. So you still generally end up with another object, but typically a bigger one.

@eerhardt eerhardt Nov 19, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the Type that is passed in to descriptors is always an IServiceCollection, which implements ICollection<T>. So the size will be known up-front and only a single Array needs to be allocated. It won't need to be resized in the end.

@stephentoub stephentoub Nov 19, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool

Populate();
}

private void Populate()
{
foreach (ServiceDescriptor descriptor in _descriptors)
{
TypeInfo serviceTypeInfo = descriptor.ServiceType.GetTypeInfo();
if (serviceTypeInfo.IsGenericTypeDefinition)
Type serviceType = descriptor.ServiceType;
if (serviceType.IsGenericTypeDefinition)
{
TypeInfo implementationTypeInfo = descriptor.ImplementationType?.GetTypeInfo();
Type implementationType = descriptor.ImplementationType;

if (implementationTypeInfo == null || !implementationTypeInfo.IsGenericTypeDefinition)
if (implementationType == null || !implementationType.IsGenericTypeDefinition)
{
throw new ArgumentException(
SR.Format(SR.OpenGenericServiceRequiresOpenGenericImplementation, descriptor.ServiceType),
"descriptors");
}

if (implementationTypeInfo.IsAbstract || implementationTypeInfo.IsInterface)
if (implementationType.IsAbstract || implementationType.IsInterface)
{
throw new ArgumentException(
SR.Format(SR.TypeCannotBeActivated, descriptor.ImplementationType, descriptor.ServiceType));
Expand All @@ -53,11 +53,11 @@ private void Populate()
else if (descriptor.ImplementationInstance == null && descriptor.ImplementationFactory == null)
{
Debug.Assert(descriptor.ImplementationType != null);
TypeInfo implementationTypeInfo = descriptor.ImplementationType.GetTypeInfo();
Type implementationType = descriptor.ImplementationType;

if (implementationTypeInfo.IsGenericTypeDefinition ||
implementationTypeInfo.IsAbstract ||
implementationTypeInfo.IsInterface)
if (implementationType.IsGenericTypeDefinition ||
implementationType.IsAbstract ||
implementationType.IsInterface)
{
throw new ArgumentException(
SR.Format(SR.TypeCannotBeActivated, descriptor.ImplementationType, descriptor.ServiceType));
Expand Down Expand Up @@ -161,7 +161,7 @@ private ServiceCallSite TryCreateEnumerable(Type serviceType, CallSiteChain call
{
int slot = 0;
// We are going in reverse so the last service in descriptor list gets slot 0
for (int i = _descriptors.Count - 1; i >= 0; i--)
for (int i = _descriptors.Length - 1; i >= 0; i--)
{
ServiceDescriptor descriptor = _descriptors[i];
ServiceCallSite callSite = TryCreateExact(descriptor, itemType, callSiteChain, slot) ??
Expand Down
Loading