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

Using FlatSharp in Unity throw compilation error. #125

Closed
shwuhk opened this issue Mar 13, 2021 · 36 comments
Closed

Using FlatSharp in Unity throw compilation error. #125

shwuhk opened this issue Mar 13, 2021 · 36 comments

Comments

@shwuhk
Copy link

shwuhk commented Mar 13, 2021

We encounter an issue when using FlatSharp in Unity.
It throw compilation error.
FlatSharp compilation error: (16,68): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> target"

Any hints how to fix this? Thanks.

@shwuhk shwuhk changed the title Using FlatSharp in Unity with IL2CPP throw compilation error. Using FlatSharp in Unity throw compilation error. Mar 13, 2021
@jamescourtney
Copy link
Owner

I'm not very familiar with Unity, so I'll probably be of limited use here, but I will try to help.

I have a few questions:

  1. Are you using the FlatSharp compiler with .fbs files? I'm assuming so, but want to clarify.
  2. Do you know which framework you are using the compiler with? The nuget package includes net5.0, net47, and netcoreapp2.1. I'm able to produce a similiar issue on my machine with net47. If you can, please give the netcoreapp2.1 or net5.0 versions a shot and let me know how those work. It may be that the net47 is missing some dll's or otherwise.

To invoke net5.0: FlatSharp.Compiler.exe --input <input file> --output <output directory>
To invoke netcoreapp2.1: dotnet FlatSharp.Compiler.dll --input <input_file> --output <output directory>

@shtse8
Copy link

shtse8 commented Mar 14, 2021

I'm not very familiar with Unity, so I'll probably be of limited use here, but I will try to help.

I have a few questions:

  1. Are you using the FlatSharp compiler with .fbs files? I'm assuming so, but want to clarify.
  2. Do you know which framework you are using the compiler with? The nuget package includes net5.0, net47, and netcoreapp2.1. I'm able to produce a similiar issue on my machine with net47. If you can, please give the netcoreapp2.1 or net5.0 versions a shot and let me know how those work. It may be that the net47 is missing some dll's or otherwise.

To invoke net5.0: FlatSharp.Compiler.exe --input <input file> --output <output directory>
To invoke netcoreapp2.1: dotnet FlatSharp.Compiler.dll --input <input_file> --output <output directory>

Yes. We are using FlatSharp compiler to compile the fbs from another core3.1 solution before serialization. Since Unity is only allow coding on Net471 and .NET standard 2.0, we are failed to test in another target framework using Unity. But it works fine with the compiled fbs in a new non unity solution with framework core3.1

@jamescourtney
Copy link
Owner

Sorry -- I think you may have misunderstood me.

If you use the FlatSharp compiler from your other project, does the .cs file it generates work in Unity?

@shtse8
Copy link

shtse8 commented Mar 14, 2021

Sorry -- I think you may have misunderstood me.

If you use the FlatSharp compiler from your other project, does the .cs file it generates work in Unity?

yes, I think it works in Unity at least it can be compiled (.cs file) well in Unity. But while I serialize/deserialize the model, error was thrown.

image

The list of errors:

FlatSharp compilation error: (16,68): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> target"
FlatSharp compilation error: (152,17): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> span"
FlatSharp compilation error: (244,17): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> span"
FlatSharp compilation error: (269,17): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> span"
FlatSharp compilation error: (13,59): error CS0535: 'GeneratedSerializer' does not implement interface member 'IGeneratedSerializer<Person>.Write<TSpanWriter>(TSpanWriter, Span<byte>, Person, int, SerializationContext)', Context = ": IGeneratedSerializer<Cubeage.Editor.Build.Person>"
FlatSharp compilation error: (163,17): error CS0433: The type 'Span<T>' exists in both 'System.Memory, Version=4.0.1.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089', Context = "Span<byte> vtable = stackalloc byte[8]"
FlatSharp compilation error: (163,37): error CS0518: Predefined type 'System.Span`1' is not defined or imported, Context = "= stackalloc byte[8]"

I have suspected the compiled .cs file taken from other projects causing the error. So I have tried this:

