Skip to content
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.

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

Add nullable type reflection cache #2715

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

israellot
Copy link

@israellot israellot commented Aug 12, 2022

Deserializing a class like the one bellow has a performance hit presumably due to calls to GetGenericTypeDefinition and Nullable.GetUnderlyingType.
This is aggravated when using converters like StringEnumConverter which add a nullable type check at CanConvert method

public class NullablePropertiesTestClass
{
    public int? a { get; set; }            
    public TestEnum? b { get; set; }
    public bool? c { get; set; }

    public enum TestEnum
    {
        a,
        b,
        c
    }
}

This PR opens up the discussion of adding a simple cache for these expensive reflection calls.
On this particular example, around 30% gains at the expense of the memory for holding the cache.
Benchmark code is included.

Screenshot 2022-08-12 132917
Screenshot 2022-08-12 133803

return new NullableTypeReflectionInfo(false, t, null);

#if !HAVE_CONCURRENT_DICTIONARY
lock(NullableInfoCache){

Choose a reason for hiding this comment

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

Suggested change
lock(NullableInfoCache){
lock(NullableInfoCache) {

if (!NullableInfoCache.TryGetValue(t, out var info))
{
var genericTypeDefinition = t.GetGenericTypeDefinition();
var isNullable = (genericTypeDefinition == typeof(Nullable<>));

Choose a reason for hiding this comment

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

Suggested change
var isNullable = (genericTypeDefinition == typeof(Nullable<>));
var isNullable = genericTypeDefinition == typeof(Nullable<>);

@israellot
Copy link
Author

israellot commented Aug 17, 2022

dotnet/runtime#73860
Related discussion

@@ -0,0 +1,92 @@
#region License

Choose a reason for hiding this comment

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

License

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants