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.

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

Jump to bottom

perf: Reduce the amount of redundant 'Change' data serialized by SyncDictionary #3610

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

perf: Reduce the amount of redundant 'Change' data serialized by SyncDictionary #3610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8b6fc21
SyncDictionary should not transmit redundant changes when the same ke…
ClaytonHunsinger Sep 17, 2023
b535a8b
Remove 2 usings that were added by mistake
ClaytonHunsinger Sep 17, 2023
89b2f83
Make new() more explicit for older versions of Unity
ClaytonHunsinger Sep 17, 2023
3eae582
Sync all changes, including redundant ones, as the default behaviour …
ClaytonHunsinger Sep 19, 2023
bc6f8ee
Merge pull request #1 from MirrorNetworking/master
cshunsinger Sep 21, 2023
e257c0a
Merge branch 'master' into SyncDictionary-do-not-sync-redundant-chang…
ClaytonHunsinger Sep 21, 2023
70771dc
Added unit tests
ClaytonHunsinger Feb 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
132 changes: 105 additions & 27 deletions Assets/Mirror/Core/SyncDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,82 @@ struct Change
internal TValue item;
}

// list of changes.
abstract class ChangesTracker
{
protected readonly List<Change> changes = new List<Change>();
public uint Count => (uint)changes.Count;
public List<Change> CurrentChanges => changes;
public virtual void ClearChanges() => changes.Clear();
public virtual void AddChange(Change changeToAdd) => changes.Add(changeToAdd);
public virtual void ResetTrackedFields() { }
}

// Traditional list of changes
// -> insert/delete/clear is only ONE change
// -> changing the same slot 10x caues 10 changes.
// -> changing the same slot 10x causes 10 changes.
// -> note that this grows until next sync(!)
// TODO Dictionary<key, change> to avoid ever growing changes / redundant changes!
readonly List<Change> changes = new List<Change>();
// -> may still be useful for some code that relies on knowing about every change that happens, including redundant changes
class AllChangesTracker : ChangesTracker { /* Nothing to override here */ }

// Map of changes
// -> insert/delete/set of any field results in AT MOST one change
// -> changing the same slot 10x causes 1 change
// -> clear operation deletes all tracked changes and replaces them with just 1 single tracked change
// -> Things only get a little trickier with tracking "changes ahead", aka "which changes were previously sent/received via SerializeAll/DeserializeAll when Deltas are being set/received."
// -> ResetTrackedFields is important for "changes head" as it means the next time any new slot gets changed, it results in at most 1 more change being added.
class SingleChangePerKeyTracker : ChangesTracker
{
readonly Dictionary<TKey, int> changeMap = new Dictionary<TKey, int>();
int changeCountDuringLastChangeMapReset = 0;

public override void ClearChanges()
{
changeMap.Clear();
changeCountDuringLastChangeMapReset = 0;
base.ClearChanges();
}

public override void AddChange(Change changeToAdd)
{
if(changeToAdd.operation == Operation.OP_CLEAR)
{
changeMap.Clear();
if(changeCountDuringLastChangeMapReset == 0)
{
// ResetTrackedFields was never called so just clear the whole changes list
changes.Clear();
}
if(changes.Count > changeCountDuringLastChangeMapReset)
{
// Remove everything from the changes list that was added after the last call to ResetTrackedFields
int removeCount = changes.Count - (changeCountDuringLastChangeMapReset + 1);
changes.RemoveRange(changeCountDuringLastChangeMapReset, removeCount);
}
base.AddChange(changeToAdd);
}
else if(changeMap.TryGetValue(changeToAdd.key, out int changeIndex))
{
// changeMap should never contain an index that does not exist in the changes list
// changeMap should always provide the index pointing to the last recorded change in the changes list for the given key
changes[changeIndex] = changeToAdd;
}
else
{
base.AddChange(changeToAdd);
changeMap[changeToAdd.key] = changes.Count - 1;
}
}

public override void ResetTrackedFields()
{
// This has to do with the 'changes ahead' system. By clearning changeMap, it means that up to 1 new change per slot
// this point will get added to the changes list even if the changes list itself already contains a change for said slot.
changeMap.Clear();
changeCountDuringLastChangeMapReset = changes.Count;
}
}

readonly ChangesTracker changeTracker;

// how many changes we need to ignore
// this is needed because when we initialize the list,
Expand All @@ -43,7 +113,7 @@ struct Change

public override void Reset()
{
changes.Clear();
changeTracker.ClearChanges();
changesAhead = 0;
objects.Clear();
}
Expand All @@ -58,11 +128,22 @@ public override void Reset()

// throw away all the changes
// this should be called after a successful sync
public override void ClearChanges() => changes.Clear();
public override void ClearChanges() => changeTracker.ClearChanges();

public SyncIDictionary(IDictionary<TKey, TValue> objects)
public SyncIDictionary(IDictionary<TKey, TValue> objects, bool efficientChangeTrackingAndSync = false)
{
this.objects = objects;

if (efficientChangeTrackingAndSync)
{
// Nearly all of the time, we should just sync 1 change per key to save bandwidth by not sending redundant changes
this.changeTracker = new SingleChangePerKeyTracker();
}
else
{
// Some applications may need to sync all changes in great detail, even if those changes are redundant
this.changeTracker = new AllChangesTracker();
}
}

void AddOperation(Operation op, TKey key, TValue item, bool checkAccess)
Expand All @@ -72,16 +153,14 @@ void AddOperation(Operation op, TKey key, TValue item, bool checkAccess)
throw new System.InvalidOperationException("SyncDictionaries can only be modified by the owner.");
}

Change change = new Change
{
operation = op,
key = key,
item = item
};

if (IsRecording())
{
changes.Add(change);
changeTracker.AddChange(new Change()
{
operation = op,
key = key,
item = item
});
OnDirty?.Invoke();
}

Expand All @@ -103,19 +182,19 @@ public override void OnSerializeAll(NetworkWriter writer)
// thus the client will need to skip all the pending changes
// or they would be applied again.
// So we write how many changes are pending
writer.WriteUInt((uint)changes.Count);
changeTracker.ResetTrackedFields();
writer.WriteUInt(changeTracker.Count);
}

public override void OnSerializeDelta(NetworkWriter writer)
{
// write all the queued up changes
writer.WriteUInt((uint)changes.Count);
writer.WriteUInt(changeTracker.Count);

for (int i = 0; i < changes.Count; i++)
List<Change> changes = changeTracker.CurrentChanges;
foreach(Change change in changes)
{
Change change = changes[i];
writer.WriteByte((byte)change.operation);

switch (change.operation)
{
case Operation.OP_ADD:
Expand All @@ -134,11 +213,11 @@ public override void OnSerializeDelta(NetworkWriter writer)

public override void OnDeserializeAll(NetworkReader reader)
{
// if init, write the full list content
// if init, write the full list content
int count = (int)reader.ReadUInt();

objects.Clear();
changes.Clear();
changeTracker.ClearChanges();

for (int i = 0; i < count; i++)
{
Expand All @@ -159,14 +238,13 @@ public override void OnDeserializeDelta(NetworkReader reader)

for (int i = 0; i < changesCount; i++)
{
Operation operation = (Operation)reader.ReadByte();

// apply the operation only if it is a new change
// that we have not applied yet
bool apply = changesAhead == 0;
TKey key = default;
TValue item = default;

Operation operation = (Operation)reader.ReadByte();
switch (operation)
{
case Operation.OP_ADD:
Expand Down Expand Up @@ -316,9 +394,9 @@ public bool Remove(KeyValuePair<TKey, TValue> item)

public class SyncDictionary<TKey, TValue> : SyncIDictionary<TKey, TValue>
{
public SyncDictionary() : base(new Dictionary<TKey, TValue>()) {}
public SyncDictionary(IEqualityComparer<TKey> eq) : base(new Dictionary<TKey, TValue>(eq)) {}
public SyncDictionary(IDictionary<TKey, TValue> d) : base(new Dictionary<TKey, TValue>(d)) {}
public SyncDictionary(bool efficientChangeTrackingAndSync = false) : base(new Dictionary<TKey, TValue>(), efficientChangeTrackingAndSync) {}
public SyncDictionary(IEqualityComparer<TKey> eq, bool efficientChangeTrackingAndSync = false) : base(new Dictionary<TKey, TValue>(eq), efficientChangeTrackingAndSync) {}
public SyncDictionary(IDictionary<TKey, TValue> d, bool efficientChangeTrackingAndSync = false) : base(new Dictionary<TKey, TValue>(d), efficientChangeTrackingAndSync) {}
public new Dictionary<TKey, TValue>.ValueCollection Values => ((Dictionary<TKey, TValue>)objects).Values;
public new Dictionary<TKey, TValue>.KeyCollection Keys => ((Dictionary<TKey, TValue>)objects).Keys;
public new Dictionary<TKey, TValue>.Enumerator GetEnumerator() => ((Dictionary<TKey, TValue>)objects).GetEnumerator();
Expand Down
13 changes: 10 additions & 3 deletions Assets/Mirror/Tests/Editor/SyncCollections/SyncDictionaryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

namespace Mirror.Tests.SyncCollections
{
[TestFixture]
[TestFixture(true)]
[TestFixture(false)]
public class SyncDictionaryTest
{
SyncDictionary<int, string> serverSyncDictionary;
SyncDictionary<int, string> clientSyncDictionary;
int serverSyncDictionaryDirtyCalled;
int clientSyncDictionaryDirtyCalled;
bool optimized;

void SerializeAllTo<T>(T fromList, T toList) where T : SyncObject
{
Expand All @@ -29,11 +31,16 @@ public class SyncDictionaryTest
fromList.ClearChanges();
}

public SyncDictionaryTest(bool optimized)
{
this.optimized = optimized;
}

[SetUp]
public void SetUp()
{
serverSyncDictionary = new SyncDictionary<int, string>();
clientSyncDictionary = new SyncDictionary<int, string>();
serverSyncDictionary = new SyncDictionary<int, string>(optimized);
clientSyncDictionary = new SyncDictionary<int, string>(optimized);

// set writable
serverSyncDictionary.IsWritable = () => true;
Expand Down