Unity consumes messagepack as a nuget package instead of source

[neuecc]

@AArnott @pCYSl5EDgo

This is a request for a significant change in the project structure.
Currently, all the source code placed on the Unity side is to be moved to the .NET side, with Unity referencing a dll.
This change will make it easier to write as a .NET library, and also allows us to adopt C# 12.
There is no need to be conscious of Unity in regular code writing.

On the Unity side, it will reference a NuGet library built as a DLL as its core,
and use a plugin method to separately reference extensions for Unity through Unity UPM.

MessagePack for C# is widely referenced as a foundational library, but
since there is no compatibility between the Unity version and the NuGet version of the library,
NuGet libraries that depend on MessagePack for C# in Unity have become unreferenceable.
Unity is now different from the past, being .NET Standard 2.1,
and excellent package managers like NuGetForUnity have also emerged.

I recently released a new library called R3, and there, I tried the same structure as this PR.
https://github.com/Cysharp/R3/#unity

I believe adopting this PR is important for the efficiency of future development and the progress in Unity.

However, since this would break compatibility with references on the Unity side,
how about timing the release to coincide with the Source Generatorization when mpc is no longer needed?

[AArnott]

That all sounds awesome. I'll review today or this weekend. I expect it'll cause massive merge conflicts with my work on #1691, so I'll suspend that work for now to not compound the problem.

[AArnott]

Should we bump the major version number given that it'll be a breaking change for unity folks, as well as users of the analyzer?

[AArnott]

How does this impact any hesitancy to take on other nuget dependencies? In the past, I think dependencies on other assemblies were taken only with huge justification because it complicated consumption for unity users.

[pCYSl5EDgo]

Should we bump the major version number given that it'll be a breaking change for unity folks, as well as users of the analyzer?

Major version 3 is appropriate because the mpc deletion is big breaking change.

Additional Info

Unity 2021.3 is using Roslyn v3 and it dies in 2024 April.
C#11 is available since Roslyn v4.4 (Visual Studio v17.4).

[neuecc]

In the past, I think dependencies on other assemblies were taken only with huge justification because it complicated consumption for unity users.

Disable Assembly Version Validation solves it.

II also think 3 is a good choice. Converting to Source Generator would be a significant change for all users.

---

This draft is rough and there are several tasks remaining.

• Need to remove code like if UNITY or IF IL2CPP from the Core
• Especially for Unity Android, there was code tailored for binary Write/Read, and we need to decide what to do with that
• There was an issue with sharing test code because signed InternalVisibleTo couldn't be referenced on the Unity side. (It was only for Sequence, so I temporarily pasted the Sequence itself to remove the compile error.)

---

Since NuGetForUnity also automatically reflects Source Generators, creating SourceGenerators for both 3.x and 4.x and including the dlls should automatically segregate them, which would be good. Let's not create a SourceGenerator package specifically for Unity.

By the way, Unity supports Incremental Generators depending on the version, and currently, it seems that versions 4.1.0(Unity 2022.2) and 4.3.0(latest) are available.

[AArnott]

All this sounds exciting. Do you mind holding off on this substantial refactoring until my source generator/analyzer work to switch to attributes is done?

[pCYSl5EDgo]

Dropping netstandard2.0 and update to netstandard2.1 in MessagePack csprojs (except Source Generator) will be permitted?
Or keep it because SignalR depends on netstandard2.0?

[AArnott]

No, we can't drop netstandard2.0. I believe what neuecc is saying is because unity supports netstandard2.1, we don't need a special source-based build for it any more.

[neuecc]

Do you mind holding off on this substantial refactoring until my source generator/analyzer work to switch to attributes is done?

OK.

[pCYSl5EDgo]

Especially for Unity Android, there was code tailored for binary Write/Read, and we need to decide what to do with that

I've installed Unity2022.3.18 and built IL2CPP in order to assess the cost of runtime Android armv7 detection.
All of the latter C++ codes are generated with windows x64 master build settings.

Results

• BitConverter.IsLittleEndian is compiled to be an inlined function returning constant boolean.
• sizeof(nint) == 4 is compiled to a constant-like expression.
• RuntimeInformation.ProcessArchitecture == Architecture.Arm is compiled to the code which accesses static field.

Considering the compile-time optimization of C++, the combination of BitConverter.IsLittleEndian and sizeof(nint) == 4 is the best method. Even if this method cannot distinguish x86-windows and armv7-android, it has the best runtime efficiency.

Methods

Test C# code

using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using UnityEngine;

public unsafe class TestForConst : MonoBehaviour
{
    void Update()
    {
        Vector3 velocity;
        if (BitConverter.IsLittleEndian)
        {
            velocity = new(0, 1, 0);
        }
        else if (sizeof(nint) == 4)
        {
            velocity = new(0, -1, 0);
        }
        else
        {
            velocity = new();
        }

        velocity += GenericVector<int>.Get();
        
        transform.position += velocity * Time.deltaTime;
    }
}

internal struct GenericVector<T>
{
    public static Vector3 Get()
    {
        if (typeof(T) == typeof(int))
        {
            if (RuntimeInformation.ProcessArchitecture == Architecture.Arm)
            {
                return new(1, 0, 1);
            }

            return new(1, 0, 0);
        }

        return new(-1, 0, 0);
    }
}

Generated C++ code for Update method

IL2CPP_EXTERN_C IL2CPP_METHOD_ATTR void TestForConst_Update_mAECC56A194C2E9D162EAFFB3B9F23B0B4EBCC9F7 (TestForConst_tDA600D04A981E1B53CD873D2147332DB5AA8480E* __this, const RuntimeMethod* method) 
{
	static bool s_Il2CppMethodInitialized;
	if (!s_Il2CppMethodInitialized)
	{
		il2cpp_codegen_initialize_runtime_metadata((uintptr_t*)&GenericVector_1_Get_m49BFA61A2FA9A6589F796D73B626ED5C0562B4A7_RuntimeMethod_var);
		s_Il2CppMethodInitialized = true;
	}
	Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 V_0;
	memset((&V_0), 0, sizeof(V_0));
	{
		if (!il2cpp_codegen_is_little_endian())
		{
			goto IL_001f;
		}
	}
	{
		Vector3__ctor_m376936E6B999EF1ECBE57D990A386303E2283DE0_inline((&V_0), (0.0f), (1.0f), (0.0f), NULL);
		goto IL_0048;
	}

IL_001f:
	{
		uint32_t L_0 = sizeof(intptr_t);
		if ((!(((uint32_t)L_0) == ((uint32_t)4))))
		{
			goto IL_0040;
		}
	}
	{
		Vector3__ctor_m376936E6B999EF1ECBE57D990A386303E2283DE0_inline((&V_0), (0.0f), (-1.0f), (0.0f), NULL);
		goto IL_0048;
	}

IL_0040:
	{
		il2cpp_codegen_initobj((&V_0), sizeof(Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2));
	}

IL_0048:
	{
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_1 = V_0;
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_2;
		L_2 = GenericVector_1_Get_m49BFA61A2FA9A6589F796D73B626ED5C0562B4A7(GenericVector_1_Get_m49BFA61A2FA9A6589F796D73B626ED5C0562B4A7_RuntimeMethod_var);
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_3;
		L_3 = Vector3_op_Addition_m78C0EC70CB66E8DCAC225743D82B268DAEE92067_inline(L_1, L_2, NULL);
		V_0 = L_3;
		Transform_tB27202C6F4E36D225EE28A13E4D662BF99785DB1* L_4;
		L_4 = Component_get_transform_m2919A1D81931E6932C7F06D4C2F0AB8DDA9A5371(__this, NULL);
		Transform_tB27202C6F4E36D225EE28A13E4D662BF99785DB1* L_5 = L_4;
		NullCheck(L_5);
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_6;
		L_6 = Transform_get_position_m69CD5FA214FDAE7BB701552943674846C220FDE1(L_5, NULL);
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_7 = V_0;
		float L_8;
		L_8 = Time_get_deltaTime_mC3195000401F0FD167DD2F948FD2BC58330D0865(NULL);
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_9;
		L_9 = Vector3_op_Multiply_m87BA7C578F96C8E49BB07088DAAC4649F83B0353_inline(L_7, L_8, NULL);
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_10;
		L_10 = Vector3_op_Addition_m78C0EC70CB66E8DCAC225743D82B268DAEE92067_inline(L_6, L_9, NULL);
		NullCheck(L_5);
		Transform_set_position_mA1A817124BB41B685043DED2A9BA48CDF37C4156(L_5, L_10, NULL);
		return;
	}
}

il2cpp_codegen_is_little_endian is well optimized.

inline bool il2cpp_codegen_is_little_endian()
{
#if IL2CPP_BYTE_ORDER == IL2CPP_LITTLE_ENDIAN
    return true;
#else
    return false;
#endif
}

`typeof(T) == typeof(int)` is bad.

IL2CPP_EXTERN_C IL2CPP_METHOD_ATTR Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 GenericVector_1_Get_m49BFA61A2FA9A6589F796D73B626ED5C0562B4A7_gshared (const RuntimeMethod* method) 
{
	static bool s_Il2CppMethodInitialized;
	if (!s_Il2CppMethodInitialized)
	{
		il2cpp_codegen_initialize_runtime_metadata((uintptr_t*)&Int32_t680FF22E76F6EFAD4375103CBBFFA0421349384C_0_0_0_var);
		il2cpp_codegen_initialize_runtime_metadata((uintptr_t*)&RuntimeInformation_tB2DFA85FB9251AE3A3112904C9DF06C30D1D3EAF_il2cpp_TypeInfo_var);
		il2cpp_codegen_initialize_runtime_metadata((uintptr_t*)&Type_t_il2cpp_TypeInfo_var);
		s_Il2CppMethodInitialized = true;
	}
	{
		RuntimeTypeHandle_t332A452B8B6179E4469B69525D0FE82A88030F7B L_0 = { reinterpret_cast<intptr_t> (il2cpp_rgctx_type(InitializedTypeInfo(method->klass)->rgctx_data, 0)) };
		il2cpp_codegen_runtime_class_init_inline(Type_t_il2cpp_TypeInfo_var);
		Type_t* L_1;
		L_1 = Type_GetTypeFromHandle_m6062B81682F79A4D6DF2640692EE6D9987858C57(L_0, NULL);
		RuntimeTypeHandle_t332A452B8B6179E4469B69525D0FE82A88030F7B L_2 = { reinterpret_cast<intptr_t> (Int32_t680FF22E76F6EFAD4375103CBBFFA0421349384C_0_0_0_var) };
		Type_t* L_3;
		L_3 = Type_GetTypeFromHandle_m6062B81682F79A4D6DF2640692EE6D9987858C57(L_2, NULL);
		bool L_4;
		L_4 = Type_op_Equality_m99930A0E44E420A685FABA60E60BA1CC5FA0EBDC(L_1, L_3, NULL);
		if (!L_4)
		{
			goto IL_004d;
		}
	}
	{
		il2cpp_codegen_runtime_class_init_inline(RuntimeInformation_tB2DFA85FB9251AE3A3112904C9DF06C30D1D3EAF_il2cpp_TypeInfo_var);
		int32_t L_5;
		L_5 = RuntimeInformation_get_ProcessArchitecture_mB2DAF77FAF4F8F97AE4045FA7DD140D60D8BF3F3_inline(NULL);
		if ((!(((uint32_t)L_5) == ((uint32_t)2))))
		{
			goto IL_0038;
		}
	}
	{
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_6;
		memset((&L_6), 0, sizeof(L_6));
		Vector3__ctor_m376936E6B999EF1ECBE57D990A386303E2283DE0_inline((&L_6), (1.0f), (0.0f), (1.0f), NULL);
		return L_6;
	}

IL_0038:
	{
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_7;
		memset((&L_7), 0, sizeof(L_7));
		Vector3__ctor_m376936E6B999EF1ECBE57D990A386303E2283DE0_inline((&L_7), (1.0f), (0.0f), (0.0f), NULL);
		return L_7;
	}

IL_004d:
	{
		Vector3_t24C512C7B96BBABAD472002D0BA2BDA40A5A80B2 L_8;
		memset((&L_8), 0, sizeof(L_8));
		Vector3__ctor_m376936E6B999EF1ECBE57D990A386303E2283DE0_inline((&L_8), (-1.0f), (0.0f), (0.0f), NULL);
		return L_8;
	}
}

[pCYSl5EDgo]

I've tested Unsafe.ReadUnaligned and found that Unity2022.3 treats Unsafe.ReadUnaligned as special.
It seems that we no longer need to take care of Android armv7 problem?

template<typename T>
inline T il2cpp_unsafe_read_unaligned(void* location)
{
    T result;
#if IL2CPP_TARGET_ARMV7 || IL2CPP_TARGET_JAVASCRIPT
    memcpy(&result, location, sizeof(T));
#else
    result = *((T*)location);
#endif
    return result;
}

template<typename T>
inline void il2cpp_unsafe_write_unaligned(void* location, T value)
{
#if IL2CPP_TARGET_ARMV7 || IL2CPP_TARGET_JAVASCRIPT
    memcpy(location, &value, sizeof(T));
#else
    *((T*)location) = value;
#endif
}

template<typename T, typename TOffset>
inline T* il2cpp_unsafe_add(void* source, TOffset offset)
{
    return reinterpret_cast<T*>(source) + offset;
}

template<typename T, typename TOffset>
inline T* il2cpp_unsafe_add_byte_offset(void* source, TOffset offset)
{
    return reinterpret_cast<T*>(reinterpret_cast<uint8_t*>(source) + offset);
}

By the way, Unity does not generate intrinsics for MemoryMarshal.GetReference and Unsafe.AddByteOffset unfortunately.
There seems to be il2cpp_unsafe_add_byte_offset but not used even if Unsafe.AddByteOffset is used!
In contrast, il2cpp_unsafe_add is used when Unsafe.Add is used.
Very surprising.

