Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.12k stars 430 forks source link

Rpc params pooling #1598

Open MrCool92 opened 2 years ago

MrCool92 commented 2 years ago

I need to send complex data that contain complex data (lists for example). So my plan was to introduce pooling. But with the current RPC send/receive netcode lifecycle I am not sure how would I return pool instances.

My approach:

private void SendClientRpcExample()
{
    ComplexData complexData = Pool<ComplexData>.Get();
    // set complexData values
    // ...
    ExampleClientRpc(complexData);
    complexData.Dispose();
}

[ClientRpc]
private void ExampleClientRpc(ComplexData complexData, ...)
{
    try
    {
         // ExampleClientRpc code...
    }
    catch (System.Exception) { }
    complexData.Dispose();
}

public static partial class SerializationExtensions
{
    public static void ReadValueSafe(this FastBufferReader reader, out ComplexData value)
    {
        value = Pool<ComplexData>.Get();
        // read complexData values
        // ...
    }

    public static void WriteValueSafe(this FastBufferWriter writer, in ComplexData value)
    {
       // write complexData values
       // ...
    }
}

struct ComplexData : IDisposable
{
    public List<int> ints;
    public List<AnotherComplexData> anotherComplexDatas;

    public void Dispose()
    {
        Pool<List<int>>.Return(ref ints);
        foreach(var a in anotherComplexDatas)
            a.Dispose();
        Pool<List<AnotherComplexData>>.Return(ref anotherComplexDatas);
    }
}

struct AnotherComplexData : IDisposable
{
    public List<int> ints;

    public void Dispose()
    {
        Pool<List<int>>.Return(ref ints);
    }
}

This kinda feels and looks very messy. So I managed to get ILPP gen code that will call Dispose on any struct/class with IDisposable interface. And I think it is missing try-catch block so that Dispose() is called if an error occurs.

// NetworkBehaviourILPP.cs after NetworkBehaviour.XXXRpc(...)

// param.Dispose();
Enumerable.Range(0, paramCount).ToList().ForEach(paramIndex =>
{
    var pt = methodDefinition.Parameters[paramIndex].ParameterType;
    var td = pt.Resolve();
    var type = Type.GetType(pt.FullName + ", " + td.Module.Assembly.FullName);
    if (typeof(IDisposable).IsAssignableFrom(type))
    {
        var method = td.Methods.First(m => m.FullName.Contains("Dispose"));
        if (td.IsValueType)
        {
            processor.Emit(OpCodes.Ldloca, paramLocalMap[paramIndex]);
            processor.Emit(OpCodes.Call, method);
        }
        else
        {
            processor.Emit(OpCodes.Ldloc, paramLocalMap[paramIndex]);
            processor.Emit(OpCodes.Callvirt, method);
        }
    }
});

Are there any plans for poolable RPC params, or are there any patterns with the current API? Something like PooledProtocolToken from Photon.BOLT would ease the pain.

edit: IDisposable is just an example. It should be replaced with something custom internal (IPoolableParam with Return method).

LukeStampfli commented 2 years ago

I think this can be achieved by using an extension function for your custom type and getting/returning values from the pool there. https://docs-multiplayer.unity3d.com/docs/advanced-topics/custom-serialization

MrCool92 commented 2 years ago

I am using serialization extensions. Thing is.. once it is fetched from the pool it must be released somewhere... I am not sure where. NetworkBehaviourILPP makes the most sense so I don't have to manually pool-release Rpc params.

LukeStampfli commented 2 years ago

Ah right, I have to apologize I only skimmed over the issue and didn't fully read it 😬. From what I can tell everything you described is accurate and your solution of editing the ILPP is the only way to do this without having to manually dispose at the end of each RPC.

I don't think we have thought yet about adding support for auto release of IDisposable (or a custom interface) but it could be a feature we want to add. Not sure though whether the added complexity is justified compared to just manually disposing at the end of the RPC function. I'll convert this to a feature request so that we can evaluate it in the future.