Skip to content

Commit 6269c7d

Browse files
Bug Fix: Virtual sort on same page (#997)
* Performance improvement for sort and virtualize / sort and page * Fix comparer not applying for virtual sort when the data set is smaller then the virtual page
1 parent 2cefd22 commit 6269c7d

File tree

4 files changed

+53
-24
lines changed

4 files changed

+53
-24
lines changed

src/DynamicData.Tests/Cache/SortAndVirtualizeFixture.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
namespace DynamicData.Tests.Cache;
1212

1313

14+
15+
16+
1417
public sealed class SortAndVirtualizeWithComparerChangesFixture : SortAndVirtualizeFixtureBase
1518
{
1619
private BehaviorSubject<IComparer<Person>> _comparerSubject ;
@@ -44,6 +47,29 @@ public void ChangeComparer()
4447
actualResult = Aggregator.Data.Items.OrderBy(p => p, Comparer);
4548
actualResult.Should().BeEquivalentTo(expectedResult);
4649
}
50+
51+
[Fact]
52+
public void ChangeComparerDataSetSmallerThanVirtualSize()
53+
{
54+
var people = Enumerable.Range(1, 10).Select(i => new Person($"P{i:00}", i)).OrderBy(p => Guid.NewGuid());
55+
Source.AddOrUpdate(people);
56+
57+
// for first batch, it should use the results of the _virtualRequests subject (if a behaviour subject is used).
58+
var expectedResult = people.OrderBy(p => p, Comparer).Take(10).ToList();
59+
var actualResult = Aggregator.Data.Items.OrderBy(p => p, Comparer);
60+
actualResult.Should().BeEquivalentTo(expectedResult);
61+
62+
Aggregator.Messages[0].All(c => c.Reason == ChangeReason.Add).Should().BeTrue();
63+
64+
// change the comparer
65+
_comparerSubject.OnNext(_descComparer);
66+
67+
expectedResult = people.OrderBy(p => p, _descComparer).Take(10).ToList();
68+
actualResult = Aggregator.Data.Items.OrderBy(p => p, Comparer);
69+
actualResult.Should().BeEquivalentTo(expectedResult);
70+
71+
Aggregator.Messages[1].All(c => c.Reason == ChangeReason.Refresh).Should().BeTrue();
72+
}
4773
}
4874

4975
public sealed class SortAndVirtualizeFixture : SortAndVirtualizeFixtureBase

src/DynamicData/Cache/Internal/KeyValueComparer.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ public int Compare(KeyValuePair<TKey, TObject> x, KeyValuePair<TKey, TObject> y)
3333
return -1;
3434
}
3535

36+
if (x.Key is IComparable<TKey> xComp)
37+
{
38+
return xComp.CompareTo(y.Key);
39+
}
40+
3641
return x.Key.GetHashCode().CompareTo(y.Key.GetHashCode());
3742
}
3843
}

src/DynamicData/Cache/Internal/SortAndVirtualize.cs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,28 @@ private static ChangeSet<TObject, TKey, VirtualContext<TObject>> CalculateVirtua
151151
result.AddRange(removes);
152152
result.AddRange(adds);
153153

154-
if (changes is null) return result;
155-
156-
var keyInPreviousAndCurrent = new HashSet<TKey>(previousItems.Intersect(currentItems, _keyComparer).Select(x => x.Key));
157-
158-
foreach (var change in changes)
154+
if (changes is null)
159155
{
160-
// An update (or refresh) can only occur if it was in the previous or current result set.
161-
// If it was in only one or the other, it would be an add or remove accordingly.
162-
if (!keyInPreviousAndCurrent.Contains(change.Key)) continue;
156+
// the comparer has changed, so we need to send a Refresh for item in the same page
157+
foreach (var kvp in previousItems.Intersect(currentItems, _keyComparer))
158+
{
159+
result.Add(new Change<TObject, TKey>(ChangeReason.Refresh, kvp.Key, kvp.Value));
160+
}
161+
}
162+
else
163+
{
164+
var keyInPreviousAndCurrent = new HashSet<TKey>(previousItems.Intersect(currentItems, _keyComparer).Select(x => x.Key));
163165

164-
if (change.Reason is ChangeReason.Update or ChangeReason.Refresh)
166+
foreach (var change in changes)
165167
{
166-
result.Add(change);
168+
// An update (or refresh) can only occur if it was in the previous or current result set.
169+
// If it was in only one or the other, it would be an add or remove accordingly.
170+
if (!keyInPreviousAndCurrent.Contains(change.Key)) continue;
171+
172+
if (change.Reason is ChangeReason.Update or ChangeReason.Refresh)
173+
{
174+
result.Add(change);
175+
}
167176
}
168177
}
169178

src/DynamicData/Cache/Internal/SortedKeyValueApplicator.cs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,21 +79,10 @@ private void ApplyChanges(IChangeSet<TObject, TKey> changes)
7979
{
8080
var previous = new KeyValuePair<TKey, TObject>(change.Key, change.Previous.Value);
8181
var currentIndex = GetCurrentPosition(previous);
82-
var updatedIndex = GetInsertPosition(item);
83-
84-
// We need to recalibrate as GetCurrentPosition includes the current item
85-
updatedIndex = currentIndex < updatedIndex ? updatedIndex - 1 : updatedIndex;
82+
_target.RemoveAt(currentIndex);
8683

87-
// Some control suites and platforms do not support replace, whiles others do, so we opt in.
88-
if (_options.UseReplaceForUpdates && currentIndex == updatedIndex)
89-
{
90-
_target[currentIndex] = item;
91-
}
92-
else
93-
{
94-
_target.RemoveAt(currentIndex);
95-
_target.Insert(updatedIndex, item);
96-
}
84+
var updatedIndex = GetInsertPosition(item);
85+
_target.Insert(updatedIndex, item);
9786
}
9887
break;
9988
case ChangeReason.Remove:

0 commit comments

Comments
 (0)