[AArnott]

I think it's time to focus on this PR. Since I introduced all the merge conflicts, I'll try to find time to freshen it up.

[AArnott]

Ok, it's freshened up, ready for someone with more Unity experience to patch up the rest. :)

[neuecc]

@AArnott

I have completed all the changes, and I think it's now safe to merge.

• Set up Unity IL2CPP build on Linux CI and run the built unit tests
  • This is a good step forward as we didn't have IL2CPP CI running before!
• Modified the Unity section in the ReadMe
  • 9845d3f
  • There are also some changes related to the Source Generator text, so please revise if further modifications are needed

By the way, is it possible to generate Source Generator externally in a library that only references the Annotations package?
It seems like it might not be possible based on the current setup, but what do you think?

I'm not sure if this is possible, but how about something like the following specifications to allow specifying external types?

// Search for [MessagePackObject] types from the assembly containing Foo
[GeneratedMessagePackResolver(Assembly: typeof(Foo))]
public partial class GeneratedMessagePackResolver
{
}

// Specify types to generate (Creating with Map might allow non-[MessagePackObject] types?)
[GeneratedMessagePackResolver(Types: [typeof(Foo), typeof(Bar)])]
public partial class GeneratedMessagePackResolver
{
}

[AArnott]

I have completed all the changes, and I think it's now safe to merge.

Super cool. I'll review today.

is it possible to generate Source Generator externally in a library that only references the Annotations package?
It seems like it might not be possible based on the current setup, but what do you think?

The source generator is already in a separate package. That package has no dependencies at all (not even annotations).
But it's meaningless for a user to reference just that package, because without annotations you can't use attributes and thus there is nothing to analyze or generate.
And if you do reference the annotations library as well, while the analyzer part would theoretically be worthwhile, the source generator is pointless because the generated source will need the full library since the code implements and references all sorts of APIs from the main library.
Since the source generator is 'on by default' in v3 for all attributes it finds, this makes referencing the analyzer/source generator package alone completely pointless.

For that matter, since MessagePack depends on MessagePackAnalyzer in order for the 'on by default' behavior, and MessagePackAnalyzer by itself is pointless, that actually begs the question as to why we don't just merge the content of MessagePackAnalyzer into the MessagePack package and call it one. That's outside the scope of this PR, but it's something we should settle one way or the other before v3 goes stable.

// Search for [MessagePackObject] types from the assembly containing Foo
[GeneratedMessagePackResolver(Assembly: typeof(Foo))]

I guess the idea there is to generate AOT formatters for data types in an assembly that wasn't compiled against v3, right? Sounds useful. If at runtime the other assembly does provide its own formatters (because at runtime a newer dll is provided) I wonder which formatters should be used. I can imagine arguments for either way. Do you want to open an issue in the v3 milestone to track the discussion?

// Specify types to generate (Creating with Map might allow non-[MessagePackObject] types?)
[GeneratedMessagePackResolver(Types: [typeof(Foo), typeof(Bar)])]

I really don't want to expand on support for contractless scenarios (which is what it sounds like you're talking about) until we have a really thorough design for it. Contractless today has a poor success rate in my experience (and based on the issues that keep coming in), and if we're going to have AllowPrivate behavior by default (which I'm very excited about), ContractlessAllowPrivate is even worse, perhaps, unless we think through it carefully. Let's consider that in a new issue. And it sounds like something we could add in v3.1 as it wouldn't be a breaking change.

[AArnott]

I see you removed prepare_release.ps1. If that means we no longer have a version number in some file to keep in sync with version.json for the sake of the unity package, that's great.

[AArnott]

Looks very nearly ready. Very exciting!

[neuecc]

Oh, we do need prepare_release.ps1. I deleted too much.
We still need to rewrite the version here to synchronize it with the release!
https://github.com/MessagePack-CSharp/MessagePack-CSharp/blob/new-structure/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/package.json#L4

[neuecc]

I think it's good that MessagePackAnalyzer is being merged into MessagePack.

Until now, a common practice for projects defining container types was
to have fewer dependencies (for example, using DataContract or referencing only the separated Annotations).
I believe there were quite a few people who structured their projects this way.
It's possible that those who had separated out the Annotations might raise complaints or questions about Source Generator support.

[AArnott]

It's possible that those who had separated out the Annotations might raise complaints or questions about Source Generator support.

Those who want the attributes and nothing else can continue to get that... but they will not be able to benefit from all the AOT work -- just like they couldn't with v2 of the library, since fundamentally the AOT formatters need to reference the main library.