Skip to content

Commit 11c6d61

Browse files
PathoschildPathoschild
committed
optimize watcher IsChanged tracking
This commit... - avoids repeated collection count accesses since we can just track the result while updating; - and adds more short-circuiting in cases where we know watchers haven't changed.
1 parent f2d1fa5 commit 11c6d61

7 files changed

Lines changed: 77 additions & 21 deletions

File tree

‎src/SMAPI/Framework/StateTracking/FieldWatchers/ComparableListWatcher.cs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ internal class ComparableListWatcher<TValue> : BaseDisposableWatcher, ICollectio
3333
public string Name { get; }
3434

3535
/// <inheritdoc />
36-
public bool IsChanged => this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
36+
public bool IsChanged { get; private set; }
3737

3838
/// <inheritdoc />
3939
public IReadOnlyCollection<TValue> Added => this.AddedImpl;
@@ -61,6 +61,7 @@ public ComparableListWatcher(string name, ICollection<TValue> values, IEqualityC
6161
public void Update()
6262
{
6363
this.AssertNotDisposed();
64+
this.IsChanged = false;
6465

6566
// optimize for zero items
6667
if (this.CurrentValues.Count == 0)
@@ -69,6 +70,7 @@ public void Update()
6970
{
7071
this.RemovedImpl.AddRange(this.LastValues);
7172
this.LastValues.Clear();
73+
this.IsChanged = true;
7274
}
7375
return;
7476
}
@@ -78,19 +80,30 @@ public void Update()
7880
this.NewValues.AddRange(this.CurrentValues);
7981

8082
// detect changes
83+
bool changed = false;
8184
foreach (TValue value in this.LastValues)
8285
{
8386
if (!this.NewValues.Contains(value))
87+
{
8488
this.RemovedImpl.Add(value);
89+
changed = true;
90+
}
8591
}
8692
foreach (TValue value in this.NewValues)
8793
{
8894
if (!this.LastValues.Contains(value))
95+
{
8996
this.AddedImpl.Add(value);
97+
changed = true;
98+
}
9099
}
91100

92101
// save result
93-
(this.LastValues, this.NewValues) = (this.NewValues, this.LastValues); // reuse the now-unused previous 'LastValues' set on the next update
102+
if (changed)
103+
{
104+
this.IsChanged = true;
105+
(this.LastValues, this.NewValues) = (this.NewValues, this.LastValues); // reuse the now-unused previous 'LastValues' set on the next update
106+
}
94107
}
95108

96109
/// <inheritdoc />
@@ -100,5 +113,6 @@ public void Reset()
100113

101114
this.AddedImpl.Clear();
102115
this.RemovedImpl.Clear();
116+
this.IsChanged = false;
103117
}
104118
}

‎src/SMAPI/Framework/StateTracking/FieldWatchers/InventoryWatcher.cs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ internal class InventoryWatcher : BaseDisposableWatcher, ICollectionWatcher<Item
3535
public string Name { get; }
3636

3737
/// <inheritdoc />
38-
public bool IsChanged => this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
38+
public bool IsChanged { get; private set; }
3939

4040
/// <inheritdoc />
4141
public IReadOnlyCollection<Item> Added => this.AddedImpl;
@@ -64,6 +64,7 @@ public void Reset()
6464
{
6565
this.AddedImpl.Clear();
6666
this.RemovedImpl.Clear();
67+
this.IsChanged = false;
6768
}
6869

6970
/// <inheritdoc />
@@ -131,10 +132,16 @@ private void Add(Item? value)
131132
if (value == null)
132133
return;
133134

134-
if (this.RemovedImpl.Remove(value))
135+
if (this.RemovedImpl.Remove(value)) // re-added an item that was just removed
136+
{
135137
this.AddedImpl.Remove(value);
138+
this.IsChanged = this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
139+
}
136140
else
141+
{
137142
this.AddedImpl.Add(value);
143+
this.IsChanged = true;
144+
}
138145
}
139146

140147
/// <summary>Track a removed item.</summary>
@@ -144,9 +151,15 @@ private void Remove(Item? value)
144151
if (value == null)
145152
return;
146153

147-
if (this.AddedImpl.Remove(value))
154+
if (this.AddedImpl.Remove(value)) // removed an item that was just added
155+
{
148156
this.RemovedImpl.Remove(value);
157+
this.IsChanged = this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
158+
}
149159
else
160+
{
150161
this.RemovedImpl.Add(value);
162+
this.IsChanged = true;
163+
}
151164
}
152165
}

‎src/SMAPI/Framework/StateTracking/FieldWatchers/NetCollectionWatcher.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ internal class NetCollectionWatcher<TValue> : BaseDisposableWatcher, ICollection
2828
public string Name { get; }
2929

