-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
Always-on AOT #1738
Comments
MessagePack attributes that the analyzer responds to only requires a reference to the |
I have the changes ready for this. I'll wait to send the pull request until after #1736 has merged, since the changes are based on that. |
What about private member access? Should formatters be generated as private classes of the formatted type to gain access? We should add an override switch to MessagePackSerializerOptions to force use of dynamic formatters. |
Actually that doesn't work, because |
Maybe the better fix for this is to not overload |
On the subject of AOT formatters that can access private members via being a nested class, I think that'll work. But we'll have to take care for the generated code to handle multiple levels of nesting. e.g. the type that needs a formatter may itself be a nested type. |
Here is my code for handling nested types in source generator. ContainingTypeInfo.csusing Microsoft.CodeAnalysis;
using System.Text;
namespace BlazorApp1.SourceGenerator;
public readonly struct ContainingTypeInfo : IEquatable<ContainingTypeInfo>
{
public readonly string? ContainingNamespaceDisplayName;
/// <summary>
/// 0: Most inner type
/// ^1: Most outer type
/// </summary>
public readonly ContainingType[] Types;
private static int CountDepth(INamedTypeSymbol symbol, int depth, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
if (symbol.ContainingType is null)
{
return depth;
}
return CountDepth(symbol.ContainingType, depth + 1, cancellationToken);
}
/// <summary>
/// Collect minimum information for generating partial class.
/// </summary>
/// <param name="symbol">Must be inner type.</param>
public ContainingTypeInfo(INamedTypeSymbol symbol, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
ContainingNamespaceDisplayName = symbol.ContainingNamespace?.ToDisplayString();
Types = new ContainingType[CountDepth(symbol, 1, cancellationToken)];
Types[0] = new(symbol, cancellationToken);
for (int index = 1; symbol.ContainingType is { } containingTypeSymbol; index++)
{
cancellationToken.ThrowIfCancellationRequested();
Types[index] = new(containingTypeSymbol, cancellationToken);
symbol = containingTypeSymbol;
}
}
public ContainingTypeInfo()
{
ContainingNamespaceDisplayName = null;
Types = [];
}
public StringBuilder AppendFullName(StringBuilder? builder = default)
{
builder ??= new StringBuilder(64);
if (ContainingNamespaceDisplayName is not null)
{
builder.Append(ContainingNamespaceDisplayName);
builder.Append('.');
}
for (int i = 0; i < Types.Length;)
{
builder.Append(Types[i].Name);
if (++i < Types.Length)
{
builder.Append('.');
}
}
return builder;
}
public override bool Equals(object? other)
{
if (other is not ContainingTypeInfo value)
{
return false;
}
return Equals(in value);
}
public bool Equals(ContainingTypeInfo other)
{
return Equals(in other);
}
public bool Equals(in ContainingTypeInfo other)
{
if (ContainingNamespaceDisplayName != other.ContainingNamespaceDisplayName)
{
return false;
}
if (Types.Length != other.Types.Length)
{
return false;
}
for (int i = 0; i < Types.Length; i++)
{
if (!Types[i].Equals(in other.Types[i]))
{
return false;
}
}
return true;
}
public override int GetHashCode()
{
return ((ContainingNamespaceDisplayName?.Length ?? 0) << 16) | (Types.Length);
}
public readonly struct ContainingType : IEquatable<ContainingType>
{
public readonly string Name;
public readonly bool IsStatic;
public readonly bool IsValueType;
public readonly bool IsRecord;
public readonly string[] TypeParameters;
public ContainingType(INamedTypeSymbol symbol, CancellationToken cancellationToken)
{
Name = symbol.Name;
IsStatic = symbol.IsStatic;
IsValueType = symbol.IsValueType;
IsRecord = symbol.IsRecord;
if (symbol.TypeParameters.Length == 0)
{
TypeParameters = [];
return;
}
TypeParameters = new string[symbol.TypeParameters.Length];
for (int i = 0; i < TypeParameters.Length; i++)
{
cancellationToken.ThrowIfCancellationRequested();
TypeParameters[i] = symbol.TypeParameters[i].Name;
}
}
public override bool Equals(object? other)
{
if (other is not ContainingType value)
{
return false;
}
return Equals(in value);
}
public bool Equals(ContainingType other)
{
return Equals(in other);
}
public bool Equals(in ContainingType other)
{
if (Name != other.Name)
{
return false;
}
if (IsStatic != other.IsStatic)
{
return false;
}
if (IsValueType != other.IsValueType)
{
return false;
}
if (IsRecord != other.IsRecord)
{
return false;
}
if (!ReferenceEquals(TypeParameters, other.TypeParameters))
{
if (TypeParameters.Length != other.TypeParameters.Length)
{
return false;
}
for (int i = 0; i < TypeParameters.Length; i++)
{
if (TypeParameters[i] != other.TypeParameters[i])
{
return false;
}
}
}
return true;
}
public override int GetHashCode()
{
return (TypeParameters.Length << 16) | (Name.Length << 3) | ((IsStatic ? 1 : 0) << 2) | ((IsValueType ? 1 : 0) << 1) | (IsRecord ? 1 : 0);
}
public static bool operator ==(in ContainingType left, in ContainingType right)
{
return left.Equals(in right);
}
public static bool operator !=(in ContainingType left, in ContainingType right)
{
return !left.Equals(in right);
}
}
public static bool operator ==(in ContainingTypeInfo left, in ContainingTypeInfo right)
{
return left.Equals(in right);
}
public static bool operator !=(in ContainingTypeInfo left, in ContainingTypeInfo right)
{
return !left.Equals(in right);
}
} |
I've been giving some thought to how we could make AOT generation more "on by default". With source generation being so much easier than the old mpc flow, it seems we may be within striking distance. So here's a thought I had tonight:
Suppose the source generator always generates the formatters, even if the user doesn't define a partial class with the
[GeneratedMessagePackResolver]
attribute. Suppose further that theDynamicObjectResolver
looks for these formatters in the same assembly as the type to be formatted before it generates one dynamically and uses the pre-compiled one if it's found.Now, we could 'discover' the formatter for a given type a variety of ways. But the most performant way is probably to... generate the resolver too! Then we have just one type to find via reflection for the whole assembly, and if we find it, we activate it (from the
DynamicObjectResolver
) and use it to search for pre-created formatters before dynamically creating them. How do we find the resolver? Well, we could emit one assembly-level attribute that we search for at runtime that points directly at the resolver. This could work whether the resolver is declared partially by the user code (and thus in a user-determined namespace) or whether we just generated it fully automatically.Now what about strictly AOT environments where reflection to find the formatters or resolver doesn't work? Well, that can work the same way today (in this PR): the user declares the partial class for the resolver to effectively control the namespace and name of the resolver so the user can write code that finds it up-front, thereby avoiding all reflection.
The user can also opt into declaring the partial class explicitly in order to specify non-default options for code generation in the attribute on the resolver class.
This proposal means that anyone compiling against MessagePack v3 would effectively get AOT performance 'for free'. It also means these AOT code generators had better be good, because they'll be promoted from being used for a (small?) subset of projects to all projects. We tend to get bug reports fairly regularly that are unique to AOT formatters, so since the new source generator is based on the same T4 templates, we'll inherit those and need to be prepared to respond quickly to incoming bug reports.
Originally posted by @AArnott in #1736 (review)
The text was updated successfully, but these errors were encountered: