Skip to content

Client Connection Ids and Game Object registration #863

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 3, 2021
Merged

Client Connection Ids and Game Object registration #863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7673dec
Client Connection Ids and Game Object registration
May 27, 2021
b71d6ab
Fixing connection to use local and avoid allocation at every new metrics
May 27, 2021
2752384
Fixed client id connections to point to other clients
Jun 3, 2021
3df98a0
Using IReadOnlyCollection instead of explicit List
Jun 3, 2021
71051d0
Merge branch 'experimental/netstats-dispatcher' into experimental/net…
Jun 3, 2021
8d30466
Changing variable name to sender and receiver
Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions com.unity.multiplayer.mlapi/Runtime/Messaging/CustomMessageManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, Stream stream)
if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32))
{
messageHandler32(sender, stream);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(m_MessageHandlerNameLookup32[hash], bytesCount);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount);
}

if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64))
{
messageHandler64(sender, stream);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(m_MessageHandlerNameLookup64[hash], bytesCount);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount);
}
}
else
Expand All @@ -114,14 +114,14 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, Stream stream)
if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32))
{
messageHandler32(sender, stream);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(m_MessageHandlerNameLookup32[hash], bytesCount);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount);
}
break;
case HashSize.VarIntEightBytes:
if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64))
{
messageHandler64(sender, stream);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(m_MessageHandlerNameLookup64[hash], bytesCount);
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount);
}
break;
}
Expand Down Expand Up @@ -190,8 +190,8 @@ public void SendNamedMessage(string name, ulong clientId, Stream stream, Network

m_NetworkManager.MessageSender.Send(clientId, NetworkConstants.NAMED_MESSAGE, networkChannel, messageBuffer);
PerformanceDataManager.Increment(ProfilerConstants.NamedMessageSent);
m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(name, (ulong)messageBuffer.Length);

m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(clientId, name, (ulong)messageBuffer.Length);
}
}

