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

Q: AOT and composite type reflection #63670

Closed
tmds opened this issue Jan 12, 2022 · 8 comments
Closed

Q: AOT and composite type reflection #63670

tmds opened this issue Jan 12, 2022 · 8 comments
Labels
area-NativeAOT-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@tmds
Copy link
Member

tmds commented Jan 12, 2022

I'm trying to make some code AOT friendly, and I'm not sure how to go about it.

The code uses the .NET type system to represent protocol types. The types can be basic types, but there are also composite types such as arrays, structs, and dictionaries.

The following example uses MakeGenericMethod to deal with the composite types, and I'm not sure that will work with AOT? and what I can do to to make it AOT friendly?

using System.Reflection;

Writer writer = default;
writer.WriteDictionary(new Dictionary<int, IDictionary<int, int>> { { 1, new Dictionary<int, int> { { 2, 3 } } } });

ref struct Writer
{
    private delegate void ValueWriter(ref Writer writer, object value);

    public void WriteDictionary<TKey, TValue>(IDictionary<TKey, TValue> dictionary)
    {
        foreach (var item in dictionary)
        {
            Write<TKey>(item.Key);
            Write<TValue>(item.Value);
        }
    }

    public void WriteInt(int i)
    {
        Console.WriteLine(i);
    }

    private static void WriteDictionaryCore<TKey, TValue>(ref Writer writer, object o)
    {
        writer.WriteDictionary<TKey, TValue>((IDictionary<TKey, TValue>)o);
    }

    private void WriteDictionaryTyped(Type keyType, Type valueType, object o)
    {
        var method = typeof(Writer).GetMethod(nameof(WriteDictionaryCore), BindingFlags.Static | BindingFlags.NonPublic)!
            .MakeGenericMethod(new[] { keyType, valueType });
        var dlg = method!.CreateDelegate<ValueWriter>();
        dlg.Invoke(ref this, o);
    }

    private void Write<T>(T value)
    {
        if (typeof(T) == typeof(int))
        {
            WriteInt((int)(object)value!);
            return;
        }
        else if (typeof(T).GetGenericTypeDefinition() == typeof(IDictionary<,>))
        {
            WriteDictionaryTyped(typeof(T).GenericTypeArguments[0], typeof(T).GenericTypeArguments[1], (object)value!);
            return;
        }
        throw new NotSupportedException(typeof(T).FullName);
    }
}

cc @MichalStrehovsky

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 12, 2022
@MichalStrehovsky MichalStrehovsky added area-NativeAOT-coreclr question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Jan 12, 2022
@MichalStrehovsky
Copy link
Member

First question would be about your perf constraints - can you go with IDictionary (the non-generic interface) instead of IDictionary<,>? Yes, it will lead to boxing, but one can do a lot of boxing before MakeGenericMethod/CreateDelegate completes (basically the strong typing is reducing boxing at the cost of doing extra code generation). Short lived allocations that die in Gen0 are pretty cheap in comparison. It just depends on how many there are.

@tmds
Copy link
Member Author

tmds commented Jan 12, 2022

First question would be about your perf constraints - can you go with IDictionary (the non-generic interface) instead of IDictionary<,>?

I want to make it as good as I can for a reasonable effort.
To avoid the boxing I would probably need to write a source generator? I'm not willing to go that far (yet).

I'll try the boxing approach.

If I still like to keep the MakeGenericMethod/CreateDelegate on platforms that support it, is there a way to check at runtime the platform is AOT and then use the boxing methods?

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Jan 12, 2022

RuntimeFeature.IsDynamicCodeSupported is the API to call if you take that route.

But if you have guarantees that WriteDictionaryCore will always be called with either a reference type, or an int (i.e. IDictionary<,> is not implemented on a struct), you can take advantage of how generic code is implemented in .NET (https://twitter.com/MStrehovsky/status/1244602445461950470):

You only need to make sure WriteDictionaryCore was seen as instantiated over all the combinations of a reference type and int that you expect. This ensures the code is generated and all MakeGenericMethod has to do is to generate a data structure (not code, because code already exists and was generated ahead of time).

if (string.Empty.Length > 0)
{
    Writer w = default;
    WriteDictionaryCore<int, object>(ref w, null);
    WriteDictionaryCore<int, int>(ref w, null);
    WriteDictionaryCore<object, object>(ref w, null);
}

This approach works as long as the list of valuetypes you're operating on has a statically knowable bound. List of reference types doesn't matter because of how generic sharing works.

@hez2010
Copy link
Contributor

hez2010 commented Jan 12, 2022

How about introducing a generic version of MakeGenericMethod/MakeGenericType so that the dependency analyzer may be able to take them into account as well based on control-flow analysis?

For example:

private readonly MethodInfo method = typeof(Foo).GetMethod("Bar"); // the analyzer recognized `Foo.Bar` from class initializer

void Test()
{
    var m = method.MakeGenericMethod<int, int>(); // given that the analyzer already knows `method` is `Foo.Bar`, add `Foo.Bar<int, int>` into the dependency graph
    m.Invoke(1, 2);
}

@MichalStrehovsky
Copy link
Member

How about introducing a generic version of MakeGenericMethod/MakeGenericType so that the dependency analyzer may be able to take them into account as well based on control-flow analysis?

Do you have a real-world scenario where this would help?

In your example, if we statically know the method, and statically know the arguments, the user should just do:

void Test()
{
    Foo.Bar<int, int>.Invoke(1, 2);
}

It looks like #81204 combined with dotnet/linker#2482

@tmds
Copy link
Member Author

tmds commented Jan 13, 2022

How about introducing a generic version of MakeGenericMethod/MakeGenericType

It doesn't help here because we can't get the generic types.

From new Dictionary<int, IDictionary<int, int>> we get TKey = int, TValue = IDictionary<int, int>.
After that there is no way to 'decompose' TValue further, we can only use it as a Type.

@tmds
Copy link
Member Author

tmds commented Jan 13, 2022

@MichalStrehovsky, thanks for your suggestions. I will try them, and may get back if I have additional questions.

@tmds tmds closed this as completed Jan 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

3 participants