3030
/// <inheritdoc />
31-
public bool IsChanged => this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
31+
public bool IsChanged { get; private set; }
3232

3333
/// <inheritdoc />
3434
public IReadOnlyCollection<TValue> Added => this.AddedImpl;
@@ -64,6 +64,7 @@ public void Reset()
6464

6565
this.AddedImpl.Clear();
6666
this.RemovedImpl.Clear();
67+
this.IsChanged = false;
6768
}
6869

6970
/// <inheritdoc />
@@ -87,12 +88,14 @@ public override void Dispose()
8788
private void OnValueAdded(TValue value)
8889
{
8990
this.AddedImpl.Add(value);
91+
this.IsChanged = true;
9092
}
9193

9294
/// <summary>A callback invoked when an entry is removed from the collection.</summary>
9395
/// <param name="value">The added value.</param>
9496
private void OnValueRemoved(TValue value)
9597
{
9698
this.RemovedImpl.Add(value);
99+
this.IsChanged = true;
97100
}
98101
}

‎src/SMAPI/Framework/StateTracking/FieldWatchers/NetDictionaryWatcher.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ internal class NetDictionaryWatcher<TKey, TValue, TField, TSerialDict, TSelf> :
3535
public string Name { get; }
3636

3737
/// <inheritdoc />
38-
public bool IsChanged => this.PairsAdded.Count > 0 || this.PairsRemoved.Count > 0;
38+
public bool IsChanged { get; private set; }
3939

4040
/// <inheritdoc />
4141
public IReadOnlyCollection<KeyValuePair<TKey, TValue>> Added => this.PairsAdded;
@@ -72,6 +72,7 @@ public void Reset()
7272

7373
this.PairsAdded.Clear();
7474
this.PairsRemoved.Clear();
75+
this.IsChanged = false;
7576
}
7677

7778
/// <inheritdoc />
@@ -95,6 +96,7 @@ public override void Dispose()
9596
private void OnValueAdded(TKey key, TValue value)
9697
{
9798
this.PairsAdded[key] = value;
99+
this.IsChanged = true;
98100
}
99101

100102
/// <summary>A callback invoked when an entry is removed from the dictionary.</summary>
@@ -103,5 +105,6 @@ private void OnValueAdded(TKey key, TValue value)
103105
private void OnValueRemoved(TKey key, TValue value)
104106
{
105107
this.PairsRemoved.TryAdd(key, value);
108+
this.IsChanged = true;
106109
}
107110
}

‎src/SMAPI/Framework/StateTracking/FieldWatchers/ObservableCollectionWatcher.cs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ internal class ObservableCollectionWatcher<TValue> : BaseDisposableWatcher, ICol
3131
public string Name { get; }
3232

3333
/// <inheritdoc />
34-
public bool IsChanged => this.AddedImpl.Count > 0 || this.RemovedImpl.Count > 0;
34+
public bool IsChanged { get; private set; }
3535

3636
/// <inheritdoc />
3737
public IReadOnlyCollection<TValue> Added => this.AddedImpl;
@@ -66,6 +66,8 @@ public void Reset()
6666

6767
this.AddedImpl.Clear();
6868
this.RemovedImpl.Clear();
69+
70+
this.IsChanged = false;
6971
}
7072

7173
/// <inheritdoc />
@@ -88,7 +90,12 @@ private void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArg
8890
// clear
8991
if (e.Action == NotifyCollectionChangedAction.Reset)
9092
{
91-
this.RemovedImpl.AddRange(this.PreviousValues);
93+
if (this.PreviousValues.Count > 0)
94+
{
95+
this.RemovedImpl.AddRange(this.PreviousValues);
96+
this.IsChanged = true;
97+
}
98+
9299
this.PreviousValues.Clear();
93100
return;
94101
}
@@ -97,7 +104,11 @@ private void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArg
97104
if (e.OldItems != null)
98105
{
99106
foreach (TValue value in e.OldItems)
107+
{
100108
this.RemovedImpl.Add(value);
109+
this.IsChanged = true;
110+
}
111+
101112
this.PreviousValues.RemoveRange(e.OldStartingIndex, e.OldItems.Count);
102113
}
103114

@@ -108,6 +119,8 @@ private void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArg
108119
foreach (TValue value in e.NewItems)
109120
{
110121
this.AddedImpl.Add(value);
122+
this.IsChanged = true;
123+
111124
this.PreviousValues.Insert(insertAt, value);
112125
insertAt++;
113126
}