Person person = new Person(...);
int maxBytesNeeded = FlatBufferSerializer.Default.GetMaxSize(person);
byte[] buffer = new byte[maxBytesNeeded];
int bytesWritten = FlatBufferSerializer.Default.Serialize(person, buffer);

still, same errors. but it works in another core3.1 project. So I think the compiled .cs file should work well. but the library not.

@jamescourtney
Copy link
Owner

You shouldn't use FlatBufferSerializer from a Unity project. FlatBufferSerializer works by creating and compiling code at runtime using Roslyn. The point of the FlatSharp.Compiler project is to do all of this work at build time.

The main page indicates which projects you should reference. Don't use the FlatSharp package if you are targeting Unity. All you need is FlatSharp.Compiler and FlatSharp.Runtime.

Using a serializer from an FBS files works like this:

table MyTable (fs_serializer) { Int:int; }

Then, from your project, you do:

MyTable table = MyTable { Int = 3 };
byte[] buffer = new byte[1024];
MyTable.Serializer.Write(buffer, table);

The samples folder in this project contains lots of resources that can help you. This one in particular should be useful.

@jamescourtney
Copy link
Owner

The issue with the compiler not working with net47 is resolved in 5512d7c. The fix will be in version 5.0.1 and will be published in the next ~30 minutes.

@shtse8
Copy link

shtse8 commented Mar 14, 2021

(fs_serializer)

it works! Thanks!

I don't understand why we are failed to use FlatBufferSerializer in Unity Editor mode, as those scripts are still compiling by Mono which support creating and compiling code at runtime, while built scripts are compiled using IL2CPP which doesn't support creating and compiling codes. The error reported above is running in Unity Editor mode, so we think it should be fine to use FlatSharp package. Also, the error shown is not related to reflection but it about Span<T> exists in two assemblies which are System.Memory and mscorlib.

Another question is, how can I compile .fbs using FlatSharp.Compiler using Unity as the project setting cannot be edited manually.

Thanks for your help!

@jamescourtney
Copy link
Owner

The Mono question is a good one, and I don't have a good answer. This probably means I should add some Mono tests to the suite or a note that it won't work 😀 Compiler version 5.0.1 is now published on NuGet, which should fix your issues with net47 (the problem was a missing app.config).

Regarding the Unity .csproj issue, I'm not sure what the best way to handle this is. My initial advice is that you can invoke the FlatSharp compiler directly through whatever build process that Unity exposes:

net47 (Windows-only):
FlatSharp.Compiler.exe --input <path to fbs file> --output <output directory>

net5.0/netcoreapp2.1 (Cross platform):
dotnet FlatSharp.Compiler.dll --input <path to fbs file> --output <output directory>

If you are interested in contributing to make Unity integration better, I would appreciate the help from someone who is familiar with Unity (I am not!)

@shtse8
Copy link

shtse8 commented Mar 14, 2021

The Mono question is a good one, and I don't have a good answer. This probably means I should add some Mono tests to the suite or a note that it won't work 😀 Compiler version 5.0.1 is now published on NuGet, which should fix your issues with net47 (the problem was a missing app.config).

Regarding the Unity .csproj issue, I'm not sure what the best way to handle this is. My initial advice is that you can invoke the FlatSharp compiler directly through whatever build process that Unity exposes:

net47 (Windows-only):
FlatSharp.Compiler.exe --input <path to fbs file> --output <output directory>

net5.0/netcoreapp2.1 (Cross platform):
dotnet FlatSharp.Compiler.dll --input <path to fbs file> --output <output directory>

If you are interested in contributing to make Unity integration better, I would appreciate the help from someone who is familiar with Unity (I am not!)

Sure. I hope I can help in the future.
One more suggestion, it's better to make model data types extend interface for the serializer generated.
Something like:

public interface IFlatSharpSerializer<T> : IFlatSharpSerializer
{
  void Write(T source);
  T Read(byte[] buffer);
}
public interface IFlatSharpSerializer
{
  void Write(object source);
  object Read(byte[] buffer);
}

With the interfaces, we can easily build our tools for using this library in general and wrap the library if needed. For example, for a game, it has more than several thousands data models. it's hard to manage codes without wrapper. but since the code is generated without interfaces, it's impossible to make tools. What do you think?

@jamescourtney
Copy link
Owner

jamescourtney commented Mar 14, 2021

You're suggesting that each Table that has a serializer implement this interface?

table SomeTable { Int:int; }
[FlatBufferTable]
public partial class SomeTable : IFlatSharpSerializable <-- add this?
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }
}

I think this will get a lot more interesting if this proposal gets implemented: dotnet/csharplang#4436

I'll need to give this some thought. However, one thing you might not know is that FlatSharp-generated classes are already partial, so you can extend them with your own partial definitions to add whatever interfaces and methods you want.

@shtse8
Copy link

shtse8 commented Mar 15, 2021

You're suggesting that each Table that has a serializer implement this interface?

table SomeTable { Int:int; }
[FlatBufferTable]
public partial class SomeTable : IFlatSharpSerializable <-- add this?
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }
}

I think this will get a lot more interesting if this proposal gets implemented: dotnet/csharplang#4436

I'll need to give this some thought. However, one thing you might not know is that FlatSharp-generated classes are already partial, so you can extend them with your own partial definitions to add whatever interfaces and methods you want.

Yes. Extend an interface on table and also another one for table with fs_serializer modifier.

Here is an example:

[FlatBufferTable]
public partial class SomeTable : IFlatTable
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }
}

if there is a fs_serializer modifier in schema, the generated code will be as follow:

[FlatBufferTable]
public partial class SomeTable : IFlatTable, IFlatSerializer
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }
    public SomeTableSerializer Serializer { get; }
}

Yes, you are right. Currently, we can have two choices to make our life easier: implement partial classes or do batch replacing on generated files to add interfaces. However, extending them one by one with thousands tables is not possible or we need to implement a tool to generate codes by ourselves.

For example, Google.Protobuf does extending IMessage. FlatBuffer official compiler does extending IFlatBufferObject, but missing an interface on ObjectT for gen-object-api.

@jamescourtney
Copy link
Owner

So, solving this for the serialize case is pretty straightforward:

public interface IFlatBufferSerializable
{
    ISerializer Serializer { get; }
}

public interface IFlatBufferSerializable<TTable> : IFlatBufferSerializable
{
   ISerializer<TTable> Serializer { get; }
}

[FlatBufferTable]
public partial class SomeTable : IFlatBufferSerializable<SomeTable>
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }

    // already exists; can't break this one
    public static ISerializer<SomeTable> Serializer { get; } = ...;

    // explicit interface implementations avoid name conflicts.
    ISerializer<SomeTable> IFlatBufferSerializable<SomeTable>.Serializer  => SomeTable.Serializer;
    ISerializer IFlatBufferSerializable.Serializer => SomeTable.Serializer;
}

However, deserialize is weird because serializers are done statically in FlatSharp. There are already ISerializer and ISerializer<T> interfaces that operate on Tables, but there is not a single place that they are aggregated. When using FBS files, FlatSharp does not help you write this method:

public T Deserialize<T>(byte[] buffer) { }

Rather, FlatSharp expects you to know the type that you are intending to deserialize. If you have access to reflection, it's pretty easy to grab the Serializer property defined on the object and cache that internally using a Type -> ISerializer dictionary.

Do you have thoughts on the API surface to expose for deserialization? I'm hesitant to do the above because I worry it only solves half your problem, which doesn't really help.

My other thought is: if you have literally thousands of tables, it may make sense to code gen both the FBS file and the partial classes from the same template engine (or same data source, at least). Then you could codegen all of the stuff that you need with just a couple of steps.

@shtse8
Copy link

shtse8 commented Mar 15, 2021

So, solving this for the serialize case is pretty straightforward:

public interface IFlatBufferSerializable
{
    ISerializer Serializer { get; }
}

public interface IFlatBufferSerializable<TTable> : IFlatBufferSerializable
{
   ISerializer<TTable> Serializer { get; }
}

[FlatBufferTable]
public partial class SomeTable : IFlatBufferSerializable<SomeTable>
{
    [FlatBufferItem(0)] public virtual int Int { get; set; }

    // already exists; can't break this one
    public static ISerializer<SomeTable> Serializer { get; } = ...;

    // explicit interface implementations avoid name conflicts.
    ISerializer<SomeTable> IFlatBufferSerializable<SomeTable>.Serializer  => SomeTable.Serializer;
    ISerializer IFlatBufferSerializable.Serializer => SomeTable.Serializer;
}

However, deserialize is weird because serializers are done statically in FlatSharp. There are already ISerializer and ISerializer<T> interfaces that operate on Tables, but there is not a single place that they are aggregated. When using FBS files, FlatSharp does not help you write this method:

public T Deserialize<T>(byte[] buffer) { }

Rather, FlatSharp expects you to know the type that you are intending to deserialize. If you have access to reflection, it's pretty easy to grab the Serializer property defined on the object and cache that internally using a Type -> ISerializer dictionary.

Do you have thoughts on the API surface to expose for deserialization? I'm hesitant to do the above because I worry it only solves half your problem, which doesn't really help.

My other thought is: if you have literally thousands of tables, it may make sense to code gen both the FBS file and the partial classes from the same template engine (or same data source, at least). Then you could codegen all of the stuff that you need with just a couple of steps.

// Serialize
var buffer = new SomeTable() 
{
   Int = 1
}.ToByteArray();

// Deserialize
var table2 = new SomeTable().ParseFrom(buffer);

(which come from Google.Protobuf design)

So we can build our custom serializer class like this:

public class FlatSharpSerializer : ICustomSerializer
{
    public byte[] Serialize<T>(T source) where T : IFlatBufferSerializable<T>, new()
    {
        return source.ToByteArray();
    }
        
    public T Deserialize<T>(byte[] source) where T : IFlatBufferSerializable<T>, new()
    {
        return new T().ParseFrom(source);
    }
}
public static class Serializer
{
    public static readonly ISerializer Binary = new BinarySerializer();
    public static readonly ISerializer Json = new JsonSerializer();
    public static readonly ISerializer Protobuf = new ProtobufSerializer();
    // public static readonly ISerializer FlatBuffer = new FlatBufferSerializer();  <== Failed to make this without interface yet.
}

@jamescourtney
Copy link
Owner

I think exposing the Serializer<T> through the interface will be sufficient to let you build what you want. You'll still need to do:

new T().Serializer.Parse(buffer);

But I suppose that is the best you can do without reflection, codegen, or static interface methods. I don't intend to add any additional methods to the interface; exposing the ISerializer<T> gives access to everything needed. Adding more methods just confuses the situation. You can add those if you want via extension methods.

The main thing I want to avoid adding to FlatSharp is some method like ParseFrom, because when I call x.ParseFrom(buffer), I have the expectation that the contents of buffer are going to be loaded into x, which is something that FlatSharp (by design) is unable to do. The best it can do is return you a new object.

I'll throw together a prototype of this tonight and give you links to private packages you can play with. Once that's done, we can see about releasing a new version.

@jamescourtney
Copy link
Owner

Preview build available here: https://ci.appveyor.com/project/jamescourtney/flatsharp/builds/38223116/artifacts

@shtse8
Copy link

shtse8 commented Mar 15, 2021

I think exposing the Serializer<T> through the interface will be sufficient to let you build what you want. You'll still need to do:

new T().Serializer.Parse(buffer);

But I suppose that is the best you can do without reflection, codegen, or static interface methods. I don't intend to add any additional methods to the interface; exposing the ISerializer<T> gives access to everything needed. Adding more methods just confuses the situation. You can add those if you want via extension methods.

The main thing I want to avoid adding to FlatSharp is some method like ParseFrom, because when I call x.ParseFrom(buffer), I have the expectation that the contents of buffer are going to be loaded into x, which is something that FlatSharp (by design) is unable to do. The best it can do is return you a new object.

