Any way of using MemoryPackable collection as generic?

[dusrdev]

I am using MemoryPack in a package, which needs to be AOT compatible, thus in every place I use it, I need to ensure the compiler has enough information to avoid it being trimmed.

For example to use types like this:

[MemoryPackable]
public partial record Person...

I can use:

public void Process<T>(T object) where T : IMemoryPackable<T> {...}

which works well because the source generator implements this in the type.

But I also want to support collections that inherit from natively supported collections, such as:

[MemoryPackable(GenerateType.Collection)]
public partial class Map : Dictionary<byte, uint> { }

But I can't figure out how or if it is even possible.

I assumed the source generator would implement Map as IMemoryPackable<Dictionary<byte, uint>>, which then would enable me to use the same abstraction, but it doesn't implement IMemoryPackable at all, nor any other interface which I can use as generic to pass down to the Serialize method.

Is there anything that could be done?

[hadashiA]

Hi, Your sample seems to work.

var bin = MemoryPackSerializer.Serialize(new Map
{
    { 1, 100 },
    { 2, 200 },
});

var d = MemoryPackSerializer.Deserialize<Map>(bin);
d[1] // #=> 100

The basic usage is MemoryPackSerializer, so I don't see a problem. How about it?

Built-in collections and primitive types are not IMemoryPackable. Considering that not all serializable types are IMemoryPack,able there didn't seem to be any consistency issues I think.

[dusrdev]

@hadashiA using MemoryPackSerializer.Serialize or Deserialize is not direct, as it is wrapped in my package, as in my example it would be:

public void Process<T>(T object) where T : IMemoryPackable<T> {...}

for stuff that implement IMemoryPackable<T>, but if I do this:

public void Process<T>(T object) { MemoryPackSerializer.Serialize(object);... }

Just using T is too loose of a constaint, AOT compilation warns that this may be trimmed, since there is no way for the compiler to ensure that T is a built-in collection, primitive, or IMemoryPackable<T>, making it possible that a non-supported type could be passed to it.

I am looking for a way to further constraint this T, to make sure it won't be trimmed or produce trimming warnings for my package, but still support built-in collections of primitive or other supported types.

[hadashiA]

It may be difficult to assume that all generated types are IMemoryPackable.

Besides primitive types and standard collections, the base type of Union, for example, is not IMemoryPackable.

// IA is not IMemoryPackable, even if A is.
[MemoryPackable]
[MemoryPackUnion(0, typeof(A))]
partial interface IA.
{
}

Currently, the IMemoryPackable interface is primarily a optimization used internally.

I am looking for a way to further constraint this T, to make sure it won't be trimmed or produce trimming warnings for my package, but still support built-in collections of primitive or other supported types.

Is your issue to work with AOT? If you want to prevent trimming of the type parameter T, it seems that you can specify the following

public void Process<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(T object);

For your information, this correspondence was added to the main body in #237. https://github.com/Cysharp/MemoryPack/pull/237/files#diff-ae58ac4745c242749c7911d46c18a74146b263b06eebd123f0ba031dd4237ab8R15-R17

Can you also try the latest version ?

Also, if the problem is that it doesn't work with the AOT build, please give me a sample that reproduces it with the AOT build.

[dusrdev]

@hadashiA I have updated to the new version (1.20.0), it seems now it is even worse (at least analyser wise), DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All) signifies that some members might be accessed by reflection, thus producing a lot of trim warnings, reflection and AOT are like oil and water, don't mix very well.

In version 1.10.0, using the constraint: where T : IMemoryPackable<T>, allowed <IsAotCompatible>true></IsAotCompatible> to be set while still using MemoryPackSerializer.Serialize<T> without any warnings whatsoever.

In version 1.20.0, this now produces warnings:

And if I do add them to may package, it won't be AotCompatible anymore (not without warnings), which is worse than before.

I would revert the change made in v1.20.0 that caused this...

As of this issue, the possible solutions would be to abstract the T to some constraint that primitives and built-in collections can both satisfy, but this is difficult since there is not control over them, meaning you can't simply add an interface implementation for them.

But, inheritence like in Map in the example above, does add some flexibilty, from what I understand (and I may be wrong), MemoryPackable(GenerateType.Collection) just adds a formatter that serializes this object as the object it inherits, but perhaps it could also implement an interface, that ensures compatability without requiring member information from reflection, like IMemoryPackable<T> used to do for T. When we declare our own types, even if they don't override anything from the original collections, like in Map we still have the flexibility of implementing basically anything we want.

The simplest way would probably be creating something like an interface IMemoryPackSupportedType (obviously feel free to change the name), then ensure that the formatters for the built-in types use this constraint, then propegate the constraint throughout the apis, and make sure the source generators implement this interface for at least MemoryPackable(GenerateType.Collection), for this simple use case, the interface doesn't even need to have any functionality, it would be used simply as a constraint, that gives the compiler confidence that reflection won't be needed to analyse this type at runtime, which would bring back AotCompatible to the table.

[hadashiA]

I would like to confirm. Which is the problem you are pointing out?

1. warnings
2. the serializer does not work due to trimming.

We would like to fix 2. But 1 would be difficult to fix right away.

MemoryPack uses reflection internally in its current implementation. My understanding is that unless reflection is removed, it is either warning will be generated or the serializer is broken by trimming.