Expand Down Expand Up @@ -235,7 +235,7 @@ public void SendNamedMessage(string name, List<ulong> clientIds, Stream stream,
m_NetworkManager.MessageSender.Send(NetworkConstants.NAMED_MESSAGE, networkChannel, clientIds, messageBuffer);
PerformanceDataManager.Increment(ProfilerConstants.NamedMessageSent);

m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(name, (ulong)messageBuffer.Length);
m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(clientIds, name, (ulong)messageBuffer.Length);
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public void HandleConnectionApproved(ulong clientId, Stream stream, float receiv

NetworkManager.ConnectedClients.Add(NetworkManager.LocalClientId, new NetworkClient { ClientId = NetworkManager.LocalClientId });


void DelayedSpawnAction(Stream continuationStream)
{
using (var continuationReader = PooledNetworkReader.Get(continuationStream))
Expand Down Expand Up @@ -408,7 +407,7 @@ public void HandleNamedMessage(ulong clientId, Stream stream)
{
PerformanceDataManager.Increment(ProfilerConstants.NamedMessageReceived);
ProfilerStatManager.NamedMessage.Record();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave these types of changes out since it's just whitespace, I'm guessing you modified this and then undid the changes later but no need to touch the file with this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a mistake. I'll revert that.

using (var reader = PooledNetworkReader.Get(stream))
{
ulong hash = reader.ReadUInt64Packed();
Expand Down
56 changes: 44 additions & 12 deletions com.unity.multiplayer.mlapi/Runtime/Metrics/NetworkMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Unity.Multiplayer.NetStats.Dispatch;
using System.Collections.Generic;
using MLAPI.Connection;
using Unity.Multiplayer.NetStats.Dispatch;
using Unity.Multiplayer.NetStats.Metrics;
using Unity.Multiplayer.NetworkProfiler;
using Unity.Multiplayer.NetworkProfiler.Models;
Expand All @@ -7,28 +9,40 @@ namespace MLAPI.Metrics
{
public interface INetworkMetrics
{
void TrackNamedMessageSent(string messageName, ulong bytesCount);
void TrackNetworkObject(NetworkObject networkObject);

void TrackNamedMessageReceived(string messageName, ulong bytesCount);
void TrackNamedMessageSent(ulong receiver, string messageName, ulong bytesCount);

void TrackNamedMessageSent(IReadOnlyCollection<ulong> receivers, string messageName, ulong bytesCount);

void TrackNamedMessageReceived(ulong sender, string messageName, ulong bytesCount);

void DispatchFrame();
}

public class NullNetworkMetrics : INetworkMetrics
{
public void TrackNamedMessageSent(string messageName, ulong bytesCount)
public void TrackNetworkObject(NetworkObject networkObject)
{
}

public void TrackNamedMessageSent(ulong receiver, string messageName, ulong bytesCount)
{
}

public void TrackNamedMessageReceived(string messageName, ulong bytesCount)
public void TrackNamedMessageSent(IReadOnlyCollection<ulong> receivers, string messageName, ulong bytesCount)
{
}

public void TrackNamedMessageReceived(ulong sender, string messageName, ulong bytesCount)
{
}

public void DispatchFrame()
{
}
}

#if true
public class NetworkMetrics : INetworkMetrics
{
Expand All @@ -38,24 +52,42 @@ public class NetworkMetrics : INetworkMetrics
private EventMetric<NamedMessageEvent> m_NamedMessageSentEvent = new EventMetric<NamedMessageEvent>("Named Message Sent");
private EventMetric<NamedMessageEvent> m_NamedMessageReceivedEvent = new EventMetric<NamedMessageEvent>("Named Message Received");

private Dictionary<ulong, NetworkObjectIdentifier> m_NetworkGameObjects = new Dictionary<ulong, NetworkObjectIdentifier>();

public NetworkMetrics(NetworkManager networkManager)
{
m_NetworkManager = networkManager;
m_Dispatcher = new MetricDispatcherBuilder()
.WithMetricEvents(m_NamedMessageSentEvent, m_NamedMessageReceivedEvent)
.Build();

m_Dispatcher.RegisterObserver(MLAPIObserver.Observer);
}

public void TrackNamedMessageSent(string messageName, ulong bytesCount)
public void TrackNetworkObject(NetworkObject networkObject)
{
if (!m_NetworkGameObjects.ContainsKey(networkObject.NetworkObjectId))
{
m_NetworkGameObjects[networkObject.NetworkObjectId] = new NetworkObjectIdentifier(networkObject.name, networkObject.NetworkObjectId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if NetworkObjectIdentifier needs to be cached like this. It is a struct so creating the instance is free. Also this means that changes to the network object's name will not appear in the profiler.

Although .name might allocate memory since it pulls from native - in which case cashing might still be preferred to avoid the allocation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of thing add complexity, and we can only guess at the benefits. I'm a fan of keeping this stuff as simple as possible, and then optimizing when we have cold hard data to back it up.

If we have to optimize part of this, the solution might turn out to be something else entirely.

Make it work, make it clean, make it fast. In that order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I was under that impression that caching would be beneficial if we're going to be reusing the structures a lot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the cost isn't worth the incidental complexity it adds to the code.

}
}

public void TrackNamedMessageSent(ulong receiver, string messageName, ulong bytesCount)
{
m_NamedMessageSentEvent.Mark(new NamedMessageEvent(new ConnectionInfo(receiver), messageName, bytesCount));
}

public void TrackNamedMessageSent(IReadOnlyCollection<ulong> receivers, string messageName, ulong bytesCount)
{
m_NamedMessageSentEvent.Mark(new NamedMessageEvent(new ConnectionInfo(m_NetworkManager.LocalClientId), messageName, bytesCount));
foreach (var receiver in receivers)
{
TrackNamedMessageSent(receiver, messageName, bytesCount);
}
}

public void TrackNamedMessageReceived(string messageName, ulong bytesCount)
public void TrackNamedMessageReceived(ulong sender, string messageName, ulong bytesCount)
{
m_NamedMessageReceivedEvent.Mark(new NamedMessageEvent(new ConnectionInfo(m_NetworkManager.LocalClientId), messageName, bytesCount));
m_NamedMessageReceivedEvent.Mark(new NamedMessageEvent(new ConnectionInfo(sender), messageName, bytesCount));
}

public void DispatchFrame()
Expand All @@ -74,4 +106,4 @@ static MLAPIObserver()
}
}
#endif
}
}
2 changes: 2 additions & 0 deletions com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong netwo
SpawnedObjects.Add(networkObject.NetworkObjectId, networkObject);
SpawnedObjectsList.Add(networkObject);

NetworkManager.Singleton.NetworkMetrics.TrackNetworkObject(networkObject);

if (ownerClientId != null)
{
if (NetworkManager.IsServer)
Expand Down