Skip to content

Assert byte counts for metrics #975

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 4 commits into from
Jul 26, 2021
Merged

Assert byte counts for metrics #975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
37641ce
Assert byte count for messaging tests
Jul 23, 2021
ee6cd6d
Assert buffer sizes for network objects metric tests
Jul 23, 2021
8fdbd97
Add byte count assertions for metric tests
Jul 23, 2021
49beda7
Review fixes
Jul 26, 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
43 changes: 15 additions & 28 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using MLAPI.Configuration;
using MLAPI.Logging;
using MLAPI.Messaging;
using MLAPI.Metrics;
using MLAPI.NetworkVariable;
using MLAPI.Profiling;
using MLAPI.Reflection;
Expand Down Expand Up @@ -89,20 +90,13 @@ internal void __endSendServerRpc(NetworkSerializer serializer, uint rpcMethodId,
return;
}

var rpcQueueContainer = NetworkManager.RpcQueueContainer;
var rpcMessageSize = 0L;
if (IsHost)
{
rpcMessageSize = rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage);
}
else
{
rpcMessageSize = rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);
}
var rpcMessageSize = IsHost
? NetworkManager.RpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage)
: NetworkManager.RpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);

if (NetworkManager.__rpc_name_table.TryGetValue(rpcMethodId, out var rpcMethodName))
{
NetworkManager.NetworkMetrics.TrackRpcSent(NetworkManager.ServerClientId, NetworkObjectId, rpcMethodName, (ulong)rpcMessageSize);
NetworkManager.NetworkMetrics.TrackRpcSent(NetworkManager.ServerClientId, NetworkObjectId, rpcMethodName, rpcMessageSize);
}
}

Expand Down Expand Up @@ -188,11 +182,9 @@ internal void __endSendClientRpc(NetworkSerializer serializer, uint rpcMethodId,
}

var rpcQueueContainer = NetworkManager.RpcQueueContainer;
var messageSize = 0L;
string rpcMethodName;
if (IsHost)
{
ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
var clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
if (clientRpcParams.Send.TargetClientIds != null && clientRpcParams.Send.TargetClientIds.Length == 0)
{
clientIds = NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
Expand All @@ -201,22 +193,17 @@ internal void __endSendClientRpc(NetworkSerializer serializer, uint rpcMethodId,
var containsServerClientId = clientIds.Contains(NetworkManager.ServerClientId);
if (containsServerClientId && clientIds.Length == 1)
{
messageSize = rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage);

if (NetworkManager.__rpc_name_table.TryGetValue(rpcMethodId, out rpcMethodName))
{
NetworkManager.NetworkMetrics.TrackRpcSent(clientIds, NetworkObjectId, rpcMethodName, (ulong)messageSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this case not necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm reading it right, this is removed because it could produce redundant messages. Do we have test coverage for, "In these cases, RPCs are sent exactly once"? Coverage for "Client sending Rpc to Server" since it's a special case in code?
@Rosme

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a feeling this should get tested in MLAPI in general and not just metrics.

}
rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage);

return;
}
}

messageSize = rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);
var messageSize = rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);

if (NetworkManager.__rpc_name_table.TryGetValue(rpcMethodId, out rpcMethodName))
if (NetworkManager.__rpc_name_table.TryGetValue(rpcMethodId, out var rpcMethodName))
{
NetworkManager.NetworkMetrics.TrackRpcSent(NetworkManager.ServerClientId, NetworkObjectId, rpcMethodName, (ulong)messageSize);
NetworkManager.NetworkMetrics.TrackRpcSent(NetworkManager.ConnectedClients.Select(x => x.Key).ToArray(), NetworkObjectId, rpcMethodName, messageSize);
}
}

Expand Down Expand Up @@ -597,7 +584,9 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
writer.WriteUInt64Packed(NetworkObjectId);
writer.WriteUInt16Packed(NetworkObject.GetNetworkBehaviourOrderIndex(this));

bool writtenAny = false;
var bufferSizeCapture = new BufferSizeCapture(buffer);

var writtenAny = false;
for (int k = 0; k < NetworkVariableFields.Count; k++)
{
if (!m_ChannelMappedNetworkVariableIndexes[j].Contains(k))
Expand Down Expand Up @@ -639,7 +628,6 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
if (shouldWrite)
{
writtenAny = true;
var deltaBuffer = 0L;

if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
Expand All @@ -650,7 +638,6 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)

writer.WriteUInt16Packed((ushort)varBuffer.Length);
buffer.CopyFrom(varBuffer);
deltaBuffer = varBuffer.Length;
}
}
else
Expand All @@ -664,7 +651,7 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
m_NetworkVariableIndexesToReset.Add(k);
}

NetworkManager.NetworkMetrics.TrackNetworkVariableDeltaSent(clientId, NetworkObjectId, name, NetworkVariableFields[k].Name, (ulong)deltaBuffer);
NetworkManager.NetworkMetrics.TrackNetworkVariableDeltaSent(clientId, NetworkObjectId, name, NetworkVariableFields[k].Name, bufferSizeCapture.Flush());
}
}

Expand Down Expand Up @@ -754,7 +741,7 @@ internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkV
networkVariableList[i].ReadDelta(stream, networkManager.IsServer);
PerformanceDataManager.Increment(ProfilerConstants.NetworkVarDeltas);
ProfilerStatManager.NetworkVarsRcvd.Record();
networkManager.NetworkMetrics.TrackNetworkVariableDeltaReceived(clientId, logInstance.NetworkObjectId, logInstance.name, networkVariableList[i].Name, (ulong)stream.Length);
networkManager.NetworkMetrics.TrackNetworkVariableDeltaReceived(clientId, logInstance.NetworkObjectId, logInstance.name, networkVariableList[i].Name, stream.Length);

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
Expand Down
4 changes: 2 additions & 2 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ private void Initialize(bool server)
if (NetworkMetrics == null)
{
#if MULTIPLAYER_TOOLS
NetworkMetrics = new NetworkMetrics(this);
NetworkMetrics = new NetworkMetrics();
#else
NetworkMetrics = new NullNetworkMetrics();
#endif
Expand Down Expand Up @@ -1449,7 +1449,7 @@ internal void InvokeRpc(RpcFrameQueueItem queueItem)

if (__rpc_name_table.TryGetValue(networkRpcMethodId, out var rpcMethodName))
{
NetworkMetrics.TrackRpcReceived(queueItem.NetworkId, networkObjectId, rpcMethodName, (ulong)queueItem.MessageData.Count);
NetworkMetrics.TrackRpcReceived(queueItem.NetworkId, networkObjectId, rpcMethodName, queueItem.StreamSize);
}
}

Expand Down
17 changes: 10 additions & 7 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using MLAPI.Exceptions;
using MLAPI.Hashing;
using MLAPI.Logging;
using MLAPI.Metrics;
using MLAPI.Serialization.Pooled;
using MLAPI.Transports;
using MLAPI.Serialization;
Expand Down Expand Up @@ -267,7 +268,6 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI
throw new NotServerException("Only server can change visibility");
}


// Do the safety loop first to prevent putting the MLAPI in an invalid state.
for (int i = 0; i < networkObjects.Count; i++)
{
Expand All @@ -286,19 +286,22 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI
throw new ArgumentNullException("All " + nameof(NetworkObject) + "s must belong to the same " + nameof(NetworkManager));
}
}

using (var buffer = PooledNetworkBuffer.Get())
using (var writer = PooledNetworkWriter.Get(buffer))
{
writer.WriteUInt16Packed((ushort)networkObjects.Count);
long prevLength = 0;

var bufferSizeCapture = new BufferSizeCapture(buffer);

for (int i = 0; i < networkObjects.Count; i++)
{
// Send spawn call
networkObjects[i].Observers.Add(clientId);

networkManager.SpawnManager.WriteSpawnCallForObject(buffer, clientId, networkObjects[i], payload);
networkManager.NetworkMetrics.TrackObjectSpawnSent(clientId, networkObjects[i].NetworkObjectId, networkObjects[i].name, (ulong)(buffer.Length - prevLength));
prevLength = buffer.Length;

networkManager.NetworkMetrics.TrackObjectSpawnSent(clientId, networkObjects[i].NetworkObjectId, networkObjects[i].name, bufferSizeCapture.Flush());
}

networkManager.MessageSender.Send(clientId, NetworkConstants.ADD_OBJECTS, NetworkChannel.Internal, buffer);
Expand Down Expand Up @@ -392,16 +395,16 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
{
writer.WriteUInt16Packed((ushort)networkObjects.Count);

long prevLength = 0;
var bufferSizeCapture = new BufferSizeCapture(buffer);

for (int i = 0; i < networkObjects.Count; i++)
{
// Send destroy call
networkObjects[i].Observers.Remove(clientId);

writer.WriteUInt64Packed(networkObjects[i].NetworkObjectId);
networkManager.NetworkMetrics.TrackObjectDestroySent(clientId, networkObjects[i].NetworkObjectId, networkObjects[i].name, (ulong)(buffer.Length - prevLength));
prevLength = buffer.Length;

networkManager.NetworkMetrics.TrackObjectDestroySent(clientId, networkObjects[i].NetworkObjectId, networkObjects[i].name, bufferSizeCapture.Flush());
}

networkManager.MessageSender.Send(clientId, NetworkConstants.DESTROY_OBJECTS, NetworkChannel.Internal, buffer);
Expand Down
2 changes: 1 addition & 1 deletion com.unity.multiplayer.mlapi/Runtime/Logging/NetworkLog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private static void LogServer(string message, LogType logType)
writer.WriteStringPacked(message);

NetworkManager.Singleton.MessageSender.Send(NetworkManager.Singleton.ServerClientId, NetworkConstants.SERVER_LOG, NetworkChannel.Internal, buffer);
NetworkManager.Singleton.NetworkMetrics.TrackServerLogSent(NetworkManager.Singleton.ServerClientId, (uint)logType, (ulong)buffer.Length);
NetworkManager.Singleton.NetworkMetrics.TrackServerLogSent(NetworkManager.Singleton.ServerClientId, (uint)logType, buffer.Length);
}
}
}
Expand Down
23 changes: 7 additions & 16 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 @@ -5,6 +5,7 @@
using MLAPI.Serialization;
using MLAPI.Serialization.Pooled;
using MLAPI.Hashing;
using MLAPI.Metrics;
using MLAPI.Profiling;
using MLAPI.Transports;

Expand Down Expand Up @@ -37,14 +38,8 @@ internal CustomMessagingManager(NetworkManager networkManager)

internal void InvokeUnnamedMessage(ulong clientId, Stream stream)
{
var bytesCount = 0UL;
if (stream.CanSeek)
{
bytesCount = (ulong)stream.Length;
}

OnUnnamedMessage?.Invoke(clientId, stream);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageReceived(clientId, bytesCount);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageReceived(clientId, stream.SafeGetLengthOrDefault());
}

/// <summary>
Expand All @@ -67,7 +62,7 @@ public void SendUnnamedMessage(List<ulong> clientIds, NetworkBuffer buffer, Netw

m_NetworkManager.MessageSender.Send(NetworkConstants.UNNAMED_MESSAGE, networkChannel, clientIds, buffer);
PerformanceDataManager.Increment(ProfilerConstants.UnnamedMessageSent);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageSent(clientIds, (ulong)buffer.Length);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageSent(clientIds, buffer.Length);
}