The reason attribute was added in 1.20.0 is to fix #189. This meant that is to fix runtime errors instead of allowing warnings.

If the serializer is working and warning is the only problem, it seems to me that you can probably just suppress warning on the application side.

Full AOT compatibility would be better discussed in another issue.

[dusrdev]

Well actually there are 3 "problems" if you consider the reason I first opened this issue, which is:

3. To have a way of constraining generics to types that are MemoryPackable(GenerateType.Collection) or built-in supported types. So that there would a constraint for T which will tell the compiler if T is either MemoryPackable(GenerateType.Collection) or other built-in supported type. Similar to how IMemoryPackable<T>.

But now without any changes on topic 3 in part, by updating to v1.20.0, my package also produces trim warnings (in places it didn't before), My package is aimed to be fully AOT compatible, and this is a big deal.

I cannot distinguish in my case between 1 and 2, since the package is a library (not an end product), Other users use it, so I can't cover all of their use cases with my tests, especially when changes could happen in runtime. So I must rely on warnings to ensure that my package will work as intended, and so do the users. When I see the warnings I can't be sure that it won't fail at runtime, If I propegate the attribute DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All), now the users will have the same concerns as me.

Moreover, in the previous version, My constraint which was where T : IMemoryPackable<T> was tighter then DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All), thus I ensure that when users use my package, it won't allow the usage of their type unless they use the MemoryPackable attribute which will make their type implement this interface. Essentially making sure it works at runtime, thus it was AOT compatible with 0 warnings.

DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All) is a looser constaint, which if I allow, the users could then pass types that will really fail during runtime, which is bad (we don't want C# to be like Python).

I think removing the DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All) attribute is now the priority issue. Because before it was added, I could use MemoryPack with full AOT compatability without any warnings, and users didn't report any issues in repo, nor did I exprience any in tests. Perhaps there is a better way to fix #189

After this we could proceed with 3 as a feature request.

[hadashiA]

To have a way of constraining generics to types that are MemoryPackable(GenerateType.Collection) or built-in supported types. So that there would be a constraint for T which will tell the compiler if T is either MemoryPackable(GenerateType.Collection) or other built-in supported types. IMemoryPackable.

Hmmm. Why is this a problem? Please tell me what the problem is other than warnings.

There are no type constraints in System.Text.Json or any other common serializer in C#. So users don't expect it. You probably expect type constraints for AOT compatibility, but I don't think that's the only way for AOT compatibility.

I think removing the DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All) attribute is now a priority issue. Because before it was added, I could use MemoryPack with full AOT compatibility without any warnings, and users didn't report any issues in repo, nor did I exprience any in tests . Perhaps there is a better way to fix #189

As per #189, in previous versions, if there is no Serialize in the source code that uses MemoryPack, but only Deserialize, deserialization will not work due to trimming. This is easily reproducible; turning off attribute will cause a runtime error.

I cannot distinguish in my case between 1 and 2, since the package is a library (not an end product), Other users use it, so I can't cover all of their use cases with my tests, especially when changes could happen in runtime. So I must rely on warnings to ensure that my package will work as intended, and so do the users. When I see the warnings I can't be sure that it won't fail at runtime, If I propegate the attribute DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All), now the users will have the same concerns as me.

The current MemoryPack works mostly with AOT, but there are no compiler guarantees. That is the reality. So it seems rather correct that warn is issued.

It does not matter whether the DynamicallyAccessdMembers is present or not. Without it, it was not visible that there was reflection, which was dangerous.

If you have created an AOT library that wraps MemoryPack, why not create an interface constraint in your library or add an UnconditionalSuppressMessageAttribute at the point where MemoryPack is called?

I may need to fix the reflection part to get rid of the warning completely.

I have created a separate issue, so it would be better to discuss the rest of the issue there. #251

[dusrdev]

@hadashiA

Hmmm. Why is this a problem? Please tell me what the problem is other than warnings.

There are no type constraints in System.Text.Json or any other common serializer in C#. So users don't expect it. You probably expect type constraints for AOT compatibility, but I don't think that's the only way for AOT compatibility.

System.Text.Json also produces trim warnings in the default Serialize and Deserialize methods, but it offers AOT safe overloads for both that require either a JsonSerializerContext (which removes the warnings but still makes it so that it a context which doesn't contain the right type information is used, in which case an exception will be thrown at runtime, or even better a JsonTypeInfo<T> which is source generated inside a JsonSerializerContext when type T is used inside via the attribute. This JsonTypeInfo<T> ensures the compiler knows that it will have the required information at runtime. And as we talked about build-in types and primitives, a JsonTypeInfo<T> can be added for any of those as well, simply by passing their type in the attribute.

Perhaps a similar approach could be implemented in MemoryPack, even without actually changing previous code, but rather creating a source generated MemoryPackSerializerContext, which then would create MemoryPackTypeInfo<T> for each type that is added to it with an attribute, then create overloads for Serialize and Deserialize that use said MemoryPackTypeInfo<T> to get all the information they need without reflection.

This would solve the issue, as well as give users an AOT compatible alternative (without reflection), meanwhile you can work out the original reflection details, or even make them obsolete.

[hadashiA]

Thanks for the explanation, and I agree with you about System.Text.Json. Perhaps you agree that talking about making IMemoryPackable all custom types is not the only solution to the AOT problem, so I will close this issue. (I'd like to continue in #251.)