‎src/SMAPI/Framework/StateTracking/LocationTracker.cs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System;
21
using System.Collections.Generic;
32
using Microsoft.Xna.Framework;
43
using StardewModdingAPI.Framework.StateTracking.FieldWatchers;
@@ -19,9 +18,6 @@ internal class LocationTracker : IWatcher
1918
/// <summary>The underlying watchers.</summary>
2019
private readonly List<IWatcher> Watchers = [];
2120

22-
/// <summary>Whether any of the <see cref="Watchers"/> changed.</summary>
23-
private bool WatchersHaveChanges;
24-
2521

2622
/*********
2723
** Accessors
@@ -30,7 +26,8 @@ internal class LocationTracker : IWatcher
3026
public string Name { get; }
3127

3228
/// <inheritdoc />
33-
public bool IsChanged => this.WatchersHaveChanges;
29+
/// <remarks>This covers added/removed chests in the location, but *not* changes to chest inventories; see <see cref="ChestWatchers"/> for those.</remarks>
30+
public bool IsChanged { get; private set; }
3431

3532
/// <summary>The tracked location.</summary>
3633
public GameLocation Location { get; }
@@ -95,25 +92,30 @@ public LocationTracker(GameLocation location)
9592
/// <inheritdoc />
9693
public void Update()
9794
{
98-
this.WatchersHaveChanges = false;
95+
// track changes to location content
96+
bool changed = false;
9997
foreach (IWatcher watcher in this.Watchers)
10098
{
10199
watcher.Update();
102100

103-
if (watcher.IsChanged)
104-
this.WatchersHaveChanges = true;
101+
if (!changed && watcher.IsChanged)
102+
changed = true;
105103
}
104+
this.IsChanged = changed;
106105

107-
this.UpdateChestWatcherList(added: this.ObjectsWatcher.Added, removed: this.ObjectsWatcher.Removed);
106+
// add/remove chests to match
107+
if (changed && this.ObjectsWatcher.IsChanged)
108+
this.UpdateChestWatcherList(added: this.ObjectsWatcher.Added, removed: this.ObjectsWatcher.Removed);
108109

110+
// update chest inventory watchers
109111
foreach (var watcher in this.ChestWatchers)
110112
watcher.Value.Update();
111113
}
112114

113115
/// <inheritdoc />
114116
public void Reset()
115117
{
116-
this.WatchersHaveChanges = false;
118+
this.IsChanged = false;
117119

118120
foreach (IWatcher watcher in this.Watchers)
119121
watcher.Reset();

‎src/SMAPI/Framework/StateTracking/WorldLocationsTracker.cs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal class WorldLocationsTracker : IWatcher
4949
public string Name => nameof(WorldLocationsTracker);
5050

5151
/// <summary>Whether locations were added or removed since the last reset.</summary>
52-
public bool IsLocationListChanged => this.Added.Count > 0 || this.Removed.Count > 0;
52+
public bool IsLocationListChanged { get; private set; }
5353

5454
/// <inheritdoc />
5555
public bool IsChanged => this.IsLocationListChanged || this.LocationsHaveChanges;
@@ -81,13 +81,15 @@ public WorldLocationsTracker(ObservableCollection<GameLocation> locations, IList
8181
/// <inheritdoc />
8282
public void Update()
8383
{
84+
this.LocationsHaveChanges = false;
85+
this.IsLocationListChanged = false;
86+
8487
// update watchers
8588
this.LocationListWatcher.Update();
8689
this.MineLocationListWatcher.Update();
8790
this.VolcanoLocationListWatcher.Update();
8891

8992
// update location content watchers
90-
this.LocationsHaveChanges = false;
9193
foreach (LocationTracker watcher in this.Locations)
9294
{
9395
watcher.Update();
@@ -142,6 +144,8 @@ public void ResetLocationList()
142144
this.LocationListWatcher.Reset();
143145
this.MineLocationListWatcher.Reset();
144146
this.VolcanoLocationListWatcher.Reset();
147+
148+
this.IsLocationListChanged = false;
145149
}
146150

147151
/// <inheritdoc />
@@ -243,6 +247,8 @@ public void Add(GameLocation? location)
243247

244248
// add buildings
245249
this.Add(location.buildings);
250+
251+
this.IsLocationListChanged = true;
246252
}
247253

248254
/// <summary>Remove the given building.</summary>
@@ -272,6 +278,8 @@ public void Remove(GameLocation? location)
272278
this.LocationDict.Remove(location);
273279
watcher.Dispose();
274280
this.Remove(location.buildings);
281+
282+
this.IsLocationListChanged = true;
275283
}
276284
}
277285

0 commit comments

Comments
 (0)