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

[V4] Address Native AOT warnings for DynamoDB's Document Model library #3583

Open
wants to merge 1 commit into
base: v4-development
Choose a base branch
from

Conversation

normj
Copy link
Member

@normj normj commented Dec 19, 2024

Description

Note: This is just the Document Model. Not the DataModel (aka Object Persistence Model). The DataModel builds on top of the Document Model so getting the Document Model library Native AOT complaint is the prerequisite

This PR removes the RequiresUnreferencedCode from the DocumentModel library and adds the appropriate DynamicallyAccessedMembers attributes and code adjustments to make Native AOT trim warnings go away when using the DocumentModel library with Native AOT.

Motivation and Context

#3475
Add support for using the Document Model library using Native AOT. Using Native AOT for the following Lambda function reduced the cold start time from approximately 1,500 ms to 500 ms.

public class Function
{
    static IAmazonDynamoDB _ddbClient;
    static Table _table;

    private static async Task Main()
    {
        _ddbClient = new AmazonDynamoDBClient(RegionEndpoint.USWest2);

        _table = new TableBuilder(_ddbClient, "ZipCodes")
                            .AddHashKey("Code", DynamoDBEntryType.String)
                            .Build();


        Func<string, ILambdaContext, Task<string>> handler = FunctionHandler;
        await LambdaBootstrapBuilder.Create(handler, new SourceGeneratorLambdaJsonSerializer<LambdaFunctionJsonSerializerContext>())
            .Build()
            .RunAsync();
    }


    public static async Task<string> FunctionHandler(string input, ILambdaContext context)
    {
        var document = await _table.GetItemAsync(input);
        if (document == null)
        {
            return $"No zip code found for {input}";
        }

        return $"{document["City"].AsString()}, {document["State"].AsString()}";
    }
}


[JsonSerializable(typeof(string))]
public partial class LambdaFunctionJsonSerializerContext : JsonSerializerContext
{
}

Testing

Successful dry run
Build Native AOT Lambda function using Native AOT to ensure no trim warnings were produced.
Built a standalone console application that exercised more API then the Lambda function compiled and executed locally using Native AOT for Windows.

When I do Native AOT compile on Windows I do still get the following trim warning but it doesn't come from the AWS SDK and the warning does not happen when targeting Linux.

> dotnet publish -r win-x64 /p:PublishAot=true
Restore complete (1.0s)
  DynamoDBDocumentAotTest succeeded with 1 warning(s) (28.0s) → bin\Release\net8.0\win-x64\publish\
    /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs(90): warning IL3000: System.Net.Quic.MsQuicApi..cctor(): 'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.

@normj normj added the v4 label Dec 19, 2024
@@ -651,19 +696,6 @@ public virtual bool TryFrom(Document d, Type targetType, out object result)

internal abstract class Converter<T> : Converter
{
public override IEnumerable<Type> GetTargetTypes()
Copy link
Member Author

@normj normj Dec 19, 2024

Choose a reason for hiding this comment

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

I removed this override in the abstract class to instead have the final subclasses return their target types without using unbounded reflection.

internal class ByteConverterV1 : Converter<byte>
{
public override IEnumerable<Type> GetTargetTypes()
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the methods that were added after removing parent override that was using unbounded reflection.

internal abstract class CollectionConverter : Converter
{
private IEnumerable<Type> targetTypes;
private static IEnumerable<Type> GetTargetTypes(IEnumerable<Type> memberTypes)
Copy link
Member Author

@normj normj Dec 19, 2024

Choose a reason for hiding this comment

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

This method was doing lots of reflection to expand all of the "primitive" types into the varies array, list and set variations. This unbounded reflection is not possible in native aot. I replaced the logic by creating a new static list of types with all of the "primitive" permeations.


public override IEnumerable<Type> GetTargetTypes()
{
return Utils.PrimitiveTypesCollectionsAndArray;
Copy link
Member Author

Choose a reason for hiding this comment

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

This new PrimitiveTypesCollectionAndArray field used here is the replacement for the deleted GetTargetTypes method above.

public static Type GetElementType(Type collectionType)

#if NET8_0_OR_GREATER
[System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("ReflectionAnalysis", "IL2073",
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't figure out how to remove the trim warning for the collectionType.GetElementType() type. I'm confident though at this point in using the library that type will already be known. The type would have been referenced in the application code.

To give me extra confidence that nobody was using these utilities methods out of context I changed all the visibility of these utility methods to internal.

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

Successfully merging this pull request may close these issues.

1 participant