I'll throw together a prototype of this tonight and give you links to private packages you can play with. Once that's done, we can see about releasing a new version.

Agreed. I don't like the name ParseFrom in FlatSharp. I would prefer Load is much better.

@shtse8
Copy link

shtse8 commented Mar 15, 2021

Preview build available here: https://ci.appveyor.com/project/jamescourtney/flatsharp/builds/38223116/artifacts

Cool! Let me try!!

@shtse8
Copy link

shtse8 commented Mar 15, 2021

One question, for my understand, FlatBuffer has no deserialization process on load and only deserialize data on getting values.

But I noticed this part of code:

private sealed class tableReader_6b2aa25fad8f447f99b8889d4870ef26<TInputBuffer>
	: my.name.space.SomeTable
	, IFlatBufferDeserializedObject
	where TInputBuffer : IInputBuffer
{
	private static readonly FlatBufferDeserializationContext __CtorContext
		= new FlatBufferDeserializationContext(FlatBufferDeserializationOption.GreedyMutable);

	private System.Int32 __index0Value;

	public tableReader_6b2aa25fad8f447f99b8889d4870ef26(TInputBuffer buffer, int offset) : base(__CtorContext)
	{
		checked
		{
			buffer.InitializeVTable(offset, out var __vtableLocation, out var __vtableMaxIndex);
			this.__index0Value = ReadIndex0Value(buffer, offset, __vtableLocation, __vtableMaxIndex);
		}
	}
...

Mainly this line.

this.__index0Value = ReadIndex0Value(buffer, offset, __vtableLocation, __vtableMaxIndex);

Serializer will read all values and set to the properties on deserialization. Will it break the theory of FlatSharp?

Should we deserialize on the getter and then cache the value?

Some roughly draft on my mind:

public class SomeTable : ISerializable<SomeTable>
{
  private byte[] _buffer;
  private int _int;

  public int Int {
    get => /* deserialize from buffer if buffer exists and then cache, else read from _int */ ;
    set => _int = value;
  }
  
  public byte[] GetByteArray();

  public void Load(byte[] buffer) => _buffer = buffer;
}

@jamescourtney
Copy link
Owner

What you describe already exists.

FlatSharp is greedy by default. Greedy is the simplest mode. If you want Flatsharp to be lazy, all you have to do is tell it.

https://github.com/jamescourtney/FlatSharp/wiki/Deserialization-Modes
https://github.com/jamescourtney/FlatSharp/wiki/FBS-Annotations-(FlatSharp-5.X)#fs_serializer

@shtse8
Copy link

shtse8 commented Mar 15, 2021

What you describe already exists.

FlatSharp is greedy by default. Greedy is the simplest mode. If you want Flatsharp to be lazy, all you have to do is tell it.

https://github.com/jamescourtney/FlatSharp/wiki/Deserialization-Modes
https://github.com/jamescourtney/FlatSharp/wiki/FBS-Annotations-(FlatSharp-5.X)#fs_serializer

I appreciate your help so much. It's very amazing! But why the aggressive mode is the default but not lazy? I think lazy deserialization is the core concept of FlatBuffer.

@shtse8
Copy link

shtse8 commented Mar 15, 2021

public class FlatBufferSerializer : ISerializer
{
    public Stream Serializer<T>(T source) where T : class, IFlatBufferSerializable<T>, new()
    {
        var serializer = source.Serializer;
        var size = serializer.GetMaxSize(source);
        var buffer = new byte[size];
        var written = serializer.Write(buffer, source);
        return new MemoryStream(buffer, 0, written);
    }

    public T Deserialize<T>(Stream stream) where T : class, IFlatBufferSerializable<T>, new()
    {
        var instance = new T();
        var serializer = instance.Serializer;
        var anotherInstance = serializer.Parse(stream.GetBytes());
        return anotherInstance;
    }
}

I am putting it into my project serializer wrapper for convenience. but there is a strange place in the coding in deserialization, Parse() returns T while I am calling Parse from a T. Is it possible to parse the data to the existing without returning a new instance?

Also, I am thinking if there is another way to serialize. Currently, we need to get the max size and then pass to the Write(), which will return written size, so we can get the length we want. However, before serializing, we need to allocate a larger buffer. what if the data table is super large but just one of the field is being used? Is it possible to make the writer to allocate memory if needed? so we don't need to pass a larger buffer into it.

@jamescourtney
Copy link
Owner

jamescourtney commented Mar 15, 2021

But why the aggressive mode is the default but not lazy? I think lazy deserialization is the core concept of FlatBuffer.

Lazy is a key concept! However, that's different than pretty much every other .NET serializer, which does things greedily. The point of confusion people may have with lazy is this:

byte[] buffer = ByteArrayPool.Take();
await File.Read("foo.txt", buffer);
T item = someSerializer.Parse(buffer); // item implicitly references buffer, but this is not obvious to the programmer
ByteArrayPool.Return(buffer);

So, in this case, the item would become invalid as soon as that buffer was reused to read a different file. This wouldn't be obvious at all to the programmer and would likely take awhile to track down, which is why I chose to make Greedy the default; it might not be the best performing option, but it does behave in a way that people are accustomed to.

Is it possible to parse the data to the existing without returning a new instance?

Not with FlatSharp, sorry. The reason for this is that when you deserialize a T, you get back a subclass of T that has properties overridden. This is what I was talking about above when I said I didn't like ParseFrom. If you have thoughts about how to work around this, please let me know.

Is it possible to make the writer to allocate memory if needed? so we don't need to pass a larger buffer into it.

Ideally, you pool your buffers so that you only occasionally need to expand them and allocate something new, right? It's also not required to call GetMaxSize. You can instead just try to serialize. If it runs out of space, FlatSharp will throw a BufferTooSmallException that tells you what size it thinks it needs. Expanding buffers isn't something FlatSharp can do, because it just sees a Span<byte> that it is writing into, which is not a resizable type.

@shtse8
Copy link

shtse8 commented Mar 16, 2021

Thanks for your detailed explanation. I understand this project more now. I will suggest a better approach if I have any good idea on that!

I have tried v5.1 preview, everything works fine in Mono. but when I build it using il2cpp. the follow exception was thrown on deserialization.

System.ExecutionEngineException: Attempting to call method 'Unity.Core.Asset.Models.Data.AssetBundlesData+GeneratedSerializer::Parse<FlatSharp.ReadOnlyMemoryInputBuffer+Wrapper>' for which no ahead of time (AOT) code was generated.
FlatSharp.ReadOnlyMemoryInputBuffer.InvokeParse(IGeneratedSerializer`1,Int32)</color>
Core.Json.FlatBufferSerializer.Deserialize(Memory`1)

Any idea of this?

========================================================

After investigation, it should because of no virtual method in AOT.
Reference: https://docs.unity3d.com/Manual/ScriptingRestrictions.html

We should add a dummy method to GeneratedSerializer to ensure the Parse<T> is generated in AOT.

public void UsedOnlyForAOTCodeGeneration() 
{
    // IL2CPP needs only this line.
    OnMessage(AnyEnum.Zero);

    // Mono also needs this line. Note that we are
    // calling directly on the Manager, not the IManager interface.
    new Manager().SendMessage(null, AnyEnum.Zero);

    // Include an exception so we can be sure to know if this method is ever called.
    throw new InvalidOperationException("This method is used for AOT code generation only. Do not call it at runtime.");
}

=======================================================

What I guess the exception is come from:

 public T Parse(IInputBuffer buffer)
    {
      if (buffer.Length >= 1073741823)
        throw new ArgumentOutOfRangeException("Buffer must be <= 1GB in size.");
      IInputBuffer inputBuffer = buffer.Length > 8 ? buffer : throw new ArgumentException("Buffer is too small to be valid!");
      SerializerSettings settings = this.settings;
      ISharedStringReader sharedStringReader;
      if (settings == null)
      {
        sharedStringReader = (ISharedStringReader) null;
      }
      else
      {
        Func<ISharedStringReader> stringReaderFactory = settings.SharedStringReaderFactory;
        sharedStringReader = stringReaderFactory != null ? stringReaderFactory() : (ISharedStringReader) null;
      }
      inputBuffer.SharedStringReader = sharedStringReader;
      return buffer.InvokeParse<T>(this.innerSerializer, 0);
    }
return buffer.InvokeParse<T>(this.innerSerializer, 0);

which refers to

public TItem InvokeParse<TItem>(IGeneratedSerializer<TItem> serializer, int offset) => serializer.Parse<ArrayInputBuffer.Wrapper>(new ArrayInputBuffer.Wrapper(this), offset);

the generic method isn't generated in AOT, so we need to make some tricks here for AOT compilation.

@jamescourtney
Copy link
Owner

It looks like this is the problem that you're referring to: https://docs.unity3d.com/Manual/ScriptingRestrictions.html

Do you have any suggestions on the best way to fix this? It looks like the problem is that IL2CPP doesn't know for sure that it needs to generate a method for ArrayInputBuffer.Wrapper, since the method call is only in terms of an interface.

What if I made this change:

OLD:

        public TItem InvokeParse<TItem>(IGeneratedSerializer<TItem> serializer, int offset)
        {
            return serializer.Parse(new Wrapper(this), offset);
        }

NEW:

        public TItem InvokeParse<TSerializer, TItem>(TSerializer serializer, int offset)
              where TSerializer : IGeneratedSerializer<TItem>
        {
            return serializer.Parse(new Wrapper(this), offset);
        }

Since TSerializer will be a concrete type, IL2CPP should see that .Parse is required, right?

@shwuhk
Copy link
Author

shwuhk commented Mar 17, 2021

It looks like the problem is that IL2CPP doesn't know for sure that it needs to generate a method for ArrayInputBuffer.Wrapper, since the method call is only in terms of an interface.

I think this is the cause of the problem. As suggested by unity, the proposed fix is to invoke it in somewhere so that the compiler will generate the method it needed.

Not sure if your proposed fix can solve it but I think it worth a try. You are really an expert in it! Thanks for your help.
BTW, may be we could try it using mono --aot?
https://www.mono-project.com/docs/advanced/aot/

@jamescourtney
Copy link
Owner

Thanks for the tip about Mono AOT. I can reproduce this issue with Mono. Interestingly, it seems to only happen for the Serialize cases. I don't see the issue with Parse in Mono, for whatever reason.

@shwuhk
Copy link
Author

shwuhk commented Mar 17, 2021

Thanks for the tip about Mono AOT. I can reproduce this issue with Mono. Interestingly, it seems to only happen for the Serialize cases. I don't see the issue with Parse in Mono, for whatever reason.

That's great if it can reproduce the issue. Do you have any idea about the cause of it?
Error happened in deserialize too in our side.

@jamescourtney
Copy link
Owner

I tried a bunch of tricks with generics and none of them got me anywhere. So, I'm leaning towards injecting this into the generated code:

    public void AotHelper()
    {
        this.Write(SpanWriter.Instance, new byte[10], null, 0, null);
        this.Parse<MemoryInputBuffer>(default, 0);
        this.Parse<ReadOnlyMemoryInputBuffer>(default, 0);
        this.Parse<ArrayInputBuffer>(default, 0);
#if FLATSHARP_UNSAFE
        this.Parse<FlatSharp.Unsafe.UnsafeArrayInputBuffer>(default, 0);
        this.Parse<FlatSharp.Unsafe.UnsafeMemoryInputBuffer>(default, 0);
        this.Write(new FlatSharp.Unsafe.UnsafeSpanWriter(), new byte[10], null, 0, null);
#endif
    }

There are pros and cons here.

  • Pro: Easy to implement, easy to consume. Just requires a special preprocessor directive if you want to enable the unsafe library, which shouldn't be too hard for Mono / Unity, right?
  • Con: Only works for already-known types of IInputBuffer/ISpanWriter. This limits the extensibility in cases where someone might want to supply their own implementation, for whatever reason. Of course, the existing implementations will work for anything that wraps contiguous memory (array, unmanaged memory, mmap files, etc), but wouldn't work if someone wanted to build, say, a StreamInputBuffer.

I really wish that FBS files had the concept of global options, so you could declare once:

option.fs_RegisterInputBufferType = "Your.Namespace.Buffer";

But that doesn't exist right now

@shwuhk
Copy link
Author

shwuhk commented Mar 17, 2021

I tried a bunch of tricks with generics and none of them got me anywhere.

That is easy to implement and at least a quick fix for the situation now. We would try if that work in Unity. After all, it is just AOT. Does it work in Mono AOT?

I think this should be mentioned in the ReadMe section about this behaviour and/or expose a aot helper function interface for those who want to implement their own type.

@shtse8
Copy link

shtse8 commented Mar 17, 2021

I tried a bunch of tricks with generics and none of them got me anywhere. So, I'm leaning towards injecting this into the generated code:

    public void AotHelper()
    {
        this.Write(SpanWriter.Instance, new byte[10], null, 0, null);
        this.Parse<MemoryInputBuffer>(default, 0);
        this.Parse<ReadOnlyMemoryInputBuffer>(default, 0);
        this.Parse<ArrayInputBuffer>(default, 0);
#if FLATSHARP_UNSAFE
        this.Parse<FlatSharp.Unsafe.UnsafeArrayInputBuffer>(default, 0);
        this.Parse<FlatSharp.Unsafe.UnsafeMemoryInputBuffer>(default, 0);
        this.Write(new FlatSharp.Unsafe.UnsafeSpanWriter(), new byte[10], null, 0, null);
#endif
    }

There are pros and cons here.

  • Pro: Easy to implement, easy to consume. Just requires a special preprocessor directive if you want to enable the unsafe library, which shouldn't be too hard for Mono / Unity, right?
  • Con: Only works for already-known types of IInputBuffer/ISpanWriter. This limits the extensibility in cases where someone might want to supply their own implementation, for whatever reason. Of course, the existing implementations will work for anything that wraps contiguous memory (array, unmanaged memory, mmap files, etc), but wouldn't work if someone wanted to build, say, a StreamInputBuffer.

I really wish that FBS files had the concept of global options, so you could declare once:

option.fs_RegisterInputBufferType = "Your.Namespace.Buffer";

But that doesn't exist right now

Agreed! btw, at least it should work on AOT now although it only works for already-known types. I tried many ways without hitting the generated files in version 5.1 and none of them worked. It seem adding a dummy method to the generated files is the simplest way to solve.

it's better to throw an exception so that we can make sure this dummy method can't be invoked.

@jamescourtney
Copy link
Owner

New preview building here: https://ci.appveyor.com/project/jamescourtney/flatsharp/builds/38266904/artifacts. Let me know how it works for you.

@shtse8
Copy link

shtse8 commented Mar 17, 2021

New preview building here: https://ci.appveyor.com/project/jamescourtney/flatsharp/builds/38266904/artifacts. Let me know how it works for you.

Tried! It works like a charm! Thanks for your hard work! We will double check and see if still any issue around.

Really want to buy you a coffee!

@jamescourtney
Copy link
Owner

Thanks for the offer; that's very kind! I'm honestly just really happy that people find the project useful.

I have one final preview before I'm ready to release version 5.1 on Nuget, if you wouldn't mind trying it: https://ci.appveyor.com/project/jamescourtney/flatsharp/builds/38279284/artifacts

Thanks again!

@shwuhk
Copy link
Author

shwuhk commented Mar 18, 2021

You are so great! The World will be better because of you!
Let us feedback to you after trying that!

@shwuhk
Copy link
Author

shwuhk commented Mar 18, 2021

Just tried! It works really great!
Would you mind publish it as an official version?

@jamescourtney
Copy link
Owner

This is published now. Thanks for all of your help and suggestions! I'm going to close out this issue, but feel free to reach out if you need anything else!

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

No branches or pull requests

3 participants