/// <summary>
Expand All @@ -80,7 +75,7 @@ public void SendUnnamedMessage(ulong clientId, NetworkBuffer buffer, NetworkChan
{
m_NetworkManager.MessageSender.Send(clientId, NetworkConstants.UNNAMED_MESSAGE, networkChannel, buffer);
PerformanceDataManager.Increment(ProfilerConstants.UnnamedMessageSent);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageSent(clientId, (ulong)buffer.Length);
m_NetworkManager.NetworkMetrics.TrackUnnamedMessageSent(clientId, buffer.Length);
}

/// <summary>
Expand All @@ -96,11 +91,7 @@ public void SendUnnamedMessage(ulong clientId, NetworkBuffer buffer, NetworkChan

internal void InvokeNamedMessage(ulong hash, ulong sender, Stream stream)
{
var bytesCount = 0UL;
if (stream.CanSeek)
{
bytesCount = (ulong)stream.Length;
}
var bytesCount = stream.SafeGetLengthOrDefault();

if (m_NetworkManager == null)
{
Expand Down Expand Up @@ -203,7 +194,7 @@ 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(clientId, name, (ulong)messageBuffer.Length);
m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(clientId, name, messageBuffer.Length);
}
}

Expand Down Expand Up @@ -247,7 +238,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(clientIds, name, (ulong)messageBuffer.Length);
m_NetworkManager.NetworkMetrics.TrackNamedMessageSent(clientIds, name, messageBuffer.Length);
}
}
}
Expand Down
8 changes: 4 additions & 4 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 @@ -182,7 +182,7 @@ public void HandleAddObject(ulong clientId, Stream stream)
}
}

m_NetworkManager.NetworkMetrics.TrackObjectSpawnReceived(clientId, networkObject.NetworkObjectId, networkObject.name, (ulong)stream.Length);
m_NetworkManager.NetworkMetrics.TrackObjectSpawnReceived(clientId, networkObject.NetworkObjectId, networkObject.name, stream.Length);
}
}

Expand All @@ -198,7 +198,7 @@ public void HandleDestroyObject(ulong clientId, Stream stream)
Debug.LogWarning($"Trying to destroy object {networkId} but it doesn't seem to exist anymore!");
return;
}
m_NetworkManager.NetworkMetrics.TrackObjectDestroyReceived(clientId, networkId, networkObject.name, (ulong)stream.Length);
m_NetworkManager.NetworkMetrics.TrackObjectDestroyReceived(clientId, networkId, networkObject.name, stream.Length);
NetworkManager.SpawnManager.OnDespawnObject(networkObject, true);
}
}
Expand Down Expand Up @@ -248,7 +248,7 @@ public void HandleChangeOwner(ulong clientId, Stream stream)

networkObject.OwnerClientId = ownerClientId;

NetworkManager.NetworkMetrics.TrackOwnershipChangeReceived(clientId, networkObject.NetworkObjectId, networkObject.name, (ulong)stream.Length);
NetworkManager.NetworkMetrics.TrackOwnershipChangeReceived(clientId, networkObject.NetworkObjectId, networkObject.name, stream.Length);
}
}

Expand Down Expand Up @@ -385,7 +385,7 @@ public void HandleNetworkLog(ulong clientId, Stream stream)
{
var length = stream.Length;
var logType = (NetworkLog.LogType)reader.ReadByte();
m_NetworkManager.NetworkMetrics.TrackServerLogReceived(clientId, (uint)logType, (ulong)length);
m_NetworkManager.NetworkMetrics.TrackServerLogReceived(clientId, (uint)logType, length);
string message = reader.ReadStringPacked();

switch (logType)
Expand Down
28 changes: 28 additions & 0 deletions com.unity.multiplayer.mlapi/Runtime/Metrics/BufferSizeCapture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using MLAPI.Serialization;

namespace MLAPI.Metrics
{
public struct BufferSizeCapture
{
private readonly long m_InitialLength;
private readonly NetworkBuffer m_Buffer;

private long m_PreviousLength;

public BufferSizeCapture(NetworkBuffer buffer)
{
m_Buffer = buffer;
m_InitialLength = buffer.Length;
m_PreviousLength = m_InitialLength;
}

public long Flush()
{
var segmentLength = m_Buffer.Length - m_PreviousLength + m_InitialLength;

m_PreviousLength = m_Buffer.Length;

return segmentLength;
}
}
}
2 changes: 1 addition & 1 deletion ...me/Metrics/NetworkMetricsRpcTests.cs.meta → ...Runtime/Metrics/BufferSizeCapture.cs.